git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* Git 2.18: RUNTIME_PREFIX... is it working?
@ 2018-07-04  5:12 Paul Smith
  2018-07-04 11:22 ` Johannes Schindelin
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Smith @ 2018-07-04  5:12 UTC (permalink / raw)
  To: Git mailing list

I was excited to see the RUNTIME_PREFIX for POSIX systems patchset go
by earlier this year.  Although I didn't see any mention of it being
included in the 2.18.0 release notes, it does appear that it was merged
in for this release.

Has anyone else tried to get it working?  It doesn't appear to be
working properly for me so I'm not sure if I'm supposed to be doing
something different... I didn't see any documentation on it.

Basically what happens is that I run configure with
--prefix=/my/install/path --with-gitconfig=etc/gitconfig
--with-gitattributes=etc/gitattributes.

Then I run make with RUNTIME_PREFIX=YesPlease.

When I look in the makefile, I see that the make variable gitexecdir is
initially properly set to libexec/git-core which is what I expect.

However, later in the makefile we include the config.mak.autogen file,
which was generated from config.mk.in by configure.  In the .in file we
have this:

 gitexecdir = @libexecdir@/git-core

After configure gets done with it, this becomes:

 gitexecdir = ${prefix}/libexec/git-core

which is a fully-qualified path.  This means that exec-cmd.c is
compiled with -DGIT_EXEC_PATH="/my/install/path/libexec/git-core" which
effectively disables RUNTIME_PREFIX, as the exec-cmd.c:system_prefix()
function always returns FALLBACK_RUNTIME_PREFIX since GIT_EXEC_PATH is
not a suffix of executable_dirname (once the install location has been
moved).

I suppose we need to pass more configure options to reset paths; is
there information somewhere on exactly which ones should be overridden?
 For example if I try to pass configure --libexecdir=libexec to solve
the above issue, I get an error from configure:

 configure: error: expected an absolute directory name for --libexecdir: libexec

Any info on how this is supposed to work, is welcome!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-04  5:12 Git 2.18: RUNTIME_PREFIX... is it working? Paul Smith
@ 2018-07-04 11:22 ` Johannes Schindelin
  2018-07-05 15:36   ` Paul Smith
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-07-04 11:22 UTC (permalink / raw)
  To: Paul Smith; +Cc: Git mailing list, Dan Jacques

Hi Paul,

On Wed, 4 Jul 2018, Paul Smith wrote:

> I was excited to see the RUNTIME_PREFIX for POSIX systems patchset go
> by earlier this year.  Although I didn't see any mention of it being
> included in the 2.18.0 release notes, it does appear that it was merged
> in for this release.
> 
> Has anyone else tried to get it working?  It doesn't appear to be
> working properly for me so I'm not sure if I'm supposed to be doing
> something different... I didn't see any documentation on it.

It is working (for ages) in the Git for Windows build.

> Basically what happens is that I run configure with
> --prefix=/my/install/path --with-gitconfig=etc/gitconfig
> --with-gitattributes=etc/gitattributes.
> 
> Then I run make with RUNTIME_PREFIX=YesPlease.

Ah. In Git for Windows, we do not use configure. I *think* this points to
an incompatibility of the RUNTIME_PREFIX feature with our autoconf
support, and this is a grand opportunity for you to step in and help.

Essentially, what you will want to do is to implement a new configure
option --with-runtime-prefix that then prevents the autoconf script from
munging the relative paths in the way it does.

> When I look in the makefile, I see that the make variable gitexecdir is
> initially properly set to libexec/git-core which is what I expect.
> 
> However, later in the makefile we include the config.mak.autogen file,
> which was generated from config.mk.in by configure.  In the .in file we
> have this:
> 
>  gitexecdir = @libexecdir@/git-core
> 
> After configure gets done with it, this becomes:
> 
>  gitexecdir = ${prefix}/libexec/git-core
> 
> which is a fully-qualified path.  This means that exec-cmd.c is
> compiled with -DGIT_EXEC_PATH="/my/install/path/libexec/git-core" which
> effectively disables RUNTIME_PREFIX, as the exec-cmd.c:system_prefix()
> function always returns FALLBACK_RUNTIME_PREFIX since GIT_EXEC_PATH is
> not a suffix of executable_dirname (once the install location has been
> moved).

Right.

I am actually quite surprised that it builds for you, given this part of
the Makefile:

-- snip --
ifdef RUNTIME_PREFIX

ifneq ($(filter /%,$(firstword $(gitexecdir_relative))),)
$(error RUNTIME_PREFIX requires a relative gitexecdir, not: $(gitexecdir))
endif

[...]
-- snap --

I am also quite surprised that config.mk.in tries to set gitexecdir. But I
guess that is for cases where you want to override it via --libexecdir?

> I suppose we need to pass more configure options to reset paths; is
> there information somewhere on exactly which ones should be overridden?
>  For example if I try to pass configure --libexecdir=libexec to solve
> the above issue, I get an error from configure:
> 
>  configure: error: expected an absolute directory name for --libexecdir: libexec
> 
> Any info on how this is supposed to work, is welcome!

I just saw another thing:

-- snip --
exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX
exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \
        '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
        '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
        '-DBINDIR="$(bindir_relative_SQ)"' \
        '-DFALLBACK_RUNTIME_PREFIX="$(prefix_SQ)"'
-- snap --

Is it possible that we should use `gitexecdir_relative_SQ` here instead?
Does that fix things for you?

I Cc:ed Dan so he could correct my hunch that this GIT_EXEC_PATH
definition needs to be fixed.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-04 11:22 ` Johannes Schindelin
@ 2018-07-05 15:36   ` Paul Smith
  2018-07-06  9:00     ` Johannes Schindelin
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Smith @ 2018-07-05 15:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git mailing list, Dan Jacques

