# Making perl scripts include the correct Git.pm 6 messages from 2006-07-02 to 2006-07-04. Participants: Petr Baudis, Junio C Hamano, Marco Costalba. Thread: https://gitlist.dev/t/4740 ## Petr Baudis, 2006-07-02 21:40 Subject: Making perl scripts include the correct Git.pm Message-ID: <20060702214012.GI29115@pasky.or.cz> URL: https://gitlist.dev/e/20060702214012.GI29115%40pasky.or.cz ``` Hi, the discussion of the topic became so scatterred that it's rather difficult to follow now and I get a feeling that we are kind of running in circles now, so this is my attempt to summarize it: Desired behaviour when running Git's perl scripts (ordered by degree of necessity): (D1) When running installed script, it should include Git.pm from the same installation. (D2) When running the testsuite, it should include Git.pm from the source tree. (D3) When running script directly from source tree, it should include Git.pm from the source tree. (i) The original solution passed -I on the #!/usr/bin/perl line, but that was ugly, was prone to hit various OS limits on the shebang line and violated both (D2) and (D3). (ii) My proposed second solution was to add an autogenerated line to the Git's perl scripts saying something like: use lib ('instlibdir', 'srclibdir'); This fulfills all (D1), (D2) and (D3), but is perceived by Junio as "disgusting". (iii) The currently used solution is to effectively use lib ('instlibdir'); to the Git's perl scripts. This violated (D3) and (D2) too, since use lib is the last from all the @INC modifiers to be seen and thus overrides and $PERL5LIB set in the testsuite. (iv) Variation of (iii), probably Junio's original intention when implementing it: push @INC, 'instlibdir'; This fulfills (D2). It does not fulfill (D3) per se since the user has to set $PERL5LIB manually when running Git without installing it, but it is at least fulfillable. However, most importantly this does not even fulfill (D1) since if you e.g. consider user-local installation of Git over system-wide installation of Git, local perl scripts will use the globally installed Git.pm. (v) If you throw away user-friendly (D3) requirement and insist on (iii) being disgusting, this is a newly proposed possible variation "(iv) meets (i)": #!/usr/bin/perl -Imarker @INC = map { $_ eq 'marker' ? 'instlibdir' : $_ } @INC; (I think this is more disgusting than (iii), but tastes differ. ;) So, what's the way out? PS: Is this the only remaining problem with Git.pm or do we have anything else to cope with, esp. before it gets considered to be a next material? -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ Snow falling on Perl. White noise covering line noise. Hides all the bugs too. -- J. Putnam ``` ## Junio C Hamano, 2006-07-03 00:02 Subject: Re: Making perl scripts include the correct Git.pm Message-ID: <7v64if1rop.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v64if1rop.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060702214012.GI29115@pasky.or.cz> ``` Petr Baudis writes: > so this is my attempt to summarize it: Ah, our message crossed -- thanks for summarizing it. I do not particularly like any of the solution so far, but maybe the patch I just sent out to "do the normal thing unless we are running tests" might be the right thing to do. > (ii) My proposed second solution was to add an autogenerated line to > the Git's perl scripts saying something like: > > use lib ('instlibdir', 'srclibdir'); > > This fulfills all (D1), (D2) and (D3) If you have srclibdir after instlibdir, aren't you breaking D2? And I do not think swapping them is right either. It would fullfil D1/D2/D3 but I think it has one bad side effect. Namely, I do not want an installed script, maybe coming from the vendor, to be affected by whatever unrelated garbage the end user happens to have on the same path as the one used for building the module on potentially remote machine, so I would like to avoid including srclibdir anywhere on the include path for normal execution. That is, if I built a binary package in /home/junio/git/git.git on my machine to be installed in /usr/lib/perl/somewhere, the users on a multi-user system can be tricked to run random stuff that happens to be in /home/junio/git/git.git/ directory on their machine. ``` ## Junio C Hamano, 2006-07-03 01:38 Subject: Re: Making perl scripts include the correct Git.pm Message-ID: <7vwtavxyas.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwtavxyas.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7v64if1rop.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano writes: > Petr Baudis writes: > >> so this is my attempt to summarize it: > > Ah, our message crossed -- thanks for summarizing it. I do not > particularly like any of the solution so far, but maybe the > patch I just sent out to "do the normal thing unless we are > running tests" might be the right thing to do. I see you caught Merlyn so I decided the patch I sent earlier as a tentative fix and merged the series in "next". We should be able to fix Perly problems in-tree while we have his attention ;-). Rene's merge-bases fix is also in "next". Hopefully this would make merge-recursive work by Alex and Johannes go smoother? On the other front, I applied the --remove-empty --parents fix by Linus to "master". Since this broke gitk and qgit it might warrant a 1.4.1.1 hotfix. ``` ## Petr Baudis, 2006-07-03 20:29 Subject: Re: Quick merge status updates. Message-ID: <20060703202925.GN29115@pasky.or.cz> URL: https://gitlist.dev/e/20060703202925.GN29115%40pasky.or.cz In-Reply-To: <7v64if1rop.fsf@assigned-by-dhcp.cox.net> ``` Dear diary, on Mon, Jul 03, 2006 at 02:02:30AM CEST, I got a letter where Junio C Hamano said that... > Petr Baudis writes: > > > so this is my attempt to summarize it: > > Ah, our message crossed -- thanks for summarizing it. I do not > particularly like any of the solution so far, but maybe the > patch I just sent out to "do the normal thing unless we are > running tests" might be the right thing to do. Yes, I roughly agree. I have some gripes, patches will follow. > > (ii) My proposed second solution was to add an autogenerated line to > > the Git's perl scripts saying something like: > > > > use lib ('instlibdir', 'srclibdir'); > > > > This fulfills all (D1), (D2) and (D3) > > If you have srclibdir after instlibdir, aren't you breaking D2? *Sigh* Why can't I get it right for once? ;-) > And I do not think swapping them is right either. It would > fullfil D1/D2/D3 but I think it has one bad side effect. Yes, I agree with your reservations. -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ Snow falling on Perl. White noise covering line noise. Hides all the bugs too. -- J. Putnam ``` ## Petr Baudis, 2006-07-03 21:16 Subject: [PATCH] Use $GITPERLLIB instead of $RUNNING_GIT_TESTS and centralize @INC munging Message-ID: <20060703211632.1662.46767.stgit@machine.or.cz> URL: https://gitlist.dev/e/20060703211632.1662.46767.stgit%40machine.or.cz In-Reply-To: <20060703202925.GN29115@pasky.or.cz> ``` This makes the Git perl scripts check $GITPERLLIB instead of $RUNNING_GIT_TESTS, which makes more sense if you are setting up your shell environment to use a non-installed Git instance. It also weeds out the @INC munging from the individual scripts and makes Makefile add it during the .perl files processing, so that we can change just a single place when we modify this shared logic. It looks ugly in the scripts, too. ;-) And instead of doing arcane things with the @INC array, we just do 'use lib' instead, which is essentialy the same thing anyway. I first want to do three separate patches but it turned out that it's quite a lot neater when bundled together, so I hope it's ok. Signed-off-by: Petr Baudis --- INSTALL | 4 ++-- Makefile | 4 +++- git-fmt-merge-msg.perl | 5 ----- git-mv.perl | 5 ----- t/test-lib.sh | 5 ++--- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/INSTALL b/INSTALL index ed502de..4e8f883 100644 --- a/INSTALL +++ b/INSTALL @@ -39,8 +39,8 @@ Issues of note: GIT_EXEC_PATH=`pwd` PATH=`pwd`:$PATH - PERL5LIB=`pwd`/perl/blib/lib:`pwd`/perl/blib/arch/auto/Git - export GIT_EXEC_PATH PATH PERL5LIB + GITPERLLIB=`pwd`/perl/blib/lib:`pwd`/perl/blib/arch/auto/Git + export GIT_EXEC_PATH PATH GITPERLLIB - Git is reasonably self-sufficient, but does depend on a few external programs and libraries: diff --git a/Makefile b/Makefile index a62e8a3..bae95c2 100644 --- a/Makefile +++ b/Makefile @@ -552,7 +552,9 @@ common-cmds.h: Documentation/git-*.txt $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl rm -f $@ $@+ INSTLIBDIR=`$(MAKE) -C perl -s --no-print-directory instlibdir` && \ - sed -e '1s|#!.*perl\(.*\)|#!$(PERL_PATH_SQ)\1|' \ + sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|1' \ + -e '2i\ + use lib (split(/:/, $$ENV{GITPERLLIB} || '\'"$$INSTLIBDIR"\''));' \ -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ $@.perl >$@+ diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl index a9805dd..f86231e 100755 --- a/git-fmt-merge-msg.perl +++ b/git-fmt-merge-msg.perl @@ -5,11 +5,6 @@ # # Read .git/FETCH_HEAD and make a human readable merge message # by grouping branches and tags together to form a single line. -BEGIN { - unless (exists $ENV{'RUNNING_GIT_TESTS'}) { - unshift @INC, '@@INSTLIBDIR@@'; - } -} use strict; use Git; use Error qw(:try); diff --git a/git-mv.perl b/git-mv.perl index 5134b80..322b9fd 100755 --- a/git-mv.perl +++ b/git-mv.perl @@ -6,11 +6,6 @@ # # This file is licensed under the GPL v2, or a later version # at the discretion of Linus Torvalds. -BEGIN { - unless (exists $ENV{'RUNNING_GIT_TESTS'}) { - unshift @INC, '@@INSTLIBDIR@@'; - } -} use warnings; use strict; use Getopt::Std; diff --git a/t/test-lib.sh b/t/test-lib.sh index 298c6ca..ad9796e 100755 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -206,9 +206,8 @@ PYTHON=`sed -e '1{ PYTHONPATH=$(pwd)/../compat export PYTHONPATH } -RUNNING_GIT_TESTS=YesWeAre -PERL5LIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git -export PERL5LIB RUNNING_GIT_TESTS +GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git +export GITPERLLIB test -d ../templates/blt || { error "You haven't built things yet, have you?" } ``` ## Marco Costalba, 2006-07-04 19:01 Subject: Re: Making perl scripts include the correct Git.pm Message-ID: URL: https://gitlist.dev/e/e5bfff550607041201q21d42fb5r49206540634354a6%40mail.gmail.com In-Reply-To: <7vwtavxyas.fsf@assigned-by-dhcp.cox.net> ``` On 7/3/06, Junio C Hamano wrote: > Junio C Hamano writes: > > On the other front, I applied the --remove-empty --parents fix > by Linus to "master". Since this broke gitk and qgit it might > warrant a 1.4.1.1 hotfix. > Well on the qgit side the bug is plus or less harmless, you see some spurious revisions in file annotation history and a warning message on stderr, but not a show stopper. Marco ```