Re: [PATCH v2 0/2] extra: new concept of extra components

[Philippe Blain <levraiphilippeblain@gmail.com>]

Hi Felipe,

Le 2021-07-10 à 19:46, Felipe Contreras a écrit :
> This patch series introduces the concept of extra components. These are
> components which are not yet part of the core but are good enough for
> distributions to ship, and in fact, they already do.
> 
> This benefits everyone:
> 
>   1. Distribution packagers that just want to do `make install`
>   2. People who download git's source code and just want to do
>      `make install`
>   3. Developers who have no idea what's production-level quality in
>      contrib/ and just want to do `make install`.
> 
> For now they'll have to do `make install install-extra`. But if the
> result is deemed correct, we might choose to add "install-extra" to the
> "install" target.

I agree with the end goal of this series. I myself carry a patch that adds an
'install-completion' target to the main Makefile that I apply before installing
Git from 'master'.

> 
> The measuring stick I'm using to gauge if a component in contrib belongs
> in extra is simple: are we already running tests for them with
> 'make test'? If the answer is "yes, we do run tests", then the answer is
> "yes, it belongs in contrib".

OK, this seems about right, it should cover prompt and completion, but...

> 
> We might want to move more components from contrib to extra once their
> tests are being run reliably.
> 
> And we might move some components from the core which aren't really part
> of the core to extra, like gitk, git-gui, git-p4, and git-svn.
> 
> For now only contrib/completion and contrib/workdir are graduated to the
> new area.

... when I read this I went "what is this workdir thing, it must date from before
'git worktree' was added". And combing through the history, it does. The latest
commit to the script is e32afab7b0 (git-new-workdir: don't fail if the target
directory is empty, 2014-11-26), which describes as v2.3.0-rc0~60^2. And
'git worktree' was shipped in Git 2.5, 2015-07-27.

Looking at the tests, I see two uses of 'git-new-workdir':
$ git grep  -p 'new-workdir'
t1021-rerere-in-workdir.sh=28=test_expect_success SYMLINKS 'rerere in workdir' '
t1021-rerere-in-workdir.sh:30:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . work &&
t1021-rerere-in-workdir.sh:41:50:# For the purpose of helping contrib/workdir/git-new-workdir users, we do not
t1021-rerere-in-workdir.sh=44=test_expect_failure SYMLINKS 'rerere in workdir (relative)' '
t1021-rerere-in-workdir.sh:46:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . krow &&
t3000-ls-files-others.sh=75=test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
t3000-ls-files-others.sh:87:57:         "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&

So they are not really testing this script per se, more like testing rerere and ls-files
in a worktree created by 'git-new-workdir'. I do not think this enough justification
to include 'git new-workdir' in 'extra/', since 'git worktree add' does the same thing
and is a builtin command. Even if its "BUGS" section in the doc says it's "in general [...]
still experimental", an experimental builtin is better than a 'contrib' script, no ?

Cheers,

Philippe.