Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >> Sorry to notice this so late, but this hunk caught my eye. What happens >> if `TEST_DIRECTORY` is provided by the user (and doesn't end in "/t")? > > I think that the preceding 2/4 should cover your concern here, i.e. I > think that's not possible. > >> Before this change, we would have set GIT_BUILD_DIR to the parent of >> whatever TEST_DIRECTORY is, whether or not it ended in "/t". We'll still >> do the same thing with this patch if TEST_DIRECTORY ends in "/t". But if >> it doesn't, then we'll set GIT_BUILD_DIR to be the same as >> TEST_DIRECTORY, which is a behavior change. > > Indeed, but I believe (again see 2/4) that can't happen. It is not like "can't happen", but "whoever wrote the TEST_DIRECTORY override logic did not mean to support such a use case". I am perfectly fine if we declared that we do not to support the use of that override mechanism by anybody but the "subtest" thing we do ourselves. If we can catch a workflow that misuses the mechansim cheaply enough (e.g. perhaps erroring out if TEST_DIRECTORY is set and it does not end in "/t"), we should do so, I would think, instead of doing the "go up and do pwd", which will make things worse. Thanks.
