From: Dan Jacques <dnj@google.com>
To: johannes.schindelin@gmx.de
Cc: avarab@gmail.com, dnj@google.com, git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v5 2/3] Makefile: add Perl runtime prefix support
Date: Mon, 8 Jan 2018 17:05:50 -0500 [thread overview]
Message-ID: <20180108220550.66256-1-dnj@google.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1801082125210.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
On 2018-01-08 20:27, Johannes Schindelin wrote:
> > 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 the reason is to accommodate Windows, I think it'd make more sense to
> change the way Git for Windows handles this, and use the same relative
> paths (if possible, that is, see the GITPERLLIB problems I mentioned
> elsewhere and which necessitated
> https://github.com/git-for-windows/git/commit/3b2f716bd8).
> (...)
> What do you think? Should we just fold the RUNTIME_PREFIX_PERL handling
> into RUNTIME_PREFIX and be done with that part?
> (...)
> As I mentioned in the mail I just finished and sent (I started it hours
> ago, but then got busy with other things while the builds were running): I
> am totally cool with changing this on Windows, too. Should simplify
> things, right?
No objections here. I see it as adding slightly more risk to this patch's
potential impact on Windows builds, but if Git-for-Windows is okay with that,
I'll go ahead and fold RUNTIME_PREFIX_PERL into RUNTIME_PREFIX for
simplicity's sake.
I'll add a "NO_RUNTIME_PREFIX_PERL" flag as per avarab@'s suggestion as a
potential mitigation if a problem does end up arising in Windows builds,
with a note that NO_RUNTIME_PREFIX_PERL can be deleted if everything seems
to be working. What do you think?
> BTW I managed to run your `runtime-prefix` branch through VSTS builds on
> Windows, macOS and Linux and they all pass the test suite. (Including the
> RUNTIME_PREFIX_PERL=YesPlease setting you added for Travis CI testing.)
Great news, thanks for doing this!
next prev parent reply other threads:[~2018-01-08 22:05 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
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 [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180108220550.66256-1-dnj@google.com \
--to=dnj@google.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.