Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

Re: [PATCH] Do not call built-in aliases from scripts

From: Sebastian Schuberth <hidden>
Date: 2016-06-15 22:57:56

On Thu, Jun 27, 2013 at 6:11 PM, Junio C Hamano [off-list ref] wrote:
quoted
Call built-in commands via the main executable (non-dashed form) without
relying on the aliases (dashed form) to be present. On some platforms,
e.g. those that do not properly support file system links, it is
inconvenient to ship the built-in aliases, so do not depend on their
presence.
In principle I have no problem with this change, if nothing other
than for consistency reasons.  We tell users to write "git foo", and
we tell users to say PATH=$(git --exec-path):$PATH if they want to
write "git-foo", but we do tell them we prefer 'git foo' form.  We
should do so ourselves where it is reasonable.
Consistency was indeed one of my intentions, though maybe not the primary one.
I vaguely recall that some people may have argued that git-foo is
one less exec(2) when we left these in our scripted Porcelains,
though, so on a platform with poorly performing fork/exec, this
change may be seen as detrimental.  I do not know it matters that
much.
But isn't this only true for commands that are not built-ins? I.e., I
can see how calling "git-pull" from a script is more efficient than
calling "git pull" from the same script, because "git pull" would
first execute "git" which in turn would spawn a shell to run the
"git-pull" script. But calling e.g. "git-merge" and "git merge" should
use almost the same code path in the "git" executable and not
fork/exec at all because the merge command is built in, right?
Having said all that, I do have a huge issue with the justification
in the proposed log message.  If your plan is to make this:

        $(git --exec-path)/git-ls-files

not to work with your port of Git, that implementation is _broken_
and is not a Git anymore.
I would at least like to have that option, yes, well knowing that such
a port would not be considered to be a Git anymore. A typical use-case
would be a "portable" Git for Windows, which is surprisingly popular.
Such a portable version usually just ships as an archive without any
kind of installer. The ZIP archive format on Windows does not support
storing any kind of file system links, which means all executables for
built-ins would need to be added as copies to the archive. While that
does not add much to the archive size because all such files are equal
to git.exe and thus compress well, the big surprise comes when
extracting that archive as all the copies of git.exe make such a
portable installation unnecessary big. On Windows, git.exe is about
1,34 MiB in size, which sums up to about 146 MiB of extra storage
required if you have copies for all the built-ins instead of file
system links.
Git can evolve over time, and if that is really your plan, the first
step you have to take is to revive the old discussion we had after
v1.6.0:

  http://thread.gmane.org/gmane.comp.version-control.git/93511/focus=93825

and see if it is now a good idea.  It could be in year 2014.  It was
not in 2008.
Can I get around that discussion by adjusting the reasoning for the
patch to mention consistency?
I cannot apply this exact patch for an obvious and unrelated reason;
it seems to have changes, e.g. "git am" option help, that do not
have anything to do with the topic.
Well, if the topic was consistency the changes would be related, or?
As you were saying yourself, we tell users to prefer the "git foo"
form, so we should also do so in the "git am" option help, IMHO.
For a future reroll of this patch with only "git-foo -> 'git foo'",
I would appreciate an independent review by at least one set of
eyes.  It is very easy to blindly do this conversion with sed/perl
and fail to spot misconversion before sending it out.
At least the test suite (running on Linux) did not throw any failures
at me after applying this patch.

-- 
Sebastian Schuberth

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help