Re: Build broken for contrib/remote-helpers...

2013-01-22 Thread Junio C Hamano
Torsten Bögershausen  writes:

>> So it finds whatever is before the first "-", which would be the test
>> number in "t-basic.sh" or similar, and then looks for duplicates.
>
> would it help to filter for numbered tests before sorting like this:
>
> sed 's/-.*//' | grep "[0-9][0-9][0-9][0-9]"| sort | uniq -d

If you are using sed you do not need grep.  Just do that in a single
process, perhaps like

sed -ne 's|\(.*/\)*t\([0-9][0-9][0-9][0-9]\)-.*|\2|p'

But more importantly, what do we want "duplicate numbers" sanity
check to mean in this context?  Is this other directory allowed to
have a numbered test that shares the same number as the main test
suite?  Is uniqueness inside the contrib/remote-helpers/ directory
sufficient?

Do we envision that perhaps someday the contrib material becomes
mature enough and will want to migrate to the main part of the tree?
If that is the case, perhaps should the check complain that these
tests are not numbered, instead of ignoring?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Build broken for contrib/remote-helpers...

2013-01-22 Thread Torsten Bögershausen
On 22.01.13 20:41, Jeff King wrote:
> On Tue, Jan 22, 2013 at 12:49:31AM -0500, John Szakmeister wrote:
> 
>> I tried running make in contrib/remote-helpers and it died with:
>>
>> :: make
>> make -e -C ../../t test
>> rm -f -r test-results
>> duplicate test numbers: /Users/jszakmeister/sources/git
>> make[1]: *** [test-lint-duplicates] Error 1
>> make: *** [test] Error 2
>>
>> The path shown is not quite correct.  I have the sources extracted to
>> /Users/jszakmeister/sources/git-1.8.1.1.  It appears that the Makefile
>> in contrib/remote-helpers is exporting T, which is causing the
>> duplicate test detection to fail.
> 
> It has to set T, because that is how t/Makefile knows what the set of
> tests is. The problem is that test-lint-duplicates does not understand
> absolute pathnames, as its regex is too simplistic:
> 
>   sed 's/-.*//' | sort | uniq -d
> 
> So it finds whatever is before the first "-", which would be the test
> number in "t-basic.sh" or similar, and then looks for duplicates.

would it help to filter for numbered tests before sorting like this:

sed 's/-.*//' | grep "[0-9][0-9][0-9][0-9]"| sort | uniq -d

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Build broken for contrib/remote-helpers...

2013-01-22 Thread Jeff King
On Tue, Jan 22, 2013 at 12:49:31AM -0500, John Szakmeister wrote:

> I tried running make in contrib/remote-helpers and it died with:
> 
> :: make
> make -e -C ../../t test
> rm -f -r test-results
> duplicate test numbers: /Users/jszakmeister/sources/git
> make[1]: *** [test-lint-duplicates] Error 1
> make: *** [test] Error 2
> 
> The path shown is not quite correct.  I have the sources extracted to
> /Users/jszakmeister/sources/git-1.8.1.1.  It appears that the Makefile
> in contrib/remote-helpers is exporting T, which is causing the
> duplicate test detection to fail.

It has to set T, because that is how t/Makefile knows what the set of
tests is. The problem is that test-lint-duplicates does not understand
absolute pathnames, as its regex is too simplistic:

  sed 's/-.*//' | sort | uniq -d

So it finds whatever is before the first "-", which would be the test
number in "t-basic.sh" or similar, and then looks for duplicates.

We can make the regex more strict to handle full paths, like:

  perl -lne 'print $1 if m{(?:^|/)(t\d{4})-}'

but that still would not help, as the tests in remote-helpers do not
follow the t convention. So I think even running
test-lint-duplicates on them is nonsensical.

Maybe something like this would be more appropriate, though it kills off
all test-lint checks, not just test-lint-duplicates:

diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
index 9a76575..9c18ed8 100644
--- a/contrib/remote-helpers/Makefile
+++ b/contrib/remote-helpers/Makefile
@@ -3,6 +3,7 @@ export PATH := $(CURDIR):$(PATH)
 export T := $(addprefix $(CURDIR)/,$(TESTS))
 export MAKE := $(MAKE) -e
 export PATH := $(CURDIR):$(PATH)
+export TEST_LINT :=
 
 test:
$(MAKE) -C ../../t $@
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html