Re: [PATCH] completion: move out of contrib

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> 
> > These have been stable and widely used for quite a long time, they even
> > have tests outside of the contrib area, and most distributions ship
> > them, so they can be considered part of the core already.
> >
> > Let's move them out of contrib and install them by default.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> > ---
> >  Makefile                                           | 5 ++++-
> >  {contrib/completion => shared}/git-completion.bash | 0
> >  {contrib/completion => shared}/git-completion.zsh  | 0
> >  {contrib/completion => shared}/git-prompt.sh       | 0
> >  t/t9902-completion.sh                              | 2 +-
> >  t/t9903-bash-prompt.sh                             | 2 +-
> >  6 files changed, 6 insertions(+), 3 deletions(-)
> >  rename {contrib/completion => shared}/git-completion.bash (100%)
> >  rename {contrib/completion => shared}/git-completion.zsh (100%)
> >  rename {contrib/completion => shared}/git-prompt.sh (100%)
> 
> I am not sure whom this change is meant to help.
> 
>  - Those who build from source *and* care about having the latest
>    completion must already have a way they have been using to
>    install them.  They will have to change their procedure to adjust
>    for the change of the path, *and* disable the part of "install"
>    that installs it to $(sharedir) which is unlikely to match where
>    they have been installing completion scripts.

No.

>  - Those who package completion for distros already have a way they
>    have been using to install them.  They suffer the same as those
>    who build from the source.

Yes, *if* they have been packaging them, they have a way. But what if
they haven't been doing so?

And for the ones that have a way, now they need one hack less.

>  - Those who use pre-packaged Git and completion scripts would not
>    care.

No.

>  - Those who have *not* installed from the source may benefit for
>    being able to say "make install" and let it be installed, but
>    they have to dot-include /usr/share/git-completion.bash location,
>    which is new, not from /etc/bash_completion.d/git as they are
>    used to.

Wrong. They don't have to dot-include anything, bash-completion does
that automagically. If they don't have bash-completion, sure, they'll
need to dot-include the new location.

And the location is not /usr/share/git-completion.bash, it's
$(sharedir)/bash-completion/completions.

Although after reading bash-completion's README, it should actually be:

 % pkg-config --variable=completionsdir bash-completion

> A better change might be to give a new Makefile target, perhaps
> 
>     $ make install.contrib-completion
> 
> without moving the scripts from their current place.

But they are not contrib, they are part of the core, and should be
distributed by default.

> That way, nobody gets hurt, and those who are new to Git who want to
> build and install from the source would not have to invent their own
> way to install stuff from contrib/ (the same goes for other contrib/
> tools such as contrib/workdir/ we may want to add a new target to let
> you say "make install.contrib-workdir").

There is already a way to install from contrib.

 % make -C contrib/remote-helpers install

The fact that you never picked my fixes to make it so is another matter.

> I _may_ be persuaded to fold the installation of all possible
> contrib/ stuff into the regular "make install", but I haven't
> thought things through.

But the completions are not contrib, they are essentially part of the
core.

If you want to demote them to contrib, then they shouldn't be tested by
default, and t9902-completion.sh and t9903-bash-prompt.sh should be
moved out to contrib.

You cannot have your cake and eat it at the same time.

> The patch does two unrelated things:
> 
>  - Move things in the source tree.
> 
>  - Install the completion by default.
> 
> I very much agree that the latter may be a good thing to have in the
> polished end result.  I am not sure if the installation location
> chosen is sensible.  At least, another redirection
> 
>     git_completion_dir = $(sharedir)
> 
> may be necessary to allow people install these in the location they
> want.

ifneq ($(prefix),$(HOME))
	bashcompletiondir := $(shell pkg-config --variable=completionsdir bash-completion 2> /dev/null)
endif
ifndef bashcompletiondir
	bashcompletiondir = $(sharedir)/bash-completion/completions
endif

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]