Re: [PATCH] instaweb: make the perl path configurable
On 06/12/2013 03:03 PM, Jakub Narębski wrote:
> On Wed, Jun 12, 2013 at 8:48 PM, Charles McGarvey wrote:
>> On 06/12/2013 08:00 AM, Jakub Narebski wrote:
>>>
>>> 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?
>
> I am sorry, this is my mistake. Of course if one is using perlbrew then one
> wants to use perlbrew perl (nb. how does "perlbrew switch" work?), not
> necessarily system perl hardcoded during installation time.
Perlbrew switch works by prepending the dirname of the specified perl to PATH
(and removing paths to "inactive" perls); it is implemented as a shell
function wrapper which is why it can change the current shell's environment.
So yes, searching PATH instead of using the hard-coded perl would allow the
instaweb command to use a perlbrew perl (or any other perl). I think this
would be better than not being able to use a non-system perl at all, but
a drawback I see -- and I mentioned this before, but I'll just restate it --
is that a perl module developer usually has a lot of perls installed and it
would be up to them to know whether or not the perl that is "active" for the
shell they're currently in has the instaweb dependencies. It's probably not
an unreasonable burden on the user, but it's something to consider. If the
perl path were configurable, the user could set it to a perl they know has the
dependencies and not have to deal with any uncertainty in the future whether
or not the git instaweb command would succeed in their current environment.
I think that's the benefit of the current code with hard-coded perl path: it's
more predictable. In other words, changes in the user's environment won't
change how git (instaweb) works in potentially surprising ways. I think this
is a benefit worth preserving, which is why the patch introduces a new config
variable rather than using PATH.
>> 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
>
> Hmmm... git moved out of using first "perl" in PATH...
>
>> and fall back
>> on the git.git build system-configured perl if there is no perl in PATH.
>
> How to do this?
If you wanted to use PATH, I guess you could first try to invoke perl
explicitly and let the shell search PATH implicitly, such as the following
(except it doesn't make sense to do this for mongoose, so the case would need
to be split).
@@ -110,11 +110,19 @@ start_httpd () {
case "$httpd" in
*mongoose*|*plackup*)
#These servers don't have a daemon mode so we'll have to fork it
- $full_httpd "$conf" &
+ perl "$full_httpd" "$conf" &
#Save the pid before doing anything else (we'll print it later)
pid=$!
+ ret=$?
- if test $? != 0; then
+ #If the shell couldn't find perl, try to exec the script file
directly
+ if test $ret = 127; then
+ $full_httpd "$conf" &
+ pid=$!
+ ret=$?
+ fi
+
+ if test $ret != 0; then
echo "Could not execute http daemon $httpd."
exit 1
fi
I'm also not completely certain how this would affect win32 users, though
I suspect it would work fine with the same caveats.
> [...]
>> 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.
>
> One
Re: [PATCH] instaweb: make the perl path configurable
On 06/12/2013 08:00 AM, Jakub Narebski wrote: > Charles McGarvey 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 signature.asc Description: OpenPGP digital signature
[PATCH] instaweb: make the perl path configurable
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.
Signed-off-by: Charles McGarvey
---
Documentation/config.txt | 4
git-instaweb.sh | 4 +++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..e103594 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1549,6 +1549,10 @@ instaweb.modulepath::
instead of /usr/lib/apache2/modules. Only used if httpd
is Apache.
+instaweb.perlpath::
+ The path to the perl executable used by linkgit:git-instaweb[1] to
+ run gitweb and/or verify that the HTTP daemon is running.
+
instaweb.port::
The port number to bind the gitweb httpd to. See
linkgit:git-instaweb[1].
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 01a1b05..8cfbdf2 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -3,7 +3,6 @@
# Copyright (c) 2006 Eric Wong
#
-PERL='@@PERL@@'
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
git instaweb [options] (--start | --stop | --restart)
@@ -26,9 +25,12 @@ local="$(git config --bool --get instaweb.local)"
httpd="$(git config --get instaweb.httpd)"
root="$(git config --get instaweb.gitwebdir)"
port=$(git config --get instaweb.port)
+perl_path="$(git config --get instaweb.perlpath)"
module_path="$(git config --get instaweb.modulepath)"
action="browse"
+PERL=${perl_path:-@@PERL@@}
+
conf="$GIT_DIR/gitweb/httpd.conf"
# Defaults:
--
1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: is there a fast web-interface to git for huge repos?
On 06/07/2013 01:02 PM, Constantine A. Murenin wrote: >> That's a one-time penalty. Why would that be a problem? And why is wget >> even mentioned? Did we misunderstood eachother? > > `wget` or `curl --head` would be used to trigger the caching. > > I don't understand how it's a one-time penalty. Noone wants to look > at an old copy of the repository, so, pretty much, if, say, I want to > have a gitweb of all 4 BSDs, updated daily, then, pretty much, even > with lots of ram (e.g. to eliminate the cold-case 5s penalty, and > reduce each page to 0.5s), on a quad-core box, I'd be kinda be lucky > to complete a generation of all the pages within 12h or so, obviously > using the machine at, or above, 50% capacity just for the caching. Or > several days or even a couple of weeks on an Intel Atom or VIA Nano > with 2GB of RAM or so. Obviously not acceptable, there has to be a > better solution. > > One could, I guess, only regenerate the pages which have changed, but > it still sounds like an ugly solution, where you'd have to be > generating a list of files that have changed between one gen and the > next, and you'd still have to have a very high cpu, cache and storage > requirements. Have you already ruled out caching on a proxy? Pages would only be generated on demand, so the first visitor would still experience the delay but the rest would be fast until the page expires. Even expiring pages as often as five minutes or less would probably provide significant processing savings (depending on how many users you have), and that level of staleness and the occasional delays may be acceptable to your users. As you say, generating the entire cache upfront and continuously is wasteful and probably unrealistic, but any type of caching, by definition, is going to involve users seeing stale content, and I don't see that you have any other option but some type of caching. Well, you could reproduce what git does in a bunch of distributed algorithms and run your app on a farm--which, I guess, is probably what GitHub is doing--but throwing up a caching reverse proxy is a lot quicker if you can accept the caveats. -- Charles McGarvey signature.asc Description: OpenPGP digital signature
Re: [Administrivia] On ruby and contrib/
On 06/06/2013 01:46 AM, Felipe Contreras wrote: > On Thu, Jun 6, 2013 at 2:26 AM, demerphq wrote: >> >> Good thing you are being objective and leaving out the Python 3.0 >> mess, the long legacy of backwards compatibility in the Perl >> community, the active community behind it, its extensive portability >> support, and fail to mention the lack of an equivalent to CPAN. We >> wouldn't want facts to get in the way of a personal bias would we? > > None of that has anything to do with Perl's popularity. > >> Just thought I'd push back on the FUD. People have been saying Perl is >> going away for decades... > > Perl has been going away for the last decade [1], and will continue to > go away. Perl is going away, and that an undeniable fact, and if you > are not interested in discussing on the basis of reality, I'm not > interested in discussing with you. > > [1] http://www.tiobe.com/content/paperinfo/tpci/images/tpci_trends.png The linchpin of your argument is that Perl is dying. Let's assume that the TIOBE index is a reliable basis for making business decisions--it's not, but let's pretend--the graph you linked to doesn't even seem to support your conclusion (or am I missing something?). It looks like Perl's popularity has pretty much been constant for at least two years. It's apparently not increasing in popularity, but this isn't an electrocardiogram (i.e. flat-lining is not dead or even dying). The same graph shows that Ruby's popularity also hasn't changed very much since 2007 after its initial surge. Now, it's probably too off-topic to pick apart TIOBE's methodology here, but suffice it to say that, like any trend indicator, it's only as useful as your knowledge of its limitations, and this has been discussed enough elsewhere. It's true that Perl isn't soon going to win any trendiness awards, but the same reasons that made Perl a good choice for git so many years ago are still there and then some. You would probably also be surprised at the number of new kids learning Perl. I guess I just denied the "undeniable fact" that Perl is going away, so maybe I'm one of those with whom you do not want to discuss this, but, for my part, I am willing to consider other evidence for the claim. As I pointed out, the evidence shown so far (one reference to the TIOBE index) isn't nearly enough to settle the matter. I also apologize for dragging this out if this thread is judged to not be worth a whole lot. -- Charles McGarvey signature.asc Description: OpenPGP digital signature
[PATCH] gitweb: fix problem causing erroneous project list
The bug is manifest when running gitweb in a persistent process (e.g.
FastCGI, PSGI), and it's easy to reproduce. If a gitweb request
includes the searchtext parameter (i.e. s), subsequent requests using
the project_list action--which is the default action--and without
a searchtext parameter will be filtered by the searchtext value of the
first request. This is because the value of the $search_regexp global
(the value of which is based on the searchtext parameter) is currently
being persisted between requests.
Instead, clear $search_regexp before dispatching each request.
Signed-off-by: Charles McGarvey
---
I don't think there are currently any persistent-process gitweb tests to
copy from, so writing a test for this seems to be non-trivial.
gitweb/gitweb.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 80950c0..8d69ada 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1086,7 +1086,7 @@ sub evaluate_and_validate_params {
our $search_use_regexp = $input_params{'search_use_regexp'};
our $searchtext = $input_params{'searchtext'};
- our $search_regexp;
+ our $search_regexp = undef;
if (defined $searchtext) {
if (length($searchtext) < 2) {
die_error(403, "At least two characters are required
for search parameter");
--
1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
