git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Dan Jacques <dnj@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com, Johannes.Schindelin@gmx.de
Subject: Re: [PATCH v5 2/3] Makefile: add Perl runtime prefix support
Date: Mon, 08 Jan 2018 10:41:16 +0100	[thread overview]
Message-ID: <87inccbscj.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20180108030239.92036-3-dnj@google.com>


On Mon, Jan 08 2018, Dan Jacques jotted:

Thanks, applied this on top of next and it works for me, i.e. install to
/tmp/git and move to /tmp/git2 = works for me. Comments below.

> Enabling RUNTIME_PREFIX_PERL overrides the system-specific Perl script
> installation path generated by MakeMaker to force installation into a
> platform-neutral location, "<prefix>/share/perl5".

Not generated by MakeMaker anymore :)

From 3/3 (not not send 2 e-mails):

>+# it. This is intentionally separate from RUNTIME_PREFIX so that notably Windows
>+# can hard-code Perl library paths while still enabling RUNTIME_PREFIX
>+# resolution.

Maybe we covered this in previous submissions, but refresh my memory,
why is the *_PERL define still needed? Reading this explanation doesn't
make sense to me, but I'm probably missing something.

If we have a system where we have some perl library paths on the system
we want to use, then they'll still be in @INC after our 'use lib'-ing,
so we'll find libraries there.

The only reason I can think of for doing this for C and not for Perl
would be if you for some reason want to have a git in /tmp/git but then
use a different version of the Git.pm from some system install, but I
can't imagine why.

Or there's another option...

> +	# GIT_EXEC_PATH is supplied by `git` or the test suite. Otherwise, resolve
> +	# against the runtime path of this script.
> +	require FindBin;
> +	require File::Spec;
> +	(my $prefix = $ENV{GIT_EXEC_PATH} || $FindBin::Bin) =~ s=${gitexecdir_relative}$==;

So why are we falling back on $FindBin::Bin? Just so you can do
e.g. /tmp/git2/libexec/git-core/git-svn like you can do
/tmp/git2/libexec/git-core/git-status, i.e. will this never be false if
invoked via "git"?

I don't mind it, just wondering if I'm missing something and we need to
use the fallback path in some "normal" codepath.

> +	return File::Spec->catdir($prefix, $relpath);

I think you initially got some version of this from me (or not), so this
is probably my fault, but reading this again I think this would be
better as just:

    return $prefix . '@@PATHSEP@@' . $relpath;

I.e. right after this we split on @@PATHSEP@@, and that clearly works
(as opposed to using File::Spec->splitpath) since we've used it
forever.

Better just to use the same idiom on both ends to not leave the reader
wondering why we can split paths one way, but need to join them another
way.

  reply	other threads:[~2018-01-08  9:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-08  3:02 [PATCH v5 0/3] RUNTIME_PREFIX relocatable Git Dan Jacques
2018-01-08  3:02 ` [PATCH v5 1/3] Makefile: generate Perl header from template file Dan Jacques
2018-01-08  3:02 ` [PATCH v5 2/3] Makefile: add Perl runtime prefix support Dan Jacques
2018-01-08  9:41   ` Ævar Arnfjörð Bjarmason [this message]
2018-01-08 19:18     ` Dan Jacques
2018-01-08 20:00       ` Ævar Arnfjörð Bjarmason
2018-01-08 20:27       ` Johannes Schindelin
2018-01-08 21:54         ` Junio C Hamano
2018-01-08 22:05         ` Dan Jacques
2018-01-08 22:16           ` Ævar Arnfjörð Bjarmason
2018-01-08 22:18             ` Dan Jacques
2018-01-08 20:24     ` Johannes Schindelin
2018-01-08  3:02 ` [PATCH v5 3/3] exec_cmd: RUNTIME_PREFIX on some POSIX systems Dan Jacques

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87inccbscj.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=dnj@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).