diff mbox series

can we pull in git as a dependency for xfstests?

Message ID ZmfjlWGk04r_R5j6@infradead.org (mailing list archive)
State New, archived
Headers show
Series can we pull in git as a dependency for xfstests? | expand

Commit Message

Christoph Hellwig June 11, 2024, 5:41 a.m. UTC
xfs/073 has been failing for me for a while on most of my test setups
with:

diff: memory exhausted

from the recursive diff.  Switching to the significantly more memory
efficient implementation in git diff as in the patch below fixes this.

Would it be ok to pull in diff (including a supported check)?

Comments

Darrick J. Wong June 11, 2024, 2:24 p.m. UTC | #1
On Mon, Jun 10, 2024 at 10:41:41PM -0700, Christoph Hellwig wrote:
> xfs/073 has been failing for me for a while on most of my test setups
> with:
> 
> diff: memory exhausted
> 
> from the recursive diff.  Switching to the significantly more memory
> efficient implementation in git diff as in the patch below fixes this.
> 
> Would it be ok to pull in diff (including a supported check)?
> 
> diff --git a/tests/xfs/073 b/tests/xfs/073
> index c7616b9e9..85d8ae8d0 100755
> --- a/tests/xfs/073
> +++ b/tests/xfs/073
> @@ -76,7 +76,7 @@ _verify_copy()
>  	fi
>  
>  	echo comparing new image files to old
> -	diff -Naur $source_dir $target_dir
> +	git diff --no-index $source_dir $target_dir

How about

	(cd $source_dir ; find . -type f -print0 | xargs -0 md5sum) | \
		(cd $target_dir ; md5sum -c --quiet)

since 073.out doesn't contain any diff output?

--D

>  
>  	echo comparing new image directories to old
>  	find $source_dir | _filter_path $source_dir > $tmp.manifest1
>
Christoph Hellwig June 11, 2024, 4:25 p.m. UTC | #2
On Tue, Jun 11, 2024 at 07:24:05AM -0700, Darrick J. Wong wrote:
> > -	diff -Naur $source_dir $target_dir
> > +	git diff --no-index $source_dir $target_dir
> 
> How about
> 
> 	(cd $source_dir ; find . -type f -print0 | xargs -0 md5sum) | \
> 		(cd $target_dir ; md5sum -c --quiet)
> 
> since 073.out doesn't contain any diff output?

That works fine as well for me.
Zorro Lang June 12, 2024, 4:58 a.m. UTC | #3
On Mon, Jun 10, 2024 at 10:41:41PM -0700, Christoph Hellwig wrote:
> xfs/073 has been failing for me for a while on most of my test setups
> with:
> 
> diff: memory exhausted
> 
> from the recursive diff.  Switching to the significantly more memory
> efficient implementation in git diff as in the patch below fixes this.
> 
> Would it be ok to pull in diff (including a supported check)?
> 
> diff --git a/tests/xfs/073 b/tests/xfs/073
> index c7616b9e9..85d8ae8d0 100755
> --- a/tests/xfs/073
> +++ b/tests/xfs/073
> @@ -76,7 +76,7 @@ _verify_copy()
>  	fi
>  
>  	echo comparing new image files to old
> -	diff -Naur $source_dir $target_dir
> +	git diff --no-index $source_dir $target_dir

The "diff" is more widespread, if we can use diff, better to not bring in
a new necessary dependence. But you can make it to be optional if you
need. Likes:

_diff()
{
	local cmd="git diff"

	grep -wq -- "--no-index" <($cmd --help 2>/dev/null)
	if [ $? -ne 0 ];then
		cmd=diff
	else
		cmd="$cmd --no-index"
	if

	$cmd $*
}

But there might be some incompatible options betweeen "diff" and "git diff".

Thanks,
Zorro

>  
>  	echo comparing new image directories to old
>  	find $source_dir | _filter_path $source_dir > $tmp.manifest1
>
Christoph Hellwig June 12, 2024, 5:10 a.m. UTC | #4
On Wed, Jun 12, 2024 at 12:58:24PM +0800, Zorro Lang wrote:
> But there might be some incompatible options betweeen "diff" and "git diff".

Yes, many of the common diff options are not supported.

But I think the md5sum version from Darrick work just as fine, so I'll
wait for him to submit that.
diff mbox series

Patch

diff --git a/tests/xfs/073 b/tests/xfs/073
index c7616b9e9..85d8ae8d0 100755
--- a/tests/xfs/073
+++ b/tests/xfs/073
@@ -76,7 +76,7 @@  _verify_copy()
 	fi
 
 	echo comparing new image files to old
-	diff -Naur $source_dir $target_dir
+	git diff --no-index $source_dir $target_dir
 
 	echo comparing new image directories to old
 	find $source_dir | _filter_path $source_dir > $tmp.manifest1