Re: [PATCH] Extend runtime prefix computation

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 18, 2016, 07:20 UTC
Message-ID: <alpine.DEB.2.20.1604180918500.2967@virtualbox>
In-Reply-To: <xmqqfuumddqm.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 15 Apr 2016, Junio C Hamano wrote:

Show 12 quoted lines

> Michael Weiser <michael.weiser@gmx.de> writes:
> 
> > Make git fully relocatable at runtime extending the runtime prefix
> > calculation. Handle absolute and relative paths in argv0. Handle no path
> > at all in argv0 in a system-specific manner.  Replace assertions with
> > initialised variables and checks that lead to fallback to the static
> > prefix.
> 
> That's a dense description of "what" without saying much about
> "why".  Hint: start by describing what case(s) the current code
> fails to find the correct runtime prefix.  That would give readers a
> better understanding of what problem you are trying to solve.

I have to admit that I am really, *really* skeptical. To me, it looks like this patch opens the door very wide to unintended consequences.

Show 7 quoted lines

> >  #ifdef RUNTIME_PREFIX
> > -	assert(argv0_path);
> > -	assert(is_absolute_path(argv0_path));
> 
> Aren't these protecting against future and careless change that
> forgets to call extract-argv0-path or make that function return
> something that is not an absolute path?

This (first) assert() indeed saved me a couple of times from hunting for bugs in the wrong place. Let's keep it.

Ciao, Dscho