Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Mon, Mar 19, 2018, 04:34 Johannes Schindelin
 wrote:
>
> This is a real problem.

No it isn't.

We already handle those special cases specially, and install them in
the bin directory (as opposed to libexec). And it all works fine.

Look into the bin directory some day. You'll find things like

  git-cvsserver
  gitk
  git-receive-pack
  git-shell
  git-upload-archive
  git-upload-pack

there, and the fact that a couple of them happen to be built-ins is an
IMPLEMENTATION DETAIL, not a "Oh we should have used just 'git' for
them".

The design of having separate programs is the *good* conceptual
design. And we damn well should keep it for these things that are used
for special purposes.

The fact that two of them have become built-ins as part of the git
binary is incidental. It shouldn't be visible in the names, because it
really is just an internal implementation thing, not anything
fundamental.

> And it is our own darned fault because we let an
> implementation detail bleed into a protocol. We could have designed that a
> lot better.

And by "we" you clearly mean "not you", and by "we could have designed
that a lot better" you must mean "and it was very well designed by
competent people who didn't use bad operating systems".

> Of course we should fix this, though. There is literally no good reason

Go away.

We shouldn't fix it, it's all fine as-is, and there were tons of
f*cking good reasons for why git did what it did. The main one being
"it's a collection of scripts", which was what git _was_, for
chrissake. And using spaces and running some idiotic and
hard-to-verify script de-multiplexer is the WRONG THING for things
like "git-shell" and "git-receive-pack" and friends.

Right now you can actually verify exactly what "git-shell" does. Or
you could replace - or remove - it entirely if you don't like it. And
never have to worry about running "git" with some "shell" subcommand.

And you know that it's not an alias, for example.  Because "git-xyz"
simply does not look up aliases.

So really. Go away, Johannes. Your concerns are complete and utter BS.

The real problem is that Windows is badly designed, but since it's
easy to work around (by using hard-linking on Windows), nobody sane
cares.

The solution is simple, and was already suggested: use symlinks (like
we used to!) on non-windows systems. End of story.

And for the libexec thing, we might want to deprecate those names, if
somebody wants to, but it's not like it actually hurts, and it gives
backwards compatibility.

Btw, real Windows people know all about backwards compatibility. Ask
around competent people inside MS whether it's an important thing.

So stop this idiotic "bad design" crap. Somebody working on Windows
simply can't afford your attitude.

Somebody who didn't design it in the first place can't afford your attitude.

 Linus

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

Hi Ævar,

On Fri, 16 Mar 2018, Ævar Arnfjörð Bjarmason wrote:

> 
> On Fri, Mar 16 2018, Johannes Schindelin jotted:
> 
> > On Thu, 15 Mar 2018, Linus Torvalds wrote:
> >
> >> We do end up still using the dashed form for certain things, but they
> >> are already special-cased (ie things like "git-receive-pack" and
> >> "git-shell" that very much get executed directly, and for fundamental
> >> reasons).
> >
> > Please do elaborate.
> 
> If you were to set set "/bin/git shell" in /etc/password it would not do
> the right thing as far as I know. Is that a shell name with a space in
> it, or the "shell" argument to /bin/git?

True. And `git-shell` is not a builtin, so it does not even matter with
regards to this discussion.

> There's also the fully dashed forms of stuff like git-receive-pack is
> part of the over-ssh convention, i.e.:
> 
> ssh  git-upload-pack ...

Even if upload-pack is not a builtin (and thus still has to be its own
executable), receive-pack *is*, so this does affect our current
discussion.

This is a real problem. And it is our own darned fault because we let an
implementation detail bleed into a protocol. We could have designed that a
lot better.

Of course we should fix this, though. There is literally no good reason
that I can think of why we should not change this to `ssh  git
upload-pack ...` (of course with an insanely long deprecation period).

Ciao,
Dscho

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Ævar Arnfjörð Bjarmason]

On Fri, Mar 16 2018, Johannes Schindelin jotted:

> Hi Linus.
>
> On Thu, 15 Mar 2018, Linus Torvalds wrote:
>
>> We do end up still using the dashed form for certain things, but they
>> are already special-cased (ie things like "git-receive-pack" and
>> "git-shell" that very much get executed directly, and for fundamental
>> reasons).
>
> Please do elaborate.

If you were to set set "/bin/git shell" in /etc/password it would not do
the right thing as far as I know. Is that a shell name with a space in
it, or the "shell" argument to /bin/git?

There's also the fully dashed forms of stuff like git-receive-pack is
part of the over-ssh convention, i.e.:

ssh  git-upload-pack ...

That being said I think Linus is conflating two things here. If we still
had just the dashed forms on *nix we'd still have the issue of what it
does to shell completion, which is one thing that got brought up in the
discussion to create the "git" wrapper at the time. There were also
other reasons IIRC.

