Re: What's cooking in git.git (Jun 2017, #09; Fri, 30)

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 5, 2017, 16:22 UTC
Message-ID: <xmqq60f6kg5m.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <87podkehcx.fsf@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 20 quoted lines

> On Fri, Jun 30 2017, Junio C. Hamano jotted:
>
>> * xz/send-email-batch-size (2017-05-23) 1 commit
>>  - send-email: --batch-size to work around some SMTP server limit
>>
>>  "git send-email" learned to overcome some SMTP server limitation
>>  that does not allow many pieces of e-mails to be sent over a single
>>  session.
>>
>>  Waiting for a response.
>>  cf. <CACBZZX5GYV50rjg9X602JHqFPaoofH9TwDf_-r_MDu8-rmNV6Q@mail.gmail.com>
>>  cf. <xmqqo9tfff2w.fsf@gitster.mtv.corp.google.com>
>>
>>  """I thought your wish (which I found reasonable) was to record
>>  whatever information that would help us in the future in the log
>>  message?  I was waiting for that to happen."""
>
> I think it's fine in lieu of xiaoqiang zhao not being responsive to just
> merge this as-is. The info that can help us in the future is in the ML
> archive, which should be good enough.

OK. I'll amend to add a few lines of note to the commit log and then merge it down.

Show 7 quoted lines

> So my WIP is, and I'd like feedback on the viability of the general approach:
>
>     create_test_file() {
>     	file=$1
>
>     	# `touch .` will succeed but obviously not do what we intend
>     	# here.

If you want to create, do not use "touch" that gives readers a false and confusing impression that you care about the timestamp of the thing being updated. If you say ">./$file", you can get an error from the shell just fine, I think.

>     	test "$file" = "." && return 1
>     	# We cannot create a file with an empty filename.
>     	test "$file" = "" && return 1

Likewise, as that would become ">./".

Show 5 quoted lines

>     	# The tests that are testing that e.g. foo//bar is matched by
>     	# foo/*/bar can't be tested on filesystems since there's no
>     	# way we're getting a double slash.
>     	echo "$file" | grep -F '//' && return 1
>     	dirs=$(echo "$file" | sed -r 's!/[^/]+$!!')

GNUism already pointed out, I think.

Show 47 quoted lines

>
>     	# We touch "./$file" instead of "$file" because even an
>     	# escaped "touch -- -" means something different.
>     	if test "$file" != "$dirs"
>     	then
>     		mkdir -p -- "$dirs" 2>/dev/null &&
>     		touch -- "./$file" 2>/dev/null &&
>     		return 0
>     	else
>     		touch -- "./$file" 2>/dev/null &&
>     		return 0
>     	fi
>     	return 1
>     }
>
> And then later on for the tests I do:
>
> 	# The existing test
> 	test_expect_success "wildmatch:     match '$text' '$pattern'" "
> 		test-wildmatch wildmatch '$text' '$pattern'
> 	"
>
> 	# My new test
> 	if create_test_file "$text"
> 	then
> 		test_expect_success "wildmatch(ls): match '$pattern' '$text'" "
> 			test_when_finished \"
> 				rm -rf -- * &&
> 				git reset
> 			\" &&
> 			git add -A &&
> 			>expect.err &&
> 			printf '%s' '$text' >expect &&
> 			git --glob-pathspecs ls-files -z -- '$pattern' 2>actual.err | tr -d '\0' >actual &&
> 			test_cmp expect.err actual.err &&
> 			test_cmp expect actual
> 		"
> 	else
> 		test_expect_failure "wildmatch(ls): match skip '$pattern' '$text'" 'false'
> 	fi
>
> This still needs to be cleaned up a bit to be parameterized (i.e. the
> --glob-pathspecs argument, whether it should error etc.).
>
> It works for me on Linux, but I'm wondering if others see any issues
> with the approach, does simply trying to create bizarro filenames on
> some OSs cause issues? I don't think so, but let's make sure.

Issues meaning that the test wants to see how a pathname with ^I in it works but create_test_file step would fail? Your above construct covers that with the if/then/else test_expect_failure/fi just fine, I think.