git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Charles McGarvey <chazmcgarvey@brokenzipper.com>
To: Jakub Narebski <jnareb@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] instaweb: make the perl path configurable
Date: Wed, 12 Jun 2013 12:48:17 -0600	[thread overview]
Message-ID: <51B8C271.9030105@brokenzipper.com> (raw)
In-Reply-To: <loom.20130612T155755-338@post.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

On 06/12/2013 08:00 AM, Jakub Narebski wrote:
> Charles McGarvey <chazmcgarvey <at> brokenzipper.com> writes:
> 
>> It is convenient for the user to be able to customize the path to perl if they
>> do not want to use the system perl.  This may be the case, for example, if the
>> user wants to use the plackup httpd but its extra dependencies are not
>> installed in the system perl; they can set the perl path to a perl that they
>> install and have control over in their own home directory.
> 
> Is it really necessary?  There is always PERL5LIB if one wants to use Perl
> modules installed in one's own home directory.  If one is using local::lib
> one has it "for free".

Yes, that's right.  Using PERL5LIB would solve the example problem in the
commit message, and it would even be pretty simple to set up using local::lib.
 So, no, this isn't strictly necessary.

> If they do not want to use system perl there is always perlbrew.

Well, perlbrew is actually what I had in mind for this patch.  Without it, it
seems like the perl path -- which is configured while building git.git so is
not easily changed by the user -- is "hard-coded" in the shebang line of the
plackup script file which is then made executable and exec'd, to start the
httpd.  Given that process, I don't see how that code allows the user to use
any other perl, or am I missing something?

If adding a new config variable seems too heavy-handed for such a trivial
problem, another approach would be to use the first perl in PATH and fall back
on the git.git build system-configured perl if there is no perl in PATH.
However, considering the nature of perlbrew, the user may have many different
perls installed and multiple terminals open with shells configured for
different perls.  To make the instaweb command search the PATH for perl would
introduce some unpredictability; the command would succeed or fail depending
on whether or not the perl being "used" in the current shell has the plackup
httpd dependencies installed.  So I think I prefer the ability to configure
which perl to have git always use for instaweb over this approach.

In short summary, this patch isn't necessary because everyone could use
local::lib to manage dependencies not installed at the system level, but I
think this patch is desirable for those of us who use perlbrew and not
local::lib.  Of course, I'm open to alternatives and other suggestions.

Thanks Jakub,

-- 
Charles McGarvey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2013-06-12 18:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 20:14 [PATCH] instaweb: make the perl path configurable Charles McGarvey
2013-06-12 14:00 ` Jakub Narebski
2013-06-12 18:48   ` Charles McGarvey [this message]
2013-06-12 21:03     ` Jakub Narębski
2013-06-12 23:13       ` Charles McGarvey

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=51B8C271.9030105@brokenzipper.com \
    --to=chazmcgarvey@brokenzipper.com \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.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.
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 NNTP newsgroup(s).