That's an entirely separate discussion from how we go about either hard-
or symlinking some stuff git is using, whether or not that's ever
directly exposed to the user.

Having said that I have a WIP re-roll which where I'm aiming to:

 * Add a NO_INSTALL_CP_FALLBACK flag, so we won't implicitly fall back
   to cp silently (unless told so)

 * Remove the 2>/dev/null we're doing on everything. That pre-dates the
   NO_*_*HARDLINKS flags and we shouldn't be doing that anymore.

 * Add an option where we optionally won't install the majority of these
   dashed forms, regardless of whether we choose hardlinks or
   symlinks. We'll still need some linking as some dashed forms we can't
   remove, as noted above.

I didn't expect Junio to merge this down to `next` so fast, so I'll wait
until INSTALL_SYMLINKS lands. As far as I know the code as-is in next
isn't buggy, I'd just like to improve it a bit more, so I'll need to
rebase what I have on top of that (which is fine).

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

Hi Linus.

On Thu, 15 Mar 2018, Linus Torvalds wrote:

> We do end up still using the dashed form for certain things, but they
> are already special-cased (ie things like "git-receive-pack" and
> "git-shell" that very much get executed directly, and for fundamental
> reasons).

Please do elaborate.

Ciao,
Johannes

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Thu, Mar 15, 2018 at 10:05 AM, Johannes Schindelin
 wrote:
> The most sensible thing, of course, would be to *not* link the builtins at
> all. I mean, we deprecated the dashed form (which was a design mistake,
> whether you admit it or not) a long time ago.

That's probably not a bad idea for the builtin commands. At least as an option.

We do end up still using the dashed form for certain things, but they
are already special-cased (ie things like "git-receive-pack" and
"git-shell" that very much get executed directly, and for fundamental
reasons).

As to it being a design mistake? No, not really. It made a lot of
sense at the time. The fact is, the problem is Windows, not git. I
know you have your hangups, but that's your problem.

   Linus

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

Hi Linus,

On Wed, 14 Mar 2018, Linus Torvalds wrote:

> On Wed, Mar 14, 2018 at 3:14 AM, Ævar Arnfjörð Bjarmason
>  wrote:
> > On Wed, Mar 14 2018, Johannes Sixt jotted:
> >>
> >> It is important to leave the default at hard-linking the binaries,
> >> because on Windows symbolic links are second class citizens (they
> >> require special privileges and there is a distinction between link
> >> targets being files or directories). Hard links work well.
> >
> > Yeah makes sense. I just want to add this as an option, and think if
> > it's proven to be un-buggy we could probably turn it on by default on
> > the *nix's if people prefer that, but yeah, we'll definitely need the
> > uname detection.
> 
> I definitely would prefer to make symlinks the default on unix.
> 
> It's what we used to do (long long ago), and as you pointed out, it's
> a lot clearer what's going on too when you don't have to look at inode
> numbers and link counts.
> 
> Forcing hardlinking everywhere by default just because Windows
> filesystems suck donkey ass through a straw is not the right thing
> either.

The most sensible thing, of course, would be to *not* link the builtins at
all. I mean, we deprecated the dashed form (which was a design mistake,
whether you admit it or not) a long time ago.

Ciao,
Johannes

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

Hi,

On Wed, 14 Mar 2018, Johannes Sixt wrote:

> Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:
> > Add a INSTALL_SYMLINKS option which if enabled, changes the default
> > hardlink installation method to one where the relevant binaries in
> > libexec/git-core are symlinked back to ../../bin, instead of being
> > hardlinked.
> > 
> > This new option also overrides the behavior of the existing
> > NO_*_HARDLINKS variables which in some cases would produce symlinks
> > within to libexec/, e.g. "git-add" symlinked to "git" which would be
> > copy of the "git" found in bin/, now "git-add" in libexec/ is always
> > going to be symlinked to the "git" found in the bin/ directory.
> 
> It is important to leave the default at hard-linking the binaries, because on
> Windows symbolic links are second class citizens (they require special
> privileges and there is a distinction between link targets being files or
> directories). Hard links work well.

To clarify: symbolic links do not exist in Windows Vista and earlier.
(There exists a concept called Junction points, but it has subtly
different semantics than symbolic links, different enough that we cannot
emulate symbolic links via Junctions).

Windows 7 and later do have symbolic links, but they require elevated
privileges to be created, as Hannes pointed out.

Since Windows 10 version 1703 (Creators Update), enabling Developer Mode
will disable this restriction and allow creating symlinks without UAC
elevation. See
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ for
details.