On Wed, 2018-07-04 at 13:22 +0200, Johannes Schindelin wrote:
> > Basically what happens is that I run configure with
> > --prefix=/my/install/path --with-gitconfig=etc/gitconfig
> > --with-gitattributes=etc/gitattributes.
> > 
> > Then I run make with RUNTIME_PREFIX=YesPlease.
> 
> Ah. In Git for Windows, we do not use configure. I *think* this
> points to an incompatibility of the RUNTIME_PREFIX feature with our
> autoconf support, and this is a grand opportunity for you to step in
> and help.
> 
> Essentially, what you will want to do is to implement a new configure
> option --with-runtime-prefix that then prevents the autoconf script
> from munging the relative paths in the way it does.

FYI I was able to get this to work by overriding variables on the make
command line, like this:

  make ... RUNTIME_PREFIX=YesPlease \
      gitexecdir=libexec/git-core \
      template_dir=share/git-core/templates \
      sysconfdir=etc

I agree a new autoconf option would be much simpler to use.  I'll think
about it as I happen to have some some experience in these areas ;) ...
but time is limited of course :).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-05 15:36   ` Paul Smith
@ 2018-07-06  9:00     ` Johannes Schindelin
  2018-07-06 13:18       ` Daniel Jacques
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-07-06  9:00 UTC (permalink / raw)
  To: Paul Smith; +Cc: Git mailing list, Dan Jacques

Hi Paul,

On Thu, 5 Jul 2018, Paul Smith wrote:

> On Wed, 2018-07-04 at 13:22 +0200, Johannes Schindelin wrote:
> > > Basically what happens is that I run configure with
> > > --prefix=/my/install/path --with-gitconfig=etc/gitconfig
> > > --with-gitattributes=etc/gitattributes.
> > > 
> > > Then I run make with RUNTIME_PREFIX=YesPlease.
> > 
> > Ah. In Git for Windows, we do not use configure. I *think* this
> > points to an incompatibility of the RUNTIME_PREFIX feature with our
> > autoconf support, and this is a grand opportunity for you to step in
> > and help.
> > 
> > Essentially, what you will want to do is to implement a new configure
> > option --with-runtime-prefix that then prevents the autoconf script
> > from munging the relative paths in the way it does.
> 
> FYI I was able to get this to work by overriding variables on the make
> command line, like this:
> 
>   make ... RUNTIME_PREFIX=YesPlease \
>       gitexecdir=libexec/git-core \
>       template_dir=share/git-core/templates \
>       sysconfdir=etc
> 
> I agree a new autoconf option would be much simpler to use.  I'll think
> about it as I happen to have some some experience in these areas ;) ...

I look forward to reviewing this...

> but time is limited of course :).

Yep. Same here ;-)

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-06  9:00     ` Johannes Schindelin
@ 2018-07-06 13:18       ` Daniel Jacques
  2018-07-08 18:52         ` Paul Smith
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacques @ 2018-07-06 13:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: paul, git

Apologies for the delayed response - I've been out of town - and It
looks like Paul is already on the right track.

Johannes: I believe the GIT_EXEC_PATH snipped that you listed is not
incorrect. It's defined to "gitexecdir_SQ", and RUNTIME_PREFIX expects
(and enforces, as you snipped) that this is a relative path in
Makefile.

