patchInvoke git-repo-config directly.

The system have GNU git installed at /usr/bin/git. I installed git-core to ~/opt/bin. ~/opt/bin is in my PATH, but is after /usr/bin. I have set alias git="$HOME/opt/bin/git".

git-push and git-pull behaves strangely, because they call "git repo-config", which runs /usr/bin/git. Using "git-repo-config" directly fixed the problem.

Signed-off-by: Qingning Huo <qhuo@mayhq.co.uk>

---

 git-pull.sh     |    4 ++--
 git-sh-setup.sh |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

a0194fff002cb12ac58b202201d387f8ea55b225

Hi,

On Tue, 14 Mar 2006, Qingning Huo wrote:

This is unlikely to be applied; there are plans to have a "libexec" path in which all git executables are stored, and just the "git" wrapper in the path. Your patch would break git in those setups.

Ciao, Dscho

P.S.: BTW there are quite a few discussions of this in the mailing list archives...

On Tue, Mar 14, 2006 at 10:20:53PM +0100, Johannes Schindelin wrote:

I do not mind whether this patch is applied. What I want is git calls its helper programs, instead of any random git program in my PATH. If git-programs are installed to libexec path, how about calling them with absolute path?

Regards, Qingning

On Tue, 14 Mar 2006, Qingning Huo wrote:

This should not be a problem with the modern "git.c" wrapper. It _should_, if you call it with the full path, automatically prepend that path to the PATH when executing sub-commands.

So if you run git as "$HOME/opt/bin/git", the PATH _should_ be 
 - first the "PREFIX/bin" path as defined by the build
 - second the "$HOME/opt/bin/" path as defined by the fact that you ran 
   git from that path
 - finally the normal $PATH.

To check this out, do this:

	ln -s /usr/bin/printenv ~/opt/bin/git-printenv
	git printenv

and you should see the proper PATH that git ends up using internally that way.

So your problem seems to be that you do "git-pull", when you really should do "git pull" (where that wrapper will set up PATH for you). Since you don't use the wrapper, the scripts end up doing the wrong thing.

		Linus

On Tue, Mar 14, 2006 at 01:58:09PM -0800, Linus Torvalds wrote:

Thanks for your detailed explanation. Yes, "git push" and "git pull" both work fine out of the box. That is the good thing. But,

$ grep git git-pull.sh

. git-sh-setup
orig_head=$(git-rev-parse --verify HEAD) || die "Pulling into a black hole?"
git-fetch --update-head-ok "$@" || exit 1
curr_head=$(git-rev-parse --verify HEAD)
        git-read-tree -u -m "$orig_head" "$curr_head" ||
        var=`git repo-config --get pull.octopus`
        var=`git repo-config --get pull.twohead`
merge_name=$(git-fmt-merge-msg <"$GIT_DIR/FETCH_HEAD")
git-merge $no_summary $no_commit $strategy_args "$merge_name" HEAD $merge_head

We have "git-read-tree" and "git repo-config" at the same time. Are there any rules saying which form should be preferred? How about pick one form and stick to it?

If we uniformly call git helper programs/scripts with "git helper" style, would git(1) append two paths to PATH everytime it is being invoked? For example, "git pull" -> "git repo-config" would prepend ~/opt/bin four times to PATH. This wouldn't be very effecient.

Regards, Qingning

On Tue, 14 Mar 2006, Qingning Huo wrote:

I agree that it is inconsistent as-is. So a patch to make it use the "git-repo-config" form (the argument being that internally, we use the full names) might be good if just for consistency.

		Linus

On Tue, Mar 14, 2006 at 03:07:39PM -0800, Linus Torvalds wrote:

Can these two patches be accepted then?  What do others think?

Qingning

Linus Torvalds <torvalds@osdl.org> writes:

If we do the dash-form for consistency's sake, we should do PATH="`git --exec-path`:$PATH" in git-setup-sh when/before we do so. For scripts that do not use git-setup-sh they must have their own.

This would make difference if/when we switch to /usr/lib/git/exec, but until then it is not.

Junio C Hamano <junkio@cox.net> writes:

(replying to myself ...)

Probably I should add that personally my preference is to standardize on the dashless form.

On Wed, Mar 15, 2006 at 01:35:22PM -0800, Junio C Hamano wrote:

That would remove the possibility to run "git-push" in my scenario. And what is the benefit? Wasting CPU cycles to an extra execve? Duplicating git --exec-path to PATH?

