Re: [PATCH v6 2/3] Makefile: add Perl runtime prefix support

From: Daniel Jacques <dnj@google.com>
Date: Mar 19, 2018, 17:21 UTC
Message-ID: <CAD1RUU9R5xfGghQP-A=yU0RkmVDaS0hPGMHVieCyb-DRg97SnQ@mail.gmail.com>
In-Reply-To: <xmqqbmfkvvhc.fsf@gitster-ct.c.googlers.com>

On Mon, Mar 19, 2018 at 1:14 PM Junio C Hamano <gitster@pobox.com> wrote:

> > +# RUNTIME_PREFIX's resolution logic requires resource paths to be

expressed

> > +# relative to each other and share an installation path.
> > +#
> > +# This is a dependnecy in:

> dependency?

Oops, this is the second typo that has been pointed out. I'll release one last series after a small review period with these fixed.

Show 7 quoted lines

> > +# - Git's binary RUNTIME_PREFIX logic in (see "exec_cmd.c").
> > +# - The runtime prefix Perl header (see
> > +#   "perl/header_templates/runtime_prefix.template.pl").
> > +ifdef RUNTIME_PREFIX
> > +
> > +ifneq ($(filter /%,$(firstword $(gitexecdir_relative))),)
> > +$(error RUNTIME_PREFIX requires a relative gitexecdir, not:

$(gitexecdir))

> > +endif

Show 5 quoted lines

> I see Dscho is CC'ed so I won't worry about "is there a more
> portable test than 'the path begins with a slash' to see if a path
> is relative, or is this good enough even for Windows in the context
> of this patch?".  It won't be a show-stopper issue as long as we do
> not error out with false positive, though ;-).

OK sounds good! There are other places in the Makefile that use this method for this purpose, so hopefully the worst-case is that this is no more broken than they are.