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

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

 



On Thu, Jun 27, 2013 at 6:11 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:

>> 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
--
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]