{"thread":{"id":"36572","subject":"[PATCH] completion: move out of contrib","startedAt":"2014-05-03T00:36:43Z","lastAt":"2014-05-07T19:45:17Z","messageCount":6,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"240602","messageId":"1399077403-19678-1-git-send-email-felipe.contreras@gmail.com","threadId":"36572","inReplyTo":null,"subject":"[PATCH] completion: move out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-03T00:36:43Z","receivedAt":"2014-05-03T00:36:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"These have been stable and widely used for quite a long time, they even\nhave tests outside of the contrib area, and most distributions ship\nthem, so they can be considered part of the core already.\n\nLet's move them out of contrib and install them by default.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Makefile                                           | 5 ++++-\n {contrib/completion => shared}/git-completion.bash | 0\n {contrib/completion => shared}/git-completion.zsh  | 0\n {contrib/completion => shared}/git-prompt.sh       | 0\n t/t9902-completion.sh                              | 2 +-\n t/t9903-bash-prompt.sh                             | 2 +-\n 6 files changed, 6 insertions(+), 3 deletions(-)\n rename {contrib/completion => shared}/git-completion.bash (100%)\n rename {contrib/completion => shared}/git-completion.zsh (100%)\n rename {contrib/completion => shared}/git-prompt.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex a53f3a8..4a022cd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1581,6 +1581,7 @@ template_dir_SQ = $(subst ','\\'',$(template_dir))\n htmldir_relative_SQ = $(subst ','\\'',$(htmldir_relative))\n prefix_SQ = $(subst ','\\'',$(prefix))\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))\n+sharedir_SQ = $(subst ','\\'',$(sharedir))\n \n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n@@ -2334,7 +2335,6 @@ endif\n mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n \n install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n-\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n@@ -2344,6 +2344,9 @@ install: all\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n \t$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n+\t$(INSTALL) -D -m 644 shared/git-completion.bash '$(DESTDIR_SQ)$(sharedir_SQ)'/bash-completion/completions/git\n+\t$(INSTALL) -D -m 644 shared/git-prompt.sh '$(DESTDIR_SQ)$(sharedir_SQ)'/git-core/git-prompt.sh\n+\t$(INSTALL) -D -m 644 shared/git-completion.zsh '$(DESTDIR_SQ)$(sharedir_SQ)'/zsh/site-functions/_git\n ifndef NO_GETTEXT\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n \t(cd po/build/locale && $(TAR) cf - .) | \\\ndiff --git a/contrib/completion/git-completion.bash b/shared/git-completion.bash\nsimilarity index 100%\nrename from contrib/completion/git-completion.bash\nrename to shared/git-completion.bash\ndiff --git a/contrib/completion/git-completion.zsh b/shared/git-completion.zsh\nsimilarity index 100%\nrename from contrib/completion/git-completion.zsh\nrename to shared/git-completion.zsh\ndiff --git a/contrib/completion/git-prompt.sh b/shared/git-prompt.sh\nsimilarity index 100%\nrename from contrib/completion/git-prompt.sh\nrename to shared/git-prompt.sh\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 2d4beb5..d8674e4 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -32,7 +32,7 @@ complete ()\n \n GIT_TESTING_COMMAND_COMPLETION='add checkout check-attr filter-branch ls-files'\n \n-. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n+. \"$GIT_BUILD_DIR/shared/git-completion.bash\"\n \n # We don't need this function to actually join words or do anything special.\n # Also, it's cleaner to avoid touching bash's internal completion variables.\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex 59f875e..272e5b3 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -7,7 +7,7 @@ test_description='test git-specific bash prompt functions'\n \n . ./lib-bash.sh\n \n-. \"$GIT_BUILD_DIR/contrib/completion/git-prompt.sh\"\n+. \"$GIT_BUILD_DIR/shared/git-prompt.sh\"\n \n actual=\"$TRASH_DIRECTORY/actual\"\n c_red='\\\\[\\\\e[31m\\\\]'\n-- \n1.9.2+fc1.20.g204a630\n"},{"id":"240862","messageId":"xmqq38gmctzn.fsf@gitster.dls.corp.google.com","threadId":"36572","inReplyTo":"1399077403-19678-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] completion: move out of contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T00:28:28Z","receivedAt":"2014-05-07T00:28:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> These have been stable and widely used for quite a long time, they even\n> have tests outside of the contrib area, and most distributions ship\n> them, so they can be considered part of the core already.\n>\n> Let's move them out of contrib and install them by default.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  Makefile                                           | 5 ++++-\n>  {contrib/completion => shared}/git-completion.bash | 0\n>  {contrib/completion => shared}/git-completion.zsh  | 0\n>  {contrib/completion => shared}/git-prompt.sh       | 0\n>  t/t9902-completion.sh                              | 2 +-\n>  t/t9903-bash-prompt.sh                             | 2 +-\n>  6 files changed, 6 insertions(+), 3 deletions(-)\n>  rename {contrib/completion => shared}/git-completion.bash (100%)\n>  rename {contrib/completion => shared}/git-completion.zsh (100%)\n>  rename {contrib/completion => shared}/git-prompt.sh (100%)\n\nI am not sure whom this change is meant to help.\n\n - Those who build from source *and* care about having the latest\n   completion must already have a way they have been using to\n   install them.  They will have to change their procedure to adjust\n   for the change of the path, *and* disable the part of \"install\"\n   that installs it to $(sharedir) which is unlikely to match where\n   they have been installing completion scripts.\n\n - Those who package completion for distros already have a way they\n   have been using to install them.  They suffer the same as those\n   who build from the source.\n\n - Those who use pre-packaged Git and completion scripts would not\n   care.\n\n - Those who have *not* installed from the source may benefit for\n   being able to say \"make install\" and let it be installed, but\n   they have to dot-include /usr/share/git-completion.bash location,\n   which is new, not from /etc/bash_completion.d/git as they are\n   used to.\n\nA better change might be to give a new Makefile target, perhaps\n\n    $ make install.contrib-completion\n\nwithout moving the scripts from their current place.  That way,\nnobody gets hurt, and those who are new to Git who want to build\nand install from the source would not have to invent their own way\nto install stuff from contrib/ (the same goes for other contrib/\ntools such as contrib/workdir/ we may want to add a new target to\nlet you say \"make install.contrib-workdir\").\n\nI _may_ be persuaded to fold the installation of all possible\ncontrib/ stuff into the regular \"make install\", but I haven't\nthought things through.\n\nThe patch does two unrelated things:\n\n - Move things in the source tree.\n\n - Install the completion by default.\n\nI very much agree that the latter may be a good thing to have in the\npolished end result.  I am not sure if the installation location\nchosen is sensible.  At least, another redirection\n\n    git_completion_dir = $(sharedir)\n\nmay be necessary to allow people install these in the location they\nwant.\n"},{"id":"240864","messageId":"536989d764672_250b14ed2ec66@nysa.notmuch","threadId":"36572","inReplyTo":"xmqq38gmctzn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] completion: move out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-07T01:18:15Z","receivedAt":"2014-05-07T01:18:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > These have been stable and widely used for quite a long time, they even\n> > have tests outside of the contrib area, and most distributions ship\n> > them, so they can be considered part of the core already.\n> >\n> > Let's move them out of contrib and install them by default.\n> >\n> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> > ---\n> >  Makefile                                           | 5 ++++-\n> >  {contrib/completion => shared}/git-completion.bash | 0\n> >  {contrib/completion => shared}/git-completion.zsh  | 0\n> >  {contrib/completion => shared}/git-prompt.sh       | 0\n> >  t/t9902-completion.sh                              | 2 +-\n> >  t/t9903-bash-prompt.sh                             | 2 +-\n> >  6 files changed, 6 insertions(+), 3 deletions(-)\n> >  rename {contrib/completion => shared}/git-completion.bash (100%)\n> >  rename {contrib/completion => shared}/git-completion.zsh (100%)\n> >  rename {contrib/completion => shared}/git-prompt.sh (100%)\n> \n> I am not sure whom this change is meant to help.\n> \n>  - Those who build from source *and* care about having the latest\n>    completion must already have a way they have been using to\n>    install them.  They will have to change their procedure to adjust\n>    for the change of the path, *and* disable the part of \"install\"\n>    that installs it to $(sharedir) which is unlikely to match where\n>    they have been installing completion scripts.\n\nNo.\n\n>  - Those who package completion for distros already have a way they\n>    have been using to install them.  They suffer the same as those\n>    who build from the source.\n\nYes, *if* they have been packaging them, they have a way. But what if\nthey haven't been doing so?\n\nAnd for the ones that have a way, now they need one hack less.\n\n>  - Those who use pre-packaged Git and completion scripts would not\n>    care.\n\nNo.\n\n>  - Those who have *not* installed from the source may benefit for\n>    being able to say \"make install\" and let it be installed, but\n>    they have to dot-include /usr/share/git-completion.bash location,\n>    which is new, not from /etc/bash_completion.d/git as they are\n>    used to.\n\nWrong. They don't have to dot-include anything, bash-completion does\nthat automagically. If they don't have bash-completion, sure, they'll\nneed to dot-include the new location.\n\nAnd the location is not /usr/share/git-completion.bash, it's\n$(sharedir)/bash-completion/completions.\n\nAlthough after reading bash-completion's README, it should actually be:\n\n % pkg-config --variable=completionsdir bash-completion\n\n> A better change might be to give a new Makefile target, perhaps\n> \n>     $ make install.contrib-completion\n> \n> without moving the scripts from their current place.\n\nBut they are not contrib, they are part of the core, and should be\ndistributed by default.\n\n> That way, nobody gets hurt, and those who are new to Git who want to\n> build and install from the source would not have to invent their own\n> way to install stuff from contrib/ (the same goes for other contrib/\n> tools such as contrib/workdir/ we may want to add a new target to let\n> you say \"make install.contrib-workdir\").\n\nThere is already a way to install from contrib.\n\n % make -C contrib/remote-helpers install\n\nThe fact that you never picked my fixes to make it so is another matter.\n\n> I _may_ be persuaded to fold the installation of all possible\n> contrib/ stuff into the regular \"make install\", but I haven't\n> thought things through.\n\nBut the completions are not contrib, they are essentially part of the\ncore.\n\nIf you want to demote them to contrib, then they shouldn't be tested by\ndefault, and t9902-completion.sh and t9903-bash-prompt.sh should be\nmoved out to contrib.\n\nYou cannot have your cake and eat it at the same time.\n\n> The patch does two unrelated things:\n> \n>  - Move things in the source tree.\n> \n>  - Install the completion by default.\n> \n> I very much agree that the latter may be a good thing to have in the\n> polished end result.  I am not sure if the installation location\n> chosen is sensible.  At least, another redirection\n> \n>     git_completion_dir = $(sharedir)\n> \n> may be necessary to allow people install these in the location they\n> want.\n\nifneq ($(prefix),$(HOME))\n\tbashcompletiondir := $(shell pkg-config --variable=completionsdir bash-completion 2> /dev/null)\nendif\nifndef bashcompletiondir\n\tbashcompletiondir = $(sharedir)/bash-completion/completions\nendif\n\n-- \nFelipe Contreras\n"},{"id":"240880","messageId":"536a067b3b1ca_5e8fbfd30879@nysa.notmuch","threadId":"36572","inReplyTo":"536989d764672_250b14ed2ec66@nysa.notmuch","subject":"Re: [PATCH] completion: move out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-07T10:10:03Z","receivedAt":"2014-05-07T10:10:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras wrote:\n> Yes, *if* they have been packaging them, they have a way. But what if\n> they haven't been doing so?\n> \n> And for the ones that have a way, now they need one hack less.\n\nAs an example of all the hacks needed by a real distribution package,\nhere's the stuff ArchLinux packagers have to do:\n\n  # bash completion\n  mkdir -p \"$pkgdir\"/usr/share/bash-completion/completions/\n  install -m644 ./contrib/completion/git-completion.bash \"$pkgdir\"/usr/share/bash-completion/completions/git\n  # fancy git prompt\n  mkdir -p \"$pkgdir\"/usr/share/git/\n  install -m644 ./contrib/completion/git-prompt.sh \"$pkgdir\"/usr/share/git/git-prompt.sh\n  # emacs\n  make -C contrib/emacs prefix=/usr DESTDIR=\"$pkgdir\" install\n  # gnome credentials helper\n  install -m755 contrib/credential/gnome-keyring/git-credential-gnome-keyring \\\n      \"$pkgdir\"/usr/lib/git-core/git-credential-gnome-keyring\n  make -C contrib/credential/gnome-keyring clean\n  # the rest of the contrib stuff\n  cp -a ./contrib/* $pkgdir/usr/share/git/\n\n  # scripts are for python 2.x\n  sed -i 's|#![ ]*/usr/bin/env python$|#!/usr/bin/env python2|' \\\n    $(find \"$pkgdir\" -name '*.py') \\\n    \"$pkgdir\"/usr/share/git/gitview/gitview \\\n    \"$pkgdir\"/usr/share/git/remote-helpers/git-remote-bzr \\\n    \"$pkgdir\"/usr/share/git/remote-helpers/git-remote-hg\n  sed -i 's|#![ ]*/usr/bin/python$|#!/usr/bin/python2|' \\\n    \"$pkgdir\"/usr/share/git/svn-fe/svnrdump_sim.py\n\n  # remove perllocal.pod, .packlist, and empty directories.\n  rm -rf \"$pkgdir\"/usr/lib/perl5\n\nAnd here's what debian packagers have to do:\n\n  # bash completion\n  install -d -m0755 '$(GIT)'/etc/bash_completion.d\n  install -m0644 contrib/completion/git-completion.bash \\\n    '$(GIT)'/etc/bash_completion.d/git\n  # gitweb\n  install -d -m0755 '$(GIT)'/usr/share/gitweb\n  ln -s gitweb.cgi '$(GIT)'/usr/share/gitweb/index.cgi\n  # contrib hooks\n  install -d -m0755 '$(GIT)'/usr/share/git-core/contrib/hooks\n  install -m0644 contrib/hooks/* \\\n    '$(GIT)'/usr/share/git-core/contrib/hooks/\n  # contrib\n  install -d -m0755 '$(GIT)'/usr/share/doc/git\n  cp -R contrib '$(GIT)'/usr/share/doc/git/\n  rm -rf '$(GIT)'/usr/share/doc/git/contrib/completion\n  rm -rf '$(GIT)'/usr/share/doc/git/contrib/emacs\n  find '$(GIT)'/usr/share/doc/git/contrib -type f | xargs chmod 0644\n  find '$(GIT)'/usr/share/doc/git/contrib -type d | xargs chmod 0755\n    # remove contrib hooks, they are now installed in\n    # /usr/share/git-core/contrib, keep symlink for backward compatibility\n  rm -rf '$(GIT)'/usr/share/doc/git/contrib/hooks\n  ln -s ../../../git-core/contrib/hooks \\\n    '$(GIT)'/usr/share/doc/git/contrib/\n  find '$(GIT)'/usr/share/doc/git/ -name .gitignore | xargs rm -f\n\nIf our build system was sane, they wouldn't need so many hacks.\n\n-- \nFelipe Contreras\n"},{"id":"240921","messageId":"xmqqppjpbicu.fsf@gitster.dls.corp.google.com","threadId":"36572","inReplyTo":"536a067b3b1ca_5e8fbfd30879@nysa.notmuch","subject":"Re: [PATCH] completion: move out of contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T17:37:21Z","receivedAt":"2014-05-07T17:37:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> As an example of all the hacks needed by a real distribution package,\n> here's the stuff ArchLinux packagers have to do:\n>\n>   # bash completion\n>   mkdir -p \"$pkgdir\"/usr/share/bash-completion/completions/\n>   install -m644 ./contrib/completion/git-completion.bash \"$pkgdir\"/usr/share/bash-completion/completions/git\n>...\n>\n> And here's what debian packagers have to do:\n>\n>   # bash completion\n>   install -d -m0755 '$(GIT)'/etc/bash_completion.d\n>   install -m0644 contrib/completion/git-completion.bash \\\n>     '$(GIT)'/etc/bash_completion.d/git\n>...\n>\n> If our build system was sane, they wouldn't need so many hacks.\n\nI do not see how the above two examples lead to that conclusion.\nHow would it help to blindly install to $(sharedir), or suggestion\nto use pkg-info when major distros do not even use one?\n\nI would understand if the saneness you seek were for distros to\nagree on where things should go, or at least agree on how to find\nout where things should go.\n\nThe situation for completion scripts unfortunately has to be even\nworse than others (like gitweb) in that distros, in order to serve\nusers (not upstream projects) better, want to package various pieces\nof completion scripts from different projects into a location their\nbash-completion package wants to see.  I applaud their effort and I\nwish that projects they pull various completion scriptlets from can\ndo more to help them, but I do not think a single makefile target to\nunconditionally install to $(sharedir) is it, and honestly speaking,\nthe distro completion packagers could do more to help us projects to\nhelp them by at least agreeing on how to find out where they want to\nsee our completion scripts installed.\n\nI do not think we are there yet.\n"},{"id":"240939","messageId":"536a8d4dd798e_76ff7a52ec26@nysa.notmuch","threadId":"36572","inReplyTo":"xmqqppjpbicu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] completion: move out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-07T19:45:17Z","receivedAt":"2014-05-07T19:45:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > As an example of all the hacks needed by a real distribution package,\n> > here's the stuff ArchLinux packagers have to do:\n> >\n> >   # bash completion\n> >   mkdir -p \"$pkgdir\"/usr/share/bash-completion/completions/\n> >   install -m644 ./contrib/completion/git-completion.bash \"$pkgdir\"/usr/share/bash-completion/completions/git\n> >...\n> >\n> > And here's what debian packagers have to do:\n> >\n> >   # bash completion\n> >   install -d -m0755 '$(GIT)'/etc/bash_completion.d\n> >   install -m0644 contrib/completion/git-completion.bash \\\n> >     '$(GIT)'/etc/bash_completion.d/git\n> >...\n> >\n\nThis is what the latest debian package does:\n\n\t# bash completion\n\tinstall -d -m0755 '$(GIT)'/usr/share/bash-completion/completions\n\tinstall -m0644 contrib/completion/git-completion.bash \\\n\t  '$(GIT)'/usr/share/bash-completion/completions/git\n\tln -s git '$(GIT)'/usr/share/bash-completion/completions/gitk\n\n> > If our build system was sane, they wouldn't need so many hacks.\n> \n> I do not see how the above two examples lead to that conclusion.\n> How would it help to blindly install to $(sharedir),\n\nIt is not blind, it is the location bash-completion uses *by default*,\nand it's what most (all?) distributions use.\n\n> or suggestion to use pkg-info when major distros do not even use one?\n\nWhich major distros do not ship with the pkg-config? It is part of\nbash-completion (as it should be part of every decent shared softare\ncomponent), they all ship it.\n\nDo you want me to go on a hunt and list all the distrubionts that ship\nboth?\n\n  /usr/share/bash-completion/completions/git\n  /usr/share/pkgconfig/bash-completion.pc\n\nHow many distributions would it take for you to accept the facts?\n\n> I would understand if the saneness you seek were for distros to\n> agree on where things should go, or at least agree on how to find\n> out where things should go.\n\nThey all gree.\n\n> I do not think we are there yet.\n\nYou are wrong.\n\n-- \nFelipe Contreras\n"}]}