{"thread":{"id":"4740","subject":"Making perl scripts include the correct Git.pm","startedAt":"2006-07-02T21:40:13Z","lastAt":"2006-07-04T19:01:53Z","messageCount":6,"participants":["Petr Baudis","Junio C Hamano","Marco Costalba"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23053","messageId":"20060702214012.GI29115@pasky.or.cz","threadId":"4740","inReplyTo":null,"subject":"Making perl scripts include the correct Git.pm","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-07-02T21:40:13Z","receivedAt":"2006-07-02T21:40:13Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":" Hi,\n\n the discussion of the topic became so scatterred that it's rather\ndifficult to follow now and I get a feeling that we are kind of running\nin circles now, so this is my attempt to summarize it:\n\n Desired behaviour when running Git's perl scripts (ordered by\ndegree of necessity):\n\n (D1) When running installed script, it should include Git.pm from the\nsame installation.\n\n (D2) When running the testsuite, it should include Git.pm from the\nsource tree.\n\n (D3) When running script directly from source tree, it should include\nGit.pm from the source tree.\n\n\n (i) The original solution passed -I on the #!/usr/bin/perl line, but\nthat was ugly, was prone to hit various OS limits on the shebang line\nand violated both (D2) and (D3).\n\n\n (ii) My proposed second solution was to add an autogenerated line to\nthe Git's perl scripts saying something like:\n\n\tuse lib ('instlibdir', 'srclibdir');\n\nThis fulfills all (D1), (D2) and (D3), but is perceived by Junio as\n\"disgusting\".\n\n\n (iii) The currently used solution is to effectively\n\n\tuse lib ('instlibdir');\n\nto the Git's perl scripts. This violated (D3) and (D2) too, since\nuse lib is the last from all the @INC modifiers to be seen and thus\noverrides and $PERL5LIB set in the testsuite.\n\n\n (iv) Variation of (iii), probably Junio's original intention when\nimplementing it:\n\n\tpush @INC, 'instlibdir';\n\nThis fulfills (D2). It does not fulfill (D3) per se since the user\nhas to set $PERL5LIB manually when running Git without installing it,\nbut it is at least fulfillable. However, most importantly this does\nnot even fulfill (D1) since if you e.g. consider user-local installation\nof Git over system-wide installation of Git, local perl scripts will\nuse the globally installed Git.pm.\n\n\n (v) If you throw away user-friendly (D3) requirement and insist\non (iii) being disgusting, this is a newly proposed possible variation\n\"(iv) meets (i)\":\n\n\t#!/usr/bin/perl -Imarker\n\t@INC = map { $_ eq 'marker' ? 'instlibdir' : $_ } @INC;\n\n(I think this is more disgusting than (iii), but tastes differ. ;)\n\n\n So, what's the way out?\n\n\n PS: Is this the only remaining problem with Git.pm or do we have\nanything else to cope with, esp. before it gets considered to be a\nnext material?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nSnow falling on Perl. White noise covering line noise.\nHides all the bugs too. -- J. Putnam\n"},{"id":"23068","messageId":"7v64if1rop.fsf@assigned-by-dhcp.cox.net","threadId":"4740","inReplyTo":"20060702214012.GI29115@pasky.or.cz","subject":"Re: Making perl scripts include the correct Git.pm","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-03T00:02:30Z","receivedAt":"2006-07-03T00:02:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> so this is my attempt to summarize it:\n\nAh, our message crossed -- thanks for summarizing it. I do not\nparticularly like any of the solution so far, but maybe the\npatch I just sent out to \"do the normal thing unless we are\nrunning tests\" might be the right thing to do.\n\n> (ii) My proposed second solution was to add an autogenerated line to\n> the Git's perl scripts saying something like:\n>\n> \tuse lib ('instlibdir', 'srclibdir');\n>\n> This fulfills all (D1), (D2) and (D3)\n\nIf you have srclibdir after instlibdir, aren't you breaking D2?\n\nAnd I do not think swapping them is right either. It would\nfullfil D1/D2/D3 but I think it has one bad side effect.\n\nNamely, I do not want an installed script, maybe coming from the\nvendor, to be affected by whatever unrelated garbage the end\nuser happens to have on the same path as the one used for\nbuilding the module on potentially remote machine, so I would\nlike to avoid including srclibdir anywhere on the include path\nfor normal execution.\n\nThat is, if I built a binary package in /home/junio/git/git.git\non my machine to be installed in /usr/lib/perl/somewhere, the\nusers on a multi-user system can be tricked to run random stuff\nthat happens to be in /home/junio/git/git.git/ directory on\ntheir machine.\n"},{"id":"23077","messageId":"7vwtavxyas.fsf@assigned-by-dhcp.cox.net","threadId":"4740","inReplyTo":"7v64if1rop.fsf@assigned-by-dhcp.cox.net","subject":"Re: Making perl scripts include the correct Git.pm","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-03T01:38:35Z","receivedAt":"2006-07-03T01:38:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Petr Baudis writes:\n>\n>> so this is my attempt to summarize it:\n>\n> Ah, our message crossed -- thanks for summarizing it. I do not\n> particularly like any of the solution so far, but maybe the\n> patch I just sent out to \"do the normal thing unless we are\n> running tests\" might be the right thing to do.\n\nI see you caught Merlyn so I decided the patch I sent earlier as\na tentative fix and merged the series in \"next\". We should be\nable to fix Perly problems in-tree while we have his attention\n;-).\n\nRene's merge-bases fix is also in \"next\". Hopefully this would\nmake merge-recursive work by Alex and Johannes go smoother?\n\nOn the other front, I applied the --remove-empty --parents fix\nby Linus to \"master\". Since this broke gitk and qgit it might\nwarrant a 1.4.1.1 hotfix.\n"},{"id":"23129","messageId":"20060703202925.GN29115@pasky.or.cz","threadId":"4740","inReplyTo":"7v64if1rop.fsf@assigned-by-dhcp.cox.net","subject":"Re: Quick merge status updates.","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-07-03T20:29:25Z","receivedAt":"2006-07-03T20:29:25Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Mon, Jul 03, 2006 at 02:02:30AM CEST, I got a letter\nwhere Junio C Hamano said that...\n> Petr Baudis writes:\n> \n> > so this is my attempt to summarize it:\n> \n> Ah, our message crossed -- thanks for summarizing it. I do not\n> particularly like any of the solution so far, but maybe the\n> patch I just sent out to \"do the normal thing unless we are\n> running tests\" might be the right thing to do.\n\nYes, I roughly agree. I have some gripes, patches will follow.\n\n> > (ii) My proposed second solution was to add an autogenerated line to\n> > the Git's perl scripts saying something like:\n> >\n> > \tuse lib ('instlibdir', 'srclibdir');\n> >\n> > This fulfills all (D1), (D2) and (D3)\n> \n> If you have srclibdir after instlibdir, aren't you breaking D2?\n\n*Sigh*\n\nWhy can't I get it right for once? ;-)\n\n> And I do not think swapping them is right either. It would\n> fullfil D1/D2/D3 but I think it has one bad side effect.\n\nYes, I agree with your reservations.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nSnow falling on Perl. White noise covering line noise.\nHides all the bugs too. -- J. Putnam\n"},{"id":"23142","messageId":"20060703211632.1662.46767.stgit@machine.or.cz","threadId":"4740","inReplyTo":"20060703202925.GN29115@pasky.or.cz","subject":"[PATCH] Use $GITPERLLIB instead of $RUNNING_GIT_TESTS and centralize @INC munging","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-07-03T21:16:32Z","receivedAt":"2006-07-03T21:16:32Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This makes the Git perl scripts check $GITPERLLIB instead of\n$RUNNING_GIT_TESTS, which makes more sense if you are setting up your shell\nenvironment to use a non-installed Git instance.\n\nIt also weeds out the @INC munging from the individual scripts and makes\nMakefile add it during the .perl files processing, so that we can change\njust a single place when we modify this shared logic. It looks ugly in the\nscripts, too. ;-)\n\nAnd instead of doing arcane things with the @INC array, we just do 'use lib'\ninstead, which is essentialy the same thing anyway.\n\nI first want to do three separate patches but it turned out that it's quite\na lot neater when bundled together, so I hope it's ok.\n\nSigned-off-by: Petr Baudis \n---\n\n INSTALL | 4 ++--\n Makefile | 4 +++-\n git-fmt-merge-msg.perl | 5 -----\n git-mv.perl | 5 -----\n t/test-lib.sh | 5 ++---\n 5 files changed, 7 insertions(+), 16 deletions(-)\n\ndiff --git a/INSTALL b/INSTALL\nindex ed502de..4e8f883 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -39,8 +39,8 @@ Issues of note:\n \n \tGIT_EXEC_PATH=`pwd`\n \tPATH=`pwd`:$PATH\n-\tPERL5LIB=`pwd`/perl/blib/lib:`pwd`/perl/blib/arch/auto/Git\n-\texport GIT_EXEC_PATH PATH PERL5LIB\n+\tGITPERLLIB=`pwd`/perl/blib/lib:`pwd`/perl/blib/arch/auto/Git\n+\texport GIT_EXEC_PATH PATH GITPERLLIB\n \n - Git is reasonably self-sufficient, but does depend on a few external\n programs and libraries:\ndiff --git a/Makefile b/Makefile\nindex a62e8a3..bae95c2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -552,7 +552,9 @@ common-cmds.h: Documentation/git-*.txt\n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n \trm -f $@ $@+\n \tINSTLIBDIR=`$(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n-\tsed -e '1s|#!.*perl\\(.*\\)|#!$(PERL_PATH_SQ)\\1|' \\\n+\tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|1' \\\n+\t -e '2i\\\n+\t use lib (split(/:/, $$ENV{GITPERLLIB} || '\\'\"$$INSTLIBDIR\"\\''));' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n \t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t $@.perl >$@+\ndiff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl\nindex a9805dd..f86231e 100755\n--- a/git-fmt-merge-msg.perl\n+++ b/git-fmt-merge-msg.perl\n@@ -5,11 +5,6 @@ #\n # Read .git/FETCH_HEAD and make a human readable merge message\n # by grouping branches and tags together to form a single line.\n \n-BEGIN {\n-\tunless (exists $ENV{'RUNNING_GIT_TESTS'}) {\n-\t\tunshift @INC, '@@INSTLIBDIR@@';\n-\t}\n-}\n use strict;\n use Git;\n use Error qw(:try);\ndiff --git a/git-mv.perl b/git-mv.perl\nindex 5134b80..322b9fd 100755\n--- a/git-mv.perl\n+++ b/git-mv.perl\n@@ -6,11 +6,6 @@ #\n # This file is licensed under the GPL v2, or a later version\n # at the discretion of Linus Torvalds.\n \n-BEGIN {\n-\tunless (exists $ENV{'RUNNING_GIT_TESTS'}) {\n-\t\tunshift @INC, '@@INSTLIBDIR@@';\n-\t}\n-}\n use warnings;\n use strict;\n use Getopt::Std;\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 298c6ca..ad9796e 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -206,9 +206,8 @@ PYTHON=`sed -e '1{\n \tPYTHONPATH=$(pwd)/../compat\n \texport PYTHONPATH\n }\n-RUNNING_GIT_TESTS=YesWeAre\n-PERL5LIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n-export PERL5LIB RUNNING_GIT_TESTS\n+GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n+export GITPERLLIB\n test -d ../templates/blt || {\n \terror \"You haven't built things yet, have you?\"\n }\n"},{"id":"23236","messageId":"e5bfff550607041201q21d42fb5r49206540634354a6@mail.gmail.com","threadId":"4740","inReplyTo":"7vwtavxyas.fsf@assigned-by-dhcp.cox.net","subject":"Re: Making perl scripts include the correct Git.pm","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-07-04T19:01:53Z","receivedAt":"2006-07-04T19:01:53Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 7/3/06, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> On the other front, I applied the --remove-empty --parents fix\n> by Linus to \"master\". Since this broke gitk and qgit it might\n> warrant a 1.4.1.1 hotfix.\n>\n\nWell on the qgit side the bug is plus or less harmless, you see some\nspurious revisions in file annotation history and a warning message on\nstderr, but not a show stopper.\n\n Marco\n"}]}