git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Dan Jacques <dnj@google.com>
To: avarab@gmail.com
Cc: Johannes.Schindelin@gmx.de, dnj@google.com, git@vger.kernel.org,
	gitster@pobox.com, pasky@ucw.cz
Subject: Re [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems
Date: Wed, 22 Nov 2017 09:27:17 -0500	[thread overview]
Message-ID: <20171122142717.15852-1-dnj@google.com> (raw)
In-Reply-To: <87efoqwm3x.fsf@evledraar.booking.com>

On Wed, 22 Nov 2017, Ævar Arnfjörð Bjarmason wrote:

> > The latter case concerns me here. If the "Config" module is loading local
> > system paths, then the relative pathing between "$Config{installsitelib}"
> > and "$Config{siteprefixexp}" may not be consistent between systems, so an
> > archive built from one system may not have a compatible relative
> > directory structure when resolved with the Config module on another
> > system.
>
> I don't see how this is different from any other option we build git
> with. When we dynamically link to e.g. PCRE under RUNTIME_PREFIX*=Yes
> you can move the installed git from /tmp/git to /tmp/git2, but it'll
> still expect the specific version of the *.so libraries to be in
> /usr/lib or whatever.

PCRE brings up a good point. When I build Git for Chromium, I am actually
statically linking in PCRE and most other runtime dependencies, so this
is the sort of portability that I am targetting. The set of configuration
options for full static linking, however, are too use-case-specific to build
into Git, so this patch set merely seeks to fundamentally enable
relocatability as an option for users to customize as they need.

> Similarly we under the default MakeMaker path add the perl version to
> the directories we have, you can move git from /tmp/git to /tmp/git2 no
> problem, since that won't change the perl version, but if you upgrade
> the global perl itself from 5.20 to 5.24 you'll need to re-build.

As you note below, the Git PERL code below, doesn't include any compiled
binary files. As far as I can tell, it's highly portable, with no real
dependencies outside of PERL 5. To that end, I very much do want to
transplant a Git distribution built on Machine-A w/ PERL 5.20 and dump it
onto Machine-B w/ PERL 5.24 and have things work.

My inclination is to follow your previous advice and export an installation
path to MakeMaker. I found that "INSTALLSITELIB" did what I wanted.

My working version exports a version-neutral installation path,
"$(prefix)/share/perl5", to MakeMaker. I can then hard-code that relative
path into the PERL header and use it for resolution, similar to what my
previous patch is doing. NO_PERL_MAKEMAKER emitted a neutral library path,
"$(prefix)/lib", and that was one of the things that made it an appealing
option in my previous patch, but prescribing the path also works fine.

At the end of the day, we're hard-coding "instlibdir" into the generated
header, so it doesn't *really* matter if we use "$(prefix)/share/perl5/5.2.4"
or "$(prefix)/share/perl5"; however, as a matter of preference, I would like
to avoid naming the build system's PERL version into the deployable bundle
when RUNTIME_PREFIX_PERL is set, so I'd opt for the latter.

> [...]
> If it's not neede dwe can just do "$p/lib".

ATM my working code does (pseudocode and filling in values):

1) $gitexecpath = "libexec/git-core";
2) $perllibpath = "share/perl5"
3) Assert that $FindBin::Bin ends with $gitexecpath (or die).
4) Strip $gitexecpath, result stored as $prefix.
5) Use File->concat($prefix, $perllibpath);

This behaves similarly to how the C code resolves things, and seems adequate
for this purpose.

> Having said the above, I don't understand why we're using MakeMaker at
> all and I think we should just get rid of it.
>
> I think the best way to proceed is probalby to just start getting rid of
> the perl/* make stuff as discussed above.
>
> Is that something you're interested in / have time for, otherwise I
> could see if I could find some time, but I don't have a lot these days.

Once thought on why MakeMaker may have value is that third-party PERL
tooling may want to import Git's PERL libraries. If this is something Git
supports, we probably need something like MakeMaker to know where the system
PERL library installation path is so it can put the Git library in the right
place in standard (non-RUNTIME_PREFIX_PERL) installations.

I agree that it shouldn't block this effort. My general judgement tends to
be that that a patch set should not sway far from a single purpose, else
it becomes hard to review and/or revert if it is problematic. Removing
MakeMaker seems like an independent cleanup task, so I'd inclined to do
that work in a separate patch set.

Having dived this deeply into things, I wouldn't mind working on this if a
block of time presents itself. Is there an issue tracker that the Git
project uses where I could add this as a backburner/cleanup task, so it's
not forgotten? Would it be appropriate to add a "TODO" comment in
"perl/Makefile"?

> [...] I could see if I could find some time, but I don't have a lot these
> days.

Thanks again for taking the time to work this out with me.
-Dan

  reply	other threads:[~2017-11-22 14:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-19 17:31 [PATCH v2 0/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems Dan Jacques
2017-11-19 17:31 ` [PATCH 1/1] " Dan Jacques
2017-11-20  1:01   ` Junio C Hamano
2017-11-20 21:00   ` Ævar Arnfjörð Bjarmason
2017-11-20 21:50     ` Ævar Arnfjörð Bjarmason
2017-11-21  2:41       ` Re " Dan Jacques
2017-11-22 12:09         ` Ævar Arnfjörð Bjarmason
2017-11-22 14:27           ` Dan Jacques [this message]
2017-11-20 21:58     ` Re(2): " Dan Jacques
2017-11-20 22:46       ` Ævar Arnfjörð Bjarmason

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=20171122142717.15852-1-dnj@google.com \
    --to=dnj@google.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pasky@ucw.cz \
    /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).