On non-RUNTIME_PREFIX builds, it should still be the absolute path, as
this is how Git self-locates, so using "gitexecdir_SQ" there makes
sense to me.

So:
RUNTIME_PREFIX=No, gitexecdir_SQ is absolute, GIT_EXEC_PATH is
absolute, used to find Git:
https://github.com/git/git/blob/ccdcbd54c4475c2238b310f7113ab3075b5abc9c/exec-cmd.c#L281
RUNTIME_PREFIX=YesPlease, gitexecdir_SQ is relative, GIT_EXEC_PATH is
relative and used to identify the search root of the Git installation:
https://github.com/git/git/blob/ccdcbd54c4475c2238b310f7113ab3075b5abc9c/exec-cmd.c#L40

The dual-use is confusing, and it took me a few to walk back through
how it is employed in each scenario. For clarity's sake, it may be
worth defining two variables and making one explicitly relative, but I
think it is functional as-is.

Paul: I used "config.mak" to configure RUNTIME_PREFIX when I used it
to the same effect:
https://chromium.googlesource.com/infra/infra/+/ca729b99c1f82665b634ef2ff69d93c97dfcda99/recipes/recipe_modules/third_party_packages/git.py#78

I forewent autoconf because I was concerned that the option was too
obscure and the configuration too nuanced to be worth adding via flag,
as RUNTIME_PREFIX requires some degree of path alignment and is fairly
special-case. If you prefer autoconf, though, it sounds like a good
thing to add, and I'm happy that you are finding the feature useful!

Cheers,
-Dan

On Fri, Jul 6, 2018 at 5:00 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Paul,
>
> On Thu, 5 Jul 2018, Paul Smith wrote:
>
> > On Wed, 2018-07-04 at 13:22 +0200, Johannes Schindelin wrote:
> > > > Basically what happens is that I run configure with
> > > > --prefix=/my/install/path --with-gitconfig=etc/gitconfig
> > > > --with-gitattributes=etc/gitattributes.
> > > >
> > > > Then I run make with RUNTIME_PREFIX=YesPlease.
> > >
> > > Ah. In Git for Windows, we do not use configure. I *think* this
> > > points to an incompatibility of the RUNTIME_PREFIX feature with our
> > > autoconf support, and this is a grand opportunity for you to step in
> > > and help.
> > >
> > > Essentially, what you will want to do is to implement a new configure
> > > option --with-runtime-prefix that then prevents the autoconf script
> > > from munging the relative paths in the way it does.
> >
> > FYI I was able to get this to work by overriding variables on the make
> > command line, like this:
> >
> >   make ... RUNTIME_PREFIX=YesPlease \
> >       gitexecdir=libexec/git-core \
> >       template_dir=share/git-core/templates \
> >       sysconfdir=etc
> >
> > I agree a new autoconf option would be much simpler to use.  I'll think
> > about it as I happen to have some some experience in these areas ;) ...
>
> I look forward to reviewing this...
>
> > but time is limited of course :).
>
> Yep. Same here ;-)
>
> Ciao,
> Johannes

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-06 13:18       ` Daniel Jacques
@ 2018-07-08 18:52         ` Paul Smith
  2018-07-08 21:52           ` Johannes Schindelin
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Smith @ 2018-07-08 18:52 UTC (permalink / raw)
  To: Daniel Jacques, Johannes Schindelin; +Cc: git

On Fri, 2018-07-06 at 09:18 -0400, Daniel Jacques wrote:
> I forewent autoconf because I was concerned that the option was too
> obscure and the configuration too nuanced to be worth adding via
> flag, as RUNTIME_PREFIX requires some degree of path alignment and is
> fairly special-case. If you prefer autoconf, though, it sounds like a
> good thing to add, and I'm happy that you are finding the feature
> useful!

Well, far from obscure, I actually think that RUNTIME_PREFIX should be
the default behavior on all platforms.  In fact speaking for myself, I
see no value at all in the hardcoded path behavior and it could be
removed and RUNTIME_PREFIX be the only option and that would be fine
with me.

The only possible advantage I can see to the current default that you
can copy the Git binary alone somewhere else, but that's of very little
value IMO: you could instead create a symbolic link or a two-line shell
script wrapper if you wanted to have "git" available outside of its
normal relation to the rest of the installation for some reason.

Thanks for making this work in any event, Daniel!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-08 18:52         ` Paul Smith
@ 2018-07-08 21:52           ` Johannes Schindelin
  2018-07-09 19:58             ` Jeff King
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-07-08 21:52 UTC (permalink / raw)
  To: Paul Smith; +Cc: Daniel Jacques, git