In Git for Windows, I originally missed the memo and forgot to add support
for the special flag, but since Git for Windows v2.13.1, users can create
symbolic links without administrators' privileges on Windows 10 (Creators
Update or later) in Developer Mode.

Of course, we still support Windows all the way back to Vista, so the
default is still: no symbolic links.

Thanks for your attention,
Dscho

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Wed, Mar 14, 2018 at 3:14 AM, Ævar Arnfjörð Bjarmason
 wrote:
> On Wed, Mar 14 2018, Johannes Sixt jotted:
>>
>> It is important to leave the default at hard-linking the binaries,
>> because on Windows symbolic links are second class citizens (they
>> require special privileges and there is a distinction between link
>> targets being files or directories). Hard links work well.
>
> Yeah makes sense. I just want to add this as an option, and think if
> it's proven to be un-buggy we could probably turn it on by default on
> the *nix's if people prefer that, but yeah, we'll definitely need the
> uname detection.

I definitely would prefer to make symlinks the default on unix.

It's what we used to do (long long ago), and as you pointed out, it's
a lot clearer what's going on too when you don't have to look at inode
numbers and link counts.

Forcing hardlinking everywhere by default just because Windows
filesystems suck donkey ass through a straw is not the right thing
either.

Linus

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 14 2018, Johannes Sixt jotted:

> Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:
>> Add a INSTALL_SYMLINKS option which if enabled, changes the default
>> hardlink installation method to one where the relevant binaries in
>> libexec/git-core are symlinked back to ../../bin, instead of being
>> hardlinked.
>>
>> This new option also overrides the behavior of the existing
>> NO_*_HARDLINKS variables which in some cases would produce symlinks
>> within to libexec/, e.g. "git-add" symlinked to "git" which would be
>> copy of the "git" found in bin/, now "git-add" in libexec/ is always
>> going to be symlinked to the "git" found in the bin/ directory.
>
> It is important to leave the default at hard-linking the binaries,
> because on Windows symbolic links are second class citizens (they
> require special privileges and there is a distinction between link
> targets being files or directories). Hard links work well.

Yeah makes sense. I just want to add this as an option, and think if
it's proven to be un-buggy we could probably turn it on by default on
the *nix's if people prefer that, but yeah, we'll definitely need the
uname detection.

>>
>> This option is being added because:
>>
>>   1) I think it makes what we're doing a lot more obvious. E.g. I'd
>>  never noticed that the libexec binaries were really just hardlinks
>>  since e.g. ls(1) won't show that in any obvious way. You need to
>>  start stat(1)-ing things and look at the inodes to see what's
>>  going on.
>>
>>   2) Some tools have very crappy support for hardlinks, e.g. the Git
>>  shipped with GitLab is much bigger than it should be because
>>  they're using a chef module that doesn't know about hardlinks, see
>>  https://github.com/chef/omnibus/issues/827
>>
>>  I've also ran into other related issues that I think are explained
>
> s/ran/run/
>
>>  by this, e.g. compiling git with debugging and rpm refusing to
>>  install a ~200MB git package with 2GB left on the FS, I think that
>>  was because it doesn't consider hardlinks, just the sum of the
>>  byte size of everything in the package.
>>
>> As for the implementation, the "../../bin" noted above will vary given
>> some given some values of "../.." and "bin" depending on the depth of
>
> s/given some//
>
>> the gitexecdir relative to the destdir, and the "bindir" target,
>> e.g. setting "bindir=/tmp/git/binaries gitexecdir=foo/bar/baz" will do
>> the right thing and produce this result:
>>
>>  $ file /tmp/git/foo/bar/baz/git-add
>>  /tmp/git/foo/bar/baz/git-add: symbolic link to ../../../binaries/git
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>   Makefile | 46 +++---
>>   1 file changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index ee0b6c8940..ac7616422d 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -329,6 +329,13 @@ all::
>>   # when hardlinking a file to another name and unlinking the original file 
>> right
>>   # away (some NTFS drivers seem to zero the contents in that scenario).
>>   #
>> +# Define INSTALL_SYMLINKS if you prefer to have everything that can be
>> +# symlinked between bin/ and libexec/ to use relative symlinks between
>> +# the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and
>
> s/ between the two//

Thanks. Will fix the above in a subsequent submission.

>> +# NO_INSTALL_HARDLINKS which will also use symlinking by indirection
>> +# within the same directory in some cases, INSTALL_SYMLINKS will
>> +# always symlink to the final target directly.
>
> "the final target"? Do you mean "the git executable installed in
> $bindir" or something like this?

I'm not explaining this well, but what I mean is that right now if you
supply NO_INSTALL_HARDLINKS you end up with this:

bin/git
libexec/git
libexec/git-add -> git

I.e. we make two copies of the "git" binary, and then just symlink
within the libexec dir, whereas with this change:

bin/git
libexec/git -> ../bin/git
libexec/git-add -> ../bin/git

I.e. we'll only install one "git" and never copy it, and to the extent
that we need symlinking we're always going to symlink directly to the
binary, i.e. not:

bin/git
libexec/git -> ../bin/git
libexec/git-add -> git

Even though that would also work, I just don't think it makes sense.

>> +#
>>   # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the 
>> installed
>>   # programs as a tar, where bin/ and libexec/ might be on different file 
>> systems.
>>   #
>> @@ -2594,35 +2601,44 @@ endif
>>  bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
>>  execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
>> +destdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 
>> 's|[^/][^/]*|..|g') && \
>>  { test "$$bindir/" = "$$execdir/" || \
>>for p in git$X $(filter 

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Sixt]

Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:

Add a INSTALL_SYMLINKS option which if enabled, changes the default
hardlink installation method to one where the relevant binaries in
libexec/git-core are symlinked back to ../../bin, instead of being
hardlinked.

This new option also overrides the behavior of the existing
NO_*_HARDLINKS variables which in some cases would produce symlinks
within to libexec/, e.g. "git-add" symlinked to "git" which would be
copy of the "git" found in bin/, now "git-add" in libexec/ is always
going to be symlinked to the "git" found in the bin/ directory.


It is important to leave the default at hard-linking the binaries, 
because on Windows symbolic links are second class citizens (they 
require special privileges and there is a distinction between link 
targets being files or directories). Hard links work well.




This option is being added because:

  1) I think it makes what we're doing a lot more obvious. E.g. I'd
 never noticed that the libexec binaries were really just hardlinks
 since e.g. ls(1) won't show that in any obvious way. You need to
 start stat(1)-ing things and look at the inodes to see what's
 going on.

  2) Some tools have very crappy support for hardlinks, e.g. the Git
 shipped with GitLab is much bigger than it should be because
 they're using a chef module that doesn't know about hardlinks, see
 https://github.com/chef/omnibus/issues/827

 I've also ran into other related issues that I think are explained


s/ran/run/


 by this, e.g. compiling git with debugging and rpm refusing to
 install a ~200MB git package with 2GB left on the FS, I think that
 was because it doesn't consider hardlinks, just the sum of the
 byte size of everything in the package.

As for the implementation, the "../../bin" noted above will vary given
some given some values of "../.." and "bin" depending on the depth of


s/given some//


the gitexecdir relative to the destdir, and the "bindir" target,
e.g. setting "bindir=/tmp/git/binaries gitexecdir=foo/bar/baz" will do
the right thing and produce this result:

 $ file /tmp/git/foo/bar/baz/git-add
 /tmp/git/foo/bar/baz/git-add: symbolic link to ../../../binaries/git

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
  Makefile | 46 +++---
  1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index ee0b6c8940..ac7616422d 100644
--- a/Makefile
+++ b/Makefile
@@ -329,6 +329,13 @@ all::
  # when hardlinking a file to another name and unlinking the original file 
right
  # away (some NTFS drivers seem to zero the contents in that scenario).
  #
+# Define INSTALL_SYMLINKS if you prefer to have everything that can be
+# symlinked between bin/ and libexec/ to use relative symlinks between
+# the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and


s/ between the two//


+# NO_INSTALL_HARDLINKS which will also use symlinking by indirection
+# within the same directory in some cases, INSTALL_SYMLINKS will
+# always symlink to the final target directly.


"the final target"? Do you mean "the git executable installed in 
$bindir" or something like this?



+#
  # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
  # programs as a tar, where bin/ and libexec/ might be on different file 
systems.
  #
@@ -2594,35 +2601,44 @@ endif
  
  	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \

execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
+   destdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 
's|[^/][^/]*|..|g') && \
{ test "$$bindir/" = "$$execdir/" || \
  for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); 
do \
$(RM) "$$execdir/$$p" && \
-   test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" 
&& \
-   ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
-   cp "$$bindir/$$p" "$$execdir/$$p" || exit; \
+   test -n "$(INSTALL_SYMLINKS)" && \
+   ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p" 
"$$execdir/$$p" || \
+   { test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" 
&& \
+ ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
+ cp "$$bindir/$$p" "$$execdir/$$p" || exit; } \


I think that it is unnecessary to place the later options in {} brackets 
because && and || have equal precedence in shell scripts. That is:


want symlinks? &&
make symlinks ||
want hard links? &&
make hard links ||
make copies ||
exit

Of course, it means that when symlinking fails, it falls back to hard 
links (if permitted) or copies, whichever works. But that also happens 
with your version.


(Ditto in the rest of the hunk, which I don't repeat here.)

-- Hannes