Personally I recommend to always run git helper programs with absolute path, this would remove all confusions, not play with users' PATH environment, and save CPU cycles.

Qingning

On Wed, 15 Mar 2006, Junio C Hamano wrote:

Yes. That would make sense too. Then git-setup-sh would look more like what the builtin git.c does.

		Linus

Linus Torvalds <torvalds@osdl.org> writes:

I think some historical background is in order.

We started without bindir vs execdir distinction but we wanted a way to someday migrate out of putting everything in bindir. As one part of the solution, "git" wrapper was invented, and as the result of that effort, some parts of the scripts, and lot of documentation pages and sample scripts, lost dashes.

Historically, git tools have always wanted everything git-* to be found on user's PATH, and we were alarmed to see 100+ git-* commands in /usr/bin. That's why "git" wrapper and GIT_EXEC_PATH environment were invented. People can have /usr/bin/git and no other git-* on their PATH, because that "git" knows where to find the rest of git-* commands. For that to work, the scripts should know where to find the rest -- and cleanest way is to run others via "git foo" form.

Consistency via s/git-foo/git foo/g _is_ the goal, but that kind of change interferes with the other patches that do the real work, and it is kind of boring, so nobody has done wholesale clean-up of all the scripts.

Invoking with the full path is not an option -- it makes building and testing-before-installing process too cumbersome. You are welcome to try and send in a patch to do that if (and only if) you volunteer to go the whole nine yards, but I have to warn you that that approach is something we have already considered and discarded, one reason why is because it makes the testsuite unworkable (testing needs to be done before installing).

If you want to use the other "git" (GNU interactive tools, which I once heard is changing its name to gitfm or something like that -- how nice of them -- but has it ever happened?), and if you want to have /usr/bin (which has that "git") and then /other/bin (which has on-topic "git") on your PATH, in that order, that would be a problem. Saying just "git" would invoke other "git" that does not know what to do.

If you can solve that without hardcoding the full path in our scripts, that would be ideal. But otherwise, especially with changing things back to "git-foo" form without making sure going backward in that way would not hinder the migration out of /usr/bin, then that creates more problems than it solves.

So that is why I said I would prefer dashless form.

I _think_ the simplest fix is to change the order of directories you list on your PATH so that "git" is found before GNU interactive tool, which is my impression that most people seem to do (many in fact do not have GNU interactive tool on their PATH anywhere).

On Wed, Mar 15, 2006 at 03:35:36PM -0800, Junio C Hamano wrote:

Thanks for the historical background.

So we recognize the full path approach is desired, but because of technique reasons (building and testing), it is not applied. I would like to have a look of the said patch. Can you give me some pointers?

This is exactly the problem I want to solve. In this case, I alias git to use our git program.

We do not hard code the full path in the script. We can use a function gitexec() (in git-sh-setup) similiar to execv_git_cmd(). gitexec searches git-command in GIT_EXEC_PATH, and then libexedir. libexedir is hardcoded in git-sh-setup. But can be override by the environment GIT_EXEC_PATH. The make test target should set GIT_EXEC_PATH to the build directory.

The remaining job is search and replace. If we call gitexec() git(), we can even keep the "git command" form unchanged. But I prefer explicit gitexec().

I cannot uninstall /usr/bin/git. It is not under my control. I am wondering whether I should move my home dir to the front of the PATH. But a good tool should not force users to change their settings.

Qingning

Qingning Huo <qhuo@mayhq.org> writes:

That's what I found your configuration quite unusual.

When people have standard system executable directories such as /usr/bin and /bin and private directories like /home/$u/bin and /usr/local/bin on their PATH, private directories always come before the system directories, so that things in the system directories (often you do not have write permission to) can be overriden by private directories (which is under your control). I've never seen anybody sane to have system path before private ones.

Qingning Huo <qhuo@mayhq.org> writes:

I said "if you can work problems your environment has _without_ doing the full path thing, then it would be ideal".

I never said full path is desired -- I despise full path, in fact. It makes certain things very inconvenient.

There were not even a patch. I suspect people involved in the discussion realized that approach was unworkably cumbersome.

In our Makefile, we have sed script mechanism to replace tokens so we _could_ change our sources to do something like this:

        diff --git a/git-commit.sh b/git-commit.sh
        index 330a434..10835c6 100755
        --- a/git-commit.sh
        +++ b/git-commit.sh
	...
        @@ -115,7 +115,7 @@ run_status () {
                    echo '#
         # Initial commit
         #'
        -	    git-ls-files |
        +	    @@GIT_PATH@@git-ls-files |
                    sed -e '
                            s/\\/\\\\/g
                            s/ /\\ /g
        @@ -126,7 +126,7 @@ run_status () {
                    committable="$?"
                fi

        -	git-diff-files --name-status |
        +	@@GIT_PATH@@git-diff-files --name-status |
                sed -e '
                        s/\\/\\\\/g
                        s/ /\\ /g
	...

and sed it out with 's/@@GIT_PATH@@/$(gitexecdir_SQ)/g'.

However, you have to realize that I often want to try things out *without* running "make", let alone installing. The current way things are set up lets me say:

	$ GIT_EXEC_PATH=$my_git_source \
          sh -x $my_git_source/git-commit.sh

to see where things break. Changing things the way I quoted above would make things _extremely_ inconvenient for me.

And it is ugly. "Making things ugly and inconvenient for what purpose?" is the question I have to ask myself at this point.

And if the answer is "to support unusual configuration which, quite frankly, I think is broken", then...

We could probably define a shell function that looks like:

        git_exec () {
                cmd="$1"
                shift
                case "${GIT_EXEC_PATH+set}" in
                set) ;;
                *) GIT_EXEC_PATH='@@GIT_EXEC_PATH@@' ;;
                esac
                "$GIT_EXEC_PATH/git-$cmd" "$@"
        }

in git-sh-setup [*1*], and then rewrite the above to something like this instead:

        diff --git a/git-commit.sh b/git-commit.sh
        index 330a434..8a73420 100755
        --- a/git-commit.sh
        +++ b/git-commit.sh
        ...
        @@ -115,7 +115,7 @@ run_status () {
                    echo '#
         # Initial commit
         #'
        -	    git-ls-files |
        +	    git_exec ls-files |
                    sed -e '
                            s/\\/\\\\/g
                            s/ /\\ /g
        @@ -126,7 +126,7 @@ run_status () {
                    committable="$?"
                fi

        -	git-diff-files  --name-status |
        +	git_exec diff-files  --name-status |
                sed -e '
                        s/\\/\\\\/g
                        s/ /\\ /g
        ...

But that does not cover Perl nor Python scripts, and does not address the ugliness either.

[Footnote]

*1* BTW, I just noticed that git-sh-setup needs to be on user's PATH, so we probably have to inline and duplicate the git_exec() shell function definition at the beginning of each script after all, when we make the initial ". git-sh-setup" inclusion to honor GIT_EXEC_PATH without munging the user's PATH.

Which is not a big deal by itself, since we preprocess *.{sh,perl,py} files anyway, but still it leaves a _big_ ugliness factor.

Junio C Hamano <junkio@cox.net> wrote:

. ${GIT_EXEC_PATH-'@@@GIT_EXEC_PATH@@@'}/git-sh-setup

isn't too grim, and shows how the git_exec shell function can be made somewhat terser.

By the way, am I the only person who /likes/ having all the git-* programs on his path? It makes shell completion work fairly well without having to install strange completion scripts which get out of date for one thing.

-- [mdw]

Mark Wooding wrote:

But it breaks the convenience when testing.

I like it too, but I don't use it unless I can't remember what the command was named (finger-training). It shouldn't be too difficult to make git.c write its own auto-generated bash-completion rules. If someone would care to teach me the syntax I'd gladly hack up a patch for it. This is a Good Thing, since it means it would also work for the internal commands, which bash's path-completion doesn't.

Andreas Ericsson <ae@op5.se> writes:

I think that's OK. When I test things in the build directory without installing I would usually do:

	$ GIT_EXEC_PATH=`pwd` PATH=`pwd`:/usr/bin:/bin ./git-cmd-to-test

and the above would not break things.

Junio C Hamano <junkio@cox.net> wrote:

Indeed. I just translated Junio's `git_exec' shell function into a one-liner.

-- [mdw]

On Thu, 16 Mar 2006 12:53:20 +0000 (UTC) Mark Wooding <mdw@distorted.org.uk> wrote:

I like git-* for the same reason. But if git potty had aliases for long commands then git-* commands would become irrelevant. Especially "git co" would be nice. It even would be faster to type than git-ch<tab>c<tab>o<tab> ;)

Timo Hirvonen wrote:

It would indeed, and it should also be fairly trivial. However, adding short-hands that are identical with cvs and svn but does a totally different thing (well, not really different, but cvs users will be surprised) is not necessarily a good thing.

It would be better, imo, to add ambiguity detection for commands that lacks an exact match. That way "git br" and "git branch" would be identical and the logic only needs doing once. I'm not terribly excited about it though, so...

On Thu, Mar 16, 2006 at 12:26:09AM -0800, Junio C Hamano wrote:

This is similiar to what I had in mind when I recommended the full path approach. Perl or Python should be able to do the similiar. I have no comment on the ugliness. The functionality and effeciency of the program is more important to me. But I do recognize the difficulties of changing all scripts overnight.

Anyway, there are at least other two ways to solve my problem. (a) setup PATH in git-sh-setup, or (b) consistently use git-command form in scripts. Even before their implementation, I can still use "git push".

Qingning

Qingning Huo <qhuo@mayhq.org> writes:

Sorry, it just occurred to me that I was totally confused, and I apologize for giving only half of the historical background, without giving the "end of the story -- conclusion", which I did not remember until now.

What we ended up deciding after that "My /usr/bin/git is sane but /usr/bin/diff is wrong so I have /home/$u/bin/diff and /home/$u/bin is listed earlier in my $PATH" discussion, as I remember, was this:

 * We expect users to invoke things on the command line via the
   "git" wrapper.  Either having the directory that has our
   "git" in earlier on the PATH, or giving the full path to it
   as the command (your use of alias qualifies as the latter).

 * One of the important things the "git" wrapper does is to
   prefix GIT_EXEC_PATH to the $PATH environment before invoking
   the "git-foo" commands [*1*].  

 * "git-foo" command, if it is written in a scripting language,
   can safely assume "git-bar" is found on the PATH while it is
   executing.  If it is C-level and uses exec[vl]_git_cmd() to
   run the subcommand, it would also work fine (it will use the
   right GIT_EXEC_PATH).

This has a few implications:

 * By doing the above setup, we are effectively punting on the
   original issue.  First of all, it is very inconceivable that
   /usr/bin/git is sane, and /usr/bin/diff is not (git wants a
   working diff after all).  If the user is overriding
   /usr/bin/diff by having a better diff under /home/$u/bin/, he
   could have his own copy of git under /home/$u/bin/ as well
   (more likely, the person with correct privilege to installed
   /usr/bin/git would have replaced faulty /usr/bin/diff).  More
   importantly, when bindir and gitexecdir are split, gitexecdir
   will not be /usr/bin (bindir will be).  It will more likely
   to be /usr/lib/git/exec, and it would contain only git-*
   things; prefixing that directory to PATH would not mask the
   /home/$u/bin/diff or anything else non-git the user wanted to
   mask stuff from /usr/bin with, so the original issue becomes
   moot at that point.

 * Typing "git-foo" on the command line would work most of the
   time, if git is built with gitexecdir set to standard bin
   directory (the way our Makefile is shipped).  git-foo will be
   found on your PATH, and it will find "git-bar" on your PATH.
   However, when we split bindir and gitexecdir, typing
   "git-foo" on the command line without having GIT_EXEC_PATH on
   your PATH would stop working, so users have been encouraged
   to train their fingers to use dashless form ever since
   GIT_EXEC_PATH was introduced [*2*].

 * If "git-foo" invoked from the command line (without necessary
   GIT_EXEC_PATH prefixing done by the "git" wrapper) tries to
   run "git", the right git needs to be found on your PATH,
   otherwise things may not necessarily work (your setup
   violates this).  But people have been encouraged to say "git
   foo" to begin with, so this is probably not a big
   deal. Retraining people to say "git foo" instead of "git-foo"
   is a bigger issue compared to this.

 * If "git foo" is typed on the command line, underlying
   "git-foo" is run with GIT_EXEC_PATH prefixed, so it will find
   "git-bar".  If the script uses "git bar", "git" needs to be
   found either in GIT_EXEC_PATH or on your PATH, since our
   Makefile does not install "git" in gitexecdir.  Right now, we
   have bindir == gitexecdir, so "git bar" in "git-foo" script
   will work fine and this is a non-issue, but when we split
   them, it will break your setup, so we need to address this
   before that happens.

So in short, there are two real issues with your original "git-push misbehaves". One is that it broke because it was spelled with dash. I think "git push" shouldn't misbehave even with your setup, because the "git" wrapper, before it calls "git-push", should set up the PATH so that its exec-path is in front of anything you have in your original PATH (either /usr/bin or ~/opt/bin), and should find the right "git".

And the second issue is the last point in the "implications" list above. You are right, and I stand corrected. Our scripts should consistently use dash form.

One thing that bothers me is that we need to keep encouraging users to use dashless form from the command line, while we update our scripts to use dash form. What a contradicting and confusing situation X-<.

[Footnote]

*1* The "remove prefixing from git.c" patch I sent out earlier tonight is wrong. That's the crucial piece to make this whole setup work.

*2* I think I've been careful enough to use dashless form in the examples I give in my list postings, but I am not sure how successful I have been.

Junio C Hamano wrote:

One of the two reasons for rewriting the git wrapper in C was the performance penalty that came from having it as a shell-script while it was desirable from a porcelainish standpoint to use the dash-less form since we thought even then that "git" would always be in PATH while "git-foo" was to be moved to the still-imaginary GIT_EXEC_PATH.

The prepending of the GIT_EXEC_PATH to PATH was a laziness workaround for scripts that use the dashed form until we'd had time to change those, although I see from the commit-message that it's a far cry from abundantly clear (reading it now I even think it's clear I meant the other way around, although I remember I didn't). Anyways, the relative parts are these, from commit 8e49d50388211a0f3e7286f6ee600bf7736f4814

---8<---8<---8<---
     The location of the GIT_EXEC_PATH (name discussion's closed,
     thank gods) can be obtained by running

     	git --exec-path

     which will hopefully give porcelainistas ample time to adapt their
     heavy-duty loops to call the core programs directly and thus save
     the extra fork() / execve() overhead, although that's not really
     necessary any more.

     The --exec-path value is prepended to $PATH, so the git-* programs
     should Just Work without ever requiring any changes to how they call
     other programs in the suite.

     Some timing values for 10000 invocations of git-var >&/dev/null:
     	git.sh: 24.194s
     	git.c:   9.044s
     	git-var: 7.377s

---8<---8<---8<---

 From the timing values there I think the performance issues of using 
the dash-less form can just be ignored. Very rarely will a porcelainish 
wrapper do 10000 invocations of git commands where less than 2 seconds 
will be a large percentage of the overall runtime.

On Thu, 2006-03-16 at 04:14, Junio C Hamano wrote:

I think it has a philosophical rationale, though.

While we want to proved an abstraction layer to the user through the "git" wrapper, we still want to be precise internally within the scripts themselves.

Or so.

jdl

On Thu, 2006-03-16 at 13:27, Jon Loeliger wrote:

Hey!  Nice English there, Jon!

s/proved/provide

jdl

Linus Torvalds <torvalds@osdl.org> writes:

I actually think what git.c does is _wrong_.

Believe it or not, there are people with bizarre configurations just like Qingning's is bizarre in different sense [*1*]. They have diff they do not like (either broken or inadequete) installed under /usr/bin, and they install GNU diff under /home/$u/bin, and have their PATH set so that /home/$u/bin/diff is found before /usr/bin/diff (up to here, there is nothing bizarre). However, somehow they have the latest "git" installed under /usr/bin (this _is_ the bizarre part).

Earlier, we prepended GIT_EXEC_PATH to user-supplied PATH when C-level routines needed to exec git programs. This breaks that "bizarre" setup -- for such a setup, GIT_EXEC_PATH is currently set to /usr/bin and when we try to exec "diff", we ended up running /usr/bin/diff. So in order to work this around, we introduced exec[vl]_git_cmd() wrappers which use --exec-path (if the command supports per invocation override), GIT_EXEC_PATH environment, and then gitexecdir in Makefile when it was built, in this order. "git" wrapper should know about this, but the older code was never removed when we introduced exec_cmd.c.

I think we need to do something like the attached patch.

[Footnote]

*1* I first wrote "configuration even more bizarre than Qingning's", but before sending the message out I came to senses. We have to admit that that configuration is quite unusual, and I would say it is the one more bizarre than the one in question here. When people have standard system executable directories such as /usr/bin and /bin and private directories like /home/$u/bin and /usr/local/bin on their PATH, private directories always come before the system directories, so that things in the system directories (often you do not have write permission to) can be overriden by private directories (which is under your control).

-- >8 --