Bug 854061 - pollutes environment
Summary: pollutes environment
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: git
Version: rawhide
Hardware: Unspecified
OS: Unspecified
urgent
unspecified
Target Milestone: ---
Assignee: Chris Wright
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-09-03 20:03 UTC by Enrico Scholz
Modified: 2013-01-10 11:03 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-11-22 15:52:16 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Proposed patch (2.08 KB, patch)
2012-10-25 14:55 UTC, Adam Tkac
no flags Details | Diff

Description Enrico Scholz 2012-09-03 20:03:41 UTC
Description of problem:

git installs a file in /etc/profile.d which pollutes environment of every user with a lot of stuff which is unrelated to git tasks.  This was introduced because a previously provided functionality has been changed by upstream and login of a handful of people will display an error message now.

People affected by this error message had to modify their startup script already (else, they would not see the error message) and can restore functionality easily when file will be moved to a fixed position (e.g. under /usr/share/git).

For all other people, the 'set' output will become extremely cluttered (e.g. it increases from 137 lines to 379 here) without providing neither any functionality nor a way to disable it.  Login will be also slowed down because new functions must be registered.

git-prompt.sh is full of bash-isms and will cause errors for users with another sh like shell.

git is the only package which installs such unused functions for every user. /etc/profile.d should not be abused for such stuff.

There are better ways to provide the old functionality:

* move stuff to a fixed location (e.g. /usr/share/git); the handful of affected users will probably not have a problem with replacing 

  | . /etc/bash-completion.d/git

  by

  | . /usr/share/git/git-prompt.sh

  This is probably the most clean solution.

* patch /etc/bash-completion.d/git so that git-prompt.sh is sourced.  It might violate the Fedora go-upstream strategy, but upstream does not install git-prompt.sh in /etc/profile.d...

* move it into an own subpackage so that people can opt-in when they require __git_ps1().  But it might be overkill to create subpackage for a single script.

* make __git_ps1 a shell script which is installed below /usr/bin.


As this affects F17 -> F18/rawhide upgrades only, there is always expected some breakage and no special upgrade path (except perhaps an entry in the upgrade notes) is required.

Comment 1 Adam Tkac 2012-09-05 12:40:21 UTC
(In reply to comment #0)
> * patch /etc/bash-completion.d/git so that git-prompt.sh is sourced.  It
> might violate the Fedora go-upstream strategy, but upstream does not install
> git-prompt.sh in /etc/profile.d...

In my opinion this is the best way. git-prompt.sh can be moved to /usr/share/git and sourced into bash-completion script.

Additionally, we can patch git's bash-completion script to automatically set PS1 based on $ZSH_VERSION/$BASH_VERSION.

However before I take an action, I would like to know opinion of other git maintainers, especially about setting the PS1 variable.

Comment 2 Todd Zullinger 2012-09-06 20:43:03 UTC
While I disagree that the number of users of __git_ps1() is some small handful (it is quite likely larger than the number of people that care about keeping the output of set small), I'm fine with moving this just so we can stop having the pointless argument about "pollution" of the environment.

I don't think we should be setting PS1 automatically at all.

As far as location, what I would suggest is doing just what we do for the contributed hooks.  They are installed in /usr/share/git-core/contrib/hooks.  The contrib/completion from upstream can be copied here as well.  If so, git-completion.bash can be symlinked to the bash-completion dir perhaps.  (We wouldn't want folks filing bugs about wasting a few bytes of disk space.)

We discussed putting all of the upstream contrib here and I think there's even an open bug on that somewhere still.  I think that's a good idea, but to do so we'd need to carefully look at the scripts that are there and ensure that we don't pick up and unwanted requires or provides (or other rpmlint warnings/errors).

Comment 3 Enrico Scholz 2012-10-09 12:47:15 UTC
As a git package with this broken packaging is going into f18 soon, this issue must be fixed asap.  Else, people might start to rely on the polluted environment and a fix would introduce regressions.

Comment 4 Adam Tkac 2012-10-25 14:55:33 UTC
Created attachment 633399 [details]
Proposed patch

What do you think about this patch? It moves git-prompt.sh into datadir/git-core/contrib and then sources it into bash completion script.

Comment 5 Todd Zullinger 2012-10-25 15:40:33 UTC
Hi Adam,

I think sourcing git-prompt.sh in the completion script is a bad idea.  It defeats the reason that upstream split things out, doesn't it?

I would also suggest putting the script in /usr/share/git-core/contrib/completion, because we're very likely to install more (or all) of contrib there as time permits and we don't want to have to move the git-prompt script again and break users that have set it up.

Thanks for working on this.  Sorry I've not had time to do anything with it lately. :/

Comment 6 Adam Tkac 2012-10-29 16:01:59 UTC
(In reply to comment #5)
> Hi Adam,
> 
> I think sourcing git-prompt.sh in the completion script is a bad idea.  It
> defeats the reason that upstream split things out, doesn't it?
> 
> I would also suggest putting the script in
> /usr/share/git-core/contrib/completion, because we're very likely to install
> more (or all) of contrib there as time permits and we don't want to have to
> move the git-prompt script again and break users that have set it up.

Agreed.

I've just pushed a patch which installs git-prompt.sh into /usr/share/git-core/contrib/completion/

Comment 7 Jean-Charles Malahieude 2012-11-01 18:13:15 UTC
bash: __git_ps1: unknown command...
jcharles@localhost ~/GIT/Lily$ 


Sorry to be so dumb, but how to activate it?

I have in my .bashrc
  # affiche branche si dans un git
  export PS1="\u@\h \w\$(__git_ps1)$ "


TIA,
Jean-Charles

Comment 8 Todd Zullinger 2012-11-01 19:18:45 UTC
Something like this:

  # affiche branche si dans un git
  source /usr/share/git-core/contrib/completion/git-prompt.sh
  export PS1="\u@\h \w\$(__git_ps1)$ "

Check the path, I typed that from memory.

Comment 9 Jean-Charles Malahieude 2012-11-01 19:24:43 UTC
It works like a charm.

Your ROM is first class !

Thanks,
Jean-Charles

Comment 10 Fedora Update System 2012-11-06 10:55:14 UTC
git-1.8.0-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/git-1.8.0-1.fc18

Comment 11 Fedora Update System 2012-11-06 18:57:14 UTC
Package git-1.8.0-1.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing git-1.8.0-1.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-17708/git-1.8.0-1.fc18
then log in and leave karma (feedback).

Comment 12 Adam Williamson 2012-11-08 00:41:36 UTC
can we please stop screwing with the location after this? i'm tired of editing my bash profiles. :)

Comment 13 Adam Tkac 2012-11-08 10:20:53 UTC
(In reply to comment #12)
> can we please stop screwing with the location after this? i'm tired of
> editing my bash profiles. :)

Sorry for that, now git-prompt.sh will be in /usr/share/git-core/contrib/completion for next decade :)

Comment 14 Todd Zullinger 2012-11-08 13:25:36 UTC
I wouldn't count on it lasting that long: http://thread.gmane.org/gmane.comp.version-control.git/198249/focus=208359

That patch does leave a symlink in contrib/completion though, so as long as we're careful to ensure that we also install a symlink, nothing will break if/when upstream promotes the file that provides __git_ps1() out of contrib.

Comment 15 Adam Tkac 2012-11-22 15:52:16 UTC
This is already fixed in git-1.8.0-1.fc18


Note You need to log in before you can comment on or make changes to this bug.