t/helper: ignore only executable files
diff mbox series

Message ID 20190920093609.24935-1-szeder.dev@gmail.com
State New
Headers show
Series
  • t/helper: ignore only executable files
Related show

Commit Message

SZEDER Gábor Sept. 20, 2019, 9:36 a.m. UTC
This patch conceptually reverts 44103f4197 (t/helper: ignore
everything but sources, 2017-12-12).  Back in those days we did have a
lot of separate test helper executables under 't/helper', and its
'.gitignore' did get out of sync every once in a while.

Since then, however, most of those separate executables were
integrated into a single 'test-tool' command [1], and new test helpers
are added as new subcommands, so the chances of that '.gitignore'
getting out of sync again are much lower.  And even if a contributor
were not careful enough and submits a patch that adds a new executable
under 't/helper' but forgets to update '.gitignore' accordingly, our
CI builds would catch it in a timely manner [2].

Ignoring everything but sources has the drawback that building an
older version of Git (e.g. during bisecting) creates all those
executables, and after going back to e.g. current 'master' the usual
cleanup commands like 'make clean' or 'git clean -fd' don't remove
them (the former doesn't know about them, and the latter doesn't
remove ignored files).

So let's ignore only the executable files under 't/helper/, i.e.
'test-tool' and the three other remaining executables that could not
be integrated into 'test-tool' (no need to ignore object files, as
they are already ignored by our toplevel '.gitignore').

[1] The topic starting with efd71f8913 (t/helper: add an empty
    test-tool program, 2018-03-24), and leading up to the merge commit
    27f25845cf (Merge branch 'nd/combined-test-helper', 2018-04-11).

[2] b92cb86ea1 (travis-ci: check that all build artifacts are
    .gitignore-d, 2017-12-31)

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/helper/.gitignore | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Jeff King Sept. 23, 2019, 10:31 p.m. UTC | #1
On Fri, Sep 20, 2019 at 11:36:09AM +0200, SZEDER Gábor wrote:

> Ignoring everything but sources has the drawback that building an
> older version of Git (e.g. during bisecting) creates all those
> executables, and after going back to e.g. current 'master' the usual
> cleanup commands like 'make clean' or 'git clean -fd' don't remove
> them (the former doesn't know about them, and the latter doesn't
> remove ignored files).

Good reasoning. I've definitely been bit by this before when manually
testing something (for some reason, I still haven't trained my fingers
to type "test-tool foo" instead of "test-foo").

A similar one that bites me sometimes is that modern t5801 will fail
with an old built version of git-remote-testgit. That one _is_ correctly
handled in .gitignore, but you do have to remember to run "git clean".
But that's the best we can do, I think, unless we want to make t5801
more paranoid about running the version from t/t5801/git-remote-testgit.

> So let's ignore only the executable files under 't/helper/, i.e.
> 'test-tool' and the three other remaining executables that could not
> be integrated into 'test-tool' (no need to ignore object files, as
> they are already ignored by our toplevel '.gitignore').

Sounds like the right solution, and the patch looks good to me.

-Peff

Patch
diff mbox series

diff --git a/t/helper/.gitignore b/t/helper/.gitignore
index 2bad28af92..48c7bb0bbb 100644
--- a/t/helper/.gitignore
+++ b/t/helper/.gitignore
@@ -1,5 +1,4 @@ 
-*
-!*.sh
-!*.[ch]
-!*.gitignore
-
+/test-tool
+/test-fake-ssh
+/test-line-buffer
+/test-svn-fe