Hi Paul,

On Sun, 8 Jul 2018, Paul Smith wrote:

> On Fri, 2018-07-06 at 09:18 -0400, Daniel Jacques wrote:
> > I forewent autoconf because I was concerned that the option was too
> > obscure and the configuration too nuanced to be worth adding via
> > flag, as RUNTIME_PREFIX requires some degree of path alignment and is
> > fairly special-case. If you prefer autoconf, though, it sounds like a
> > good thing to add, and I'm happy that you are finding the feature
> > useful!
> 
> Well, far from obscure, I actually think that RUNTIME_PREFIX should be
> the default behavior on all platforms.  In fact speaking for myself, I
> see no value at all in the hardcoded path behavior and it could be
> removed and RUNTIME_PREFIX be the only option and that would be fine
> with me.
> 
> The only possible advantage I can see to the current default that you
> can copy the Git binary alone somewhere else, but that's of very little
> value IMO: you could instead create a symbolic link or a two-line shell
> script wrapper if you wanted to have "git" available outside of its
> normal relation to the rest of the installation for some reason.

In theory, I agree with you, I would love for RUNTIME_PREFIX not even to
be needed.

In practice, however, a *loooong* time ago it was decided that it was okay
to implement parts of Git as shell scripts, and when those shell scripts
finally became too many, in order not to clutter the `PATH`, they were
moved to the libexec/git-core/ directory.

Obviously, for this to work, Git needs to prefix the `PATH` variable
internally, and for that it has to know where that libexec/git-core/
directory lives.

Now, if you care to have a look at Dan's (and my) patches to implement
RUNTIME_PREFIX so that it looks for a directory *relative to the Git
binary*, you will see that it is far from portable. In fact, it is very
definitely not portable, and needs specific support for *every single
supported Operating System*. And while we covered a lot, we did not cover
all of them.

So unfortunately, it is impossible to make it the default, I am afraid.
Until the time when we can ship a single `git` binary (which is sadly
unlikely to happen, as there has been a *lot* of pushback against that
e.g. on the grounds that having to (lazy-)load the cURL library adds a
tiny bit to the startup time of the `git` binary).

It is all a bit complex, due to non-technical reasons.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-08 21:52           ` Johannes Schindelin
@ 2018-07-09 19:58             ` Jeff King
  2018-07-09 20:26               ` Johannes Schindelin
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Jeff King @ 2018-07-09 19:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Paul Smith, Daniel Jacques, git

On Sun, Jul 08, 2018 at 11:52:22PM +0200, Johannes Schindelin wrote:

> Now, if you care to have a look at Dan's (and my) patches to implement
> RUNTIME_PREFIX so that it looks for a directory *relative to the Git
> binary*, you will see that it is far from portable. In fact, it is very
> definitely not portable, and needs specific support for *every single
> supported Operating System*. And while we covered a lot, we did not cover
> all of them.
> 
> So unfortunately, it is impossible to make it the default, I am afraid.

Would it be reasonable to make RUNTIME_PREFIX the default on systems
where we _do_ have that support? AFAIK there is no downside to having it
enabled (minus a few syscalls to find the prefix, I suppose, but I
assume that's negligible).

I.e., a patch to config.mak.uname (and possibly better support for
_disabling_ it, though I think "make RUNTIME_PREFIX=" would probably
work).

-Peff

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-09 19:58             ` Jeff King
@ 2018-07-09 20:26               ` Johannes Schindelin
  2018-07-10  2:21                 ` Jeff King
  2018-07-09 21:05               ` Junio C Hamano
  2018-07-10  3:56               ` Jeff King
  2 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-07-09 20:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Paul Smith, Daniel Jacques, git

Hi Peff,

On Mon, 9 Jul 2018, Jeff King wrote:

> On Sun, Jul 08, 2018 at 11:52:22PM +0200, Johannes Schindelin wrote:
> 
> > Now, if you care to have a look at Dan's (and my) patches to implement
> > RUNTIME_PREFIX so that it looks for a directory *relative to the Git
> > binary*, you will see that it is far from portable. In fact, it is very
> > definitely not portable, and needs specific support for *every single
> > supported Operating System*. And while we covered a lot, we did not cover
> > all of them.
> > 
> > So unfortunately, it is impossible to make it the default, I am afraid.
> 
> Would it be reasonable to make RUNTIME_PREFIX the default on systems
> where we _do_ have that support? AFAIK there is no downside to having it
> enabled (minus a few syscalls to find the prefix, I suppose, but I
> assume that's negligible).
> 
> I.e., a patch to config.mak.uname (and possibly better support for
> _disabling_ it, though I think "make RUNTIME_PREFIX=" would probably
> work).

The obvious downside is that we would be a lot more likely to break one
side of the equation. At least right now, we have Git for Windows being a
prime user of RUNTIME_PREFIX (so breakages should be caught relatively
quickly), and macOS/Linux *not* being users of that feature (so breakages
in the non-RUNTIME_PREFIX code paths should be caught even quicker). By
turning on RUNTIME_PREFIX for the major platforms, the fringe platforms
are even further out on their own.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-09 19:58             ` Jeff King
  2018-07-09 20:26               ` Johannes Schindelin
@ 2018-07-09 21:05               ` Junio C Hamano
  2018-07-10  3:56               ` Jeff King
  2 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2018-07-09 21:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Paul Smith, Daniel Jacques, git

Jeff King <peff@peff.net> writes:

> On Sun, Jul 08, 2018 at 11:52:22PM +0200, Johannes Schindelin wrote:
>
>> Now, if you care to have a look at Dan's (and my) patches to implement
>> RUNTIME_PREFIX so that it looks for a directory *relative to the Git
>> binary*, you will see that it is far from portable. In fact, it is very
>> definitely not portable, and needs specific support for *every single
>> supported Operating System*. And while we covered a lot, we did not cover
>> all of them.
>> 
>> So unfortunately, it is impossible to make it the default, I am afraid.
>
> Would it be reasonable to make RUNTIME_PREFIX the default on systems
> where we _do_ have that support? AFAIK there is no downside to having it
> enabled (minus a few syscalls to find the prefix, I suppose, but I
> assume that's negligible).
>
> I.e., a patch to config.mak.uname (and possibly better support for
> _disabling_ it, though I think "make RUNTIME_PREFIX=" would probably
> work).

I think that is a sensible approach.




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-09 20:26               ` Johannes Schindelin
@ 2018-07-10  2:21                 ` Jeff King
  2018-07-10 11:40                   ` Johannes Schindelin
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2018-07-10  2:21 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Paul Smith, Daniel Jacques, git

On Mon, Jul 09, 2018 at 10:26:54PM +0200, Johannes Schindelin wrote:

> > Would it be reasonable to make RUNTIME_PREFIX the default on systems
> > where we _do_ have that support? AFAIK there is no downside to having it
> > enabled (minus a few syscalls to find the prefix, I suppose, but I
> > assume that's negligible).
> > 
> > I.e., a patch to config.mak.uname (and possibly better support for
> > _disabling_ it, though I think "make RUNTIME_PREFIX=" would probably
> > work).
> 
> The obvious downside is that we would be a lot more likely to break one
> side of the equation. At least right now, we have Git for Windows being a
> prime user of RUNTIME_PREFIX (so breakages should be caught relatively
> quickly), and macOS/Linux *not* being users of that feature (so breakages
> in the non-RUNTIME_PREFIX code paths should be caught even quicker). By
> turning on RUNTIME_PREFIX for the major platforms, the fringe platforms
> are even further out on their own.

That's true. On the other hand, we have a zillion compat features for
fringe platforms already, so there already is an expectation that people
on those platforms would need to occasionally report and fix
system-specific bugs. Perhaps thinking of it not as an feature to opt
into, but rather as a compat for "your system has not caught up to the
modern world by implementing RUNTIME_PREFIX" would encourage people on
those platforms to implement the necessary scaffolding.

I also have a gut feeling that it is much easier for static-path devs to
break RUNTIME_PREFIX folks, rather than the other way around, simply
because RUNTIME_PREFIX has a lot more moving parts. But I admit that's
just a feeling.

-Peff

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-09 19:58             ` Jeff King
  2018-07-09 20:26               ` Johannes Schindelin
  2018-07-09 21:05               ` Junio C Hamano
@ 2018-07-10  3:56               ` Jeff King
  2018-07-10  7:13                 ` perryh
  2 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2018-07-10  3:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Paul Smith, Daniel Jacques, git

On Mon, Jul 09, 2018 at 03:58:22PM -0400, Jeff King wrote:

> On Sun, Jul 08, 2018 at 11:52:22PM +0200, Johannes Schindelin wrote:
> 
> > Now, if you care to have a look at Dan's (and my) patches to implement
> > RUNTIME_PREFIX so that it looks for a directory *relative to the Git
> > binary*, you will see that it is far from portable. In fact, it is very
> > definitely not portable, and needs specific support for *every single
> > supported Operating System*. And while we covered a lot, we did not cover
> > all of them.
> > 
> > So unfortunately, it is impossible to make it the default, I am afraid.
> 
> Would it be reasonable to make RUNTIME_PREFIX the default on systems
> where we _do_ have that support? AFAIK there is no downside to having it
> enabled (minus a few syscalls to find the prefix, I suppose, but I
> assume that's negligible).

Brainstorming a little more on "what could be the possible downsides".

If I understand correctly, the Linux implementation requires reading
from /proc. So an executable that only did RUNTIME_PREFIX (with no
fallback to static paths) would be unhappy inside a chroot or other
container that didn't mount /proc.

-Peff

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10  3:56               ` Jeff King
@ 2018-07-10  7:13                 ` perryh
  2018-07-10 14:03                   ` Jeff King
  0 siblings, 1 reply; 20+ messages in thread
From: perryh @ 2018-07-10  7:13 UTC (permalink / raw)
  To: peff, Johannes.Schindelin; +Cc: git, paul, dnj

Jeff King <peff@peff.net> wrote:

> If I understand correctly, the Linux implementation requires reading
> from /proc. So an executable that only did RUNTIME_PREFIX (with no
> fallback to static paths) would be unhappy inside a chroot or other
> container that didn't mount /proc.

If we need /proc, wouldn't we _already_ be unhappy inside a chroot
that didn't mount /proc, even _with_ fallback to static paths?
Last I knew, the whole point of chroots/containers/jails/etc. was to
prevent access, from a process running inside the container, to any
part of the FS that's outside of the container.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10  2:21                 ` Jeff King
@ 2018-07-10 11:40                   ` Johannes Schindelin
  2018-07-10 14:03                     ` Daniel Jacques
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-07-10 11:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Paul Smith, Daniel Jacques, git

Hi Peff,

On Mon, 9 Jul 2018, Jeff King wrote:

> On Mon, Jul 09, 2018 at 10:26:54PM +0200, Johannes Schindelin wrote:
> 
> > > Would it be reasonable to make RUNTIME_PREFIX the default on systems
> > > where we _do_ have that support? AFAIK there is no downside to having it
> > > enabled (minus a few syscalls to find the prefix, I suppose, but I
> > > assume that's negligible).
> > > 
> > > I.e., a patch to config.mak.uname (and possibly better support for
> > > _disabling_ it, though I think "make RUNTIME_PREFIX=" would probably
> > > work).
> > 
> > The obvious downside is that we would be a lot more likely to break one
> > side of the equation. At least right now, we have Git for Windows being a
> > prime user of RUNTIME_PREFIX (so breakages should be caught relatively
> > quickly), and macOS/Linux *not* being users of that feature (so breakages
> > in the non-RUNTIME_PREFIX code paths should be caught even quicker). By
> > turning on RUNTIME_PREFIX for the major platforms, the fringe platforms
> > are even further out on their own.
> 
> That's true. On the other hand, we have a zillion compat features for
> fringe platforms already, so there already is an expectation that people
> on those platforms would need to occasionally report and fix
> system-specific bugs. Perhaps thinking of it not as an feature to opt
> into, but rather as a compat for "your system has not caught up to the
> modern world by implementing RUNTIME_PREFIX" would encourage people on
> those platforms to implement the necessary scaffolding.
> 
> I also have a gut feeling that it is much easier for static-path devs to
> break RUNTIME_PREFIX folks, rather than the other way around, simply
> because RUNTIME_PREFIX has a lot more moving parts. But I admit that's
> just a feeling.

Your gut feeling comes from a lot of experience that I trust. So I'll go
with it, too.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10 11:40                   ` Johannes Schindelin
@ 2018-07-10 14:03                     ` Daniel Jacques
  2018-07-10 16:09                       ` Junio C Hamano
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacques @ 2018-07-10 14:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Paul Smith, git

Perry Hutchison wrote:

> If we need /proc, wouldn't we _already_ be unhappy inside a chroot
> that didn't mount /proc, even _with_ fallback to static paths?
> Last I knew, the whole point of chroots/containers/jails/etc. was to
> prevent access, from a process running inside the container, to any
> part of the FS that's outside of the container.

Yep. The code also allows for use of "argv[0]", but that has its own set of
problems. These were previously covered in the discussions leading up to
the patch landing, but to summarize, "argv[0]" can be completely manipulated
by the calling process, whimsically or in response to constraints such as
links, and is wholly unreliable for self-location.

Other kernels have their own behaviors with respect to path self-location,
and none seem to be straightforward. This link seems to have a good
rundown of the details and differences:

https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe

All things considered, I think executable path self-location is markedly more
fragile than using static paths, both with increased dependencies and added
inconsistent behavior and limitations, and should not be the default
on any platform.

Both Johannes' original RUNTIME_PREFIX implementation for Windows and the
Linux/etc. expansions that I did were written to serve constrained special case
deployments. In that capacity, they can be really useful, as the fragility is
managed by their respective environments.

My particular use case served the purpose of building Git once and deploying
it via archive on other systems. This capability requires the additional work of
building portable versions of Git's dependencies and their associated
resources, and statically linking everything together.

This is a lot more portability than the conventional user requires, and also
necessitates a significantly more complex build process. However, making Git
itself portable via RUNTIME_PREFIX without similar work on its dependencies
limits the usefulness of that portability, since it's still bound to
the system's
libraries and their resources.

Cheers,
-Dan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10  7:13                 ` perryh
@ 2018-07-10 14:03                   ` Jeff King
  2018-07-10 22:20                     ` Jonathan Nieder
  2018-07-14 20:51                     ` brian m. carlson
  0 siblings, 2 replies; 20+ messages in thread
From: Jeff King @ 2018-07-10 14:03 UTC (permalink / raw)
  To: Perry Hutchison; +Cc: Jonathan Nieder, Johannes.Schindelin, git, paul, dnj

On Tue, Jul 10, 2018 at 12:13:42AM -0700, Perry Hutchison wrote:

> Jeff King <peff@peff.net> wrote:
> 
> > If I understand correctly, the Linux implementation requires reading
> > from /proc. So an executable that only did RUNTIME_PREFIX (with no
> > fallback to static paths) would be unhappy inside a chroot or other
> > container that didn't mount /proc.
> 
> If we need /proc, wouldn't we _already_ be unhappy inside a chroot
> that didn't mount /proc, even _with_ fallback to static paths?
> Last I knew, the whole point of chroots/containers/jails/etc. was to
> prevent access, from a process running inside the container, to any
> part of the FS that's outside of the container.

My point is that aside from RUNTIME_PREFIX, we don't need /proc. So
somebody who currently builds Git with a static path like
"/usr/libexec/git-core" and runs it inside a chroot will be just fine as
long as /usr/libexec/git-core is available at that name inside the
chroot. But if the build starts relying on RUNTIME_PREFIX, it's going to
regress their case.

I'm not sure how hypothetical this is. A lot of Debian tools use chroots
to build packages for specific releases by basically installing the
distro inside the chroot. I don't know whether /proc is available in
those chroots or not. If not, then I suspect builds that rely on
installing Git inside the chroot are going to break.

+cc Jonathan, who maintains the Git package for Debian, and can probably
say immediately whether I am way off base. ;)

-Peff

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10 14:03                     ` Daniel Jacques
@ 2018-07-10 16:09                       ` Junio C Hamano
  0 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2018-07-10 16:09 UTC (permalink / raw)
  To: Daniel Jacques; +Cc: Johannes Schindelin, Jeff King, Paul Smith, git

Daniel Jacques <dnj@google.com> writes:

> All things considered, I think executable path self-location is markedly more
> fragile than using static paths, both with increased dependencies and added
> inconsistent behavior and limitations, and should not be the default
> on any platform.
>
> Both Johannes' original RUNTIME_PREFIX implementation for Windows and the
> Linux/etc. expansions that I did were written to serve constrained special case
> deployments. In that capacity, they can be really useful, as the fragility is
> managed by their respective environments.

Wow, an original author of a non-trivial feature who does not push
his own creation to be used everywhere and instead cautions against
blindly using it?  

That sort of restraint is a rare trait to be commended.  Thanks for
injeting a doze of sanity.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10 14:03                   ` Jeff King
@ 2018-07-10 22:20                     ` Jonathan Nieder
  2018-07-10 22:21                       ` Jonathan Nieder
  2018-07-14 20:51                     ` brian m. carlson
  1 sibling, 1 reply; 20+ messages in thread
From: Jonathan Nieder @ 2018-07-10 22:20 UTC (permalink / raw)
  To: Jeff King; +Cc: Perry Hutchison, Johannes.Schindelin, git, paul, dnj

Hi,

Jeff King wrote:

> My point is that aside from RUNTIME_PREFIX, we don't need /proc. So
> somebody who currently builds Git with a static path like
> "/usr/libexec/git-core" and runs it inside a chroot will be just fine as
> long as /usr/libexec/git-core is available at that name inside the
> chroot. But if the build starts relying on RUNTIME_PREFIX, it's going to
> regress their case.
>
> I'm not sure how hypothetical this is. A lot of Debian tools use chroots
> to build packages for specific releases by basically installing the
> distro inside the chroot. I don't know whether /proc is available in
> those chroots or not. If not, then I suspect builds that rely on
> installing Git inside the chroot are going to break.
>
> +cc Jonathan, who maintains the Git package for Debian, and can probably
> say immediately whether I am way off base. ;)

The chroots typically have /proc.

Various libc features also count on /proc being mounted, though in
general libc tries to handle the case when it isn't mounted
gracefully.  Similarly, various features of other tools (like bash's
support for <(echo hi)) also rely on /proc.

If this is the main obstacle to enabling RUNTIME_PREFIX by default,
one option would be to make RUNTIME_PREFIX fall back to a hard-coded
path when and only when git is not able to find the path from which it
was run.  That would increase complexity, though, so I am not thrilled
with the idea.

I might try enabling RUNTIME_PREFIX in Debian experimental's git and
seeing what breaks. ;-)

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10 22:20                     ` Jonathan Nieder
@ 2018-07-10 22:21                       ` Jonathan Nieder
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Nieder @ 2018-07-10 22:21 UTC (permalink / raw)
  To: Jeff King; +Cc: Perry Hutchison, Johannes.Schindelin, git, paul, dnj

A quick correction:

Jonathan Nieder wrote:

>              Similarly, various features of other tools (like bash's
> support for <(echo hi)) also rely on /proc.

That relies on /dev/fd, not /proc, but same idea.

Tools like "ps" rely on /proc.

Sorry for the noise,
Jonathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Git 2.18: RUNTIME_PREFIX... is it working?
  2018-07-10 14:03                   ` Jeff King
  2018-07-10 22:20                     ` Jonathan Nieder
@ 2018-07-14 20:51                     ` brian m. carlson
  1 sibling, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2018-07-14 20:51 UTC (permalink / raw)
  To: Jeff King
  Cc: Perry Hutchison, Jonathan Nieder, Johannes.Schindelin, git, paul, dnj

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Tue, Jul 10, 2018 at 10:03:10AM -0400, Jeff King wrote:
> My point is that aside from RUNTIME_PREFIX, we don't need /proc. So
> somebody who currently builds Git with a static path like
> "/usr/libexec/git-core" and runs it inside a chroot will be just fine as
> long as /usr/libexec/git-core is available at that name inside the
> chroot. But if the build starts relying on RUNTIME_PREFIX, it's going to
> regress their case.

I will say that at cPanel, we have a configuration where end users can
end up inside a mount namespace without /proc (depending on the
preferences of the administrator).  However, it's easy enough for us to
simply build without RUNTIME_PREFIX if necessary.

If we turn it on by default, it would be nice if we documented (maybe in
the Makefile) that it requires /proc on Linux for the benefit of other
people who might be in a similar situation.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, back to index

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04  5:12 Git 2.18: RUNTIME_PREFIX... is it working? Paul Smith
2018-07-04 11:22 ` Johannes Schindelin
2018-07-05 15:36   ` Paul Smith
2018-07-06  9:00     ` Johannes Schindelin
2018-07-06 13:18       ` Daniel Jacques
2018-07-08 18:52         ` Paul Smith
2018-07-08 21:52           ` Johannes Schindelin
2018-07-09 19:58             ` Jeff King
2018-07-09 20:26               ` Johannes Schindelin
2018-07-10  2:21                 ` Jeff King
2018-07-10 11:40                   ` Johannes Schindelin
2018-07-10 14:03                     ` Daniel Jacques
2018-07-10 16:09                       ` Junio C Hamano
2018-07-09 21:05               ` Junio C Hamano
2018-07-10  3:56               ` Jeff King
2018-07-10  7:13                 ` perryh
2018-07-10 14:03                   ` Jeff King
2018-07-10 22:20                     ` Jonathan Nieder
2018-07-10 22:21                       ` Jonathan Nieder
2018-07-14 20:51                     ` brian m. carlson

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox