On 10/3/24 17:59, Junio C Hamano wrote: > Ignacio Encinas <ignacio@xxxxxxxxxxxx> writes: > >> +test_expect_success 'conditional include, hostname' ' >> + cat >>.git/config <<-EOF && >> + [includeIf "hostname:$(hostname)a"] > > This unconditionally runs the $(hostname) command assuming it exists > everywhere, but > > $ git grep '$(hostname' t/ > t/t6500-gc.sh: hostname=$(hostname || echo unknown) && > > tells us that we should be prepared to meet a platform where such a > command does not exist. Oops, it didn't even cross my mind. Thanks for the catch! > I have a feeling that it is better done with a test prerequisite > than hardcoded "unknown", as xgethostname() at C level may come up > with some random string but it is not guaranteed to be "unknown". I agree. Not being able to query the current hostname defeats the purpose of the tests. > Perhaps have one instance of this before these added tests > > test_lazy_prereq WORKING_HOSTNAME ' > hostname >/dev/null 2>&1 > ' > > and then start them with > > test_expect_success WORKING_HOSTNAME 'hostname: includeIf' ' > ... > ' Thanks for providing an example. > or something? Others may think of a better way to make sure this > test does not cause false failures on platforms only because they > lack working hostname(1) but have a working gethostname(2) and their > xgethostname() may be working fine. I can't think of any room for improvement other than integrating hostname (or a custom hostname) into git and using it in the tests, but I doubt it is worth it. > Thanks. Thank you. I will wait a couple of days to post the v3 to see if anyone has a suggestion.
