{"thread":{"id":"5665","subject":"[PATCH] Git.pm: Kill Git.xs for now","startedAt":"2006-09-23T18:20:47Z","lastAt":"2006-09-24T12:09:02Z","messageCount":7,"participants":["Petr Baudis","Junio C Hamano","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"27496","messageId":"20060923181849.28753.56984.stgit@machine.or.cz","threadId":"5665","inReplyTo":null,"subject":"[PATCH] Git.pm: Kill Git.xs for now","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-23T18:20:47Z","receivedAt":"2006-09-23T18:20:47Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch removes Git.xs from the repository for the time being. This\nshould hopefully enable Git.pm to finally make its way to master.\n\nGit.xs is not going away forever. When the Git libification makes some\nprogress, it will hopefully return (but most likely as an optional\ncomponent, due to the portability woes) since the performance boosts are\nreally important for applications like Gitweb or Cogito. It needs to go\naway now since it is not really reliable in case you use it for several\nrepositories in the scope of a single process, and that is not possible\nto fix without some either very ugly or very intrusive core changes.\n\nRest in peace. (While you can.)\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n Makefile         |   17 ++-----\n perl/.gitignore  |    3 -\n perl/Git.pm      |   76 ++++---------------------------\n perl/Git.xs      |  134 ------------------------------------------------------\n perl/Makefile.PL |    9 ----\n 5 files changed, 15 insertions(+), 224 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex dd74908..58848e4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -116,8 +116,6 @@ PIC_FLAG = -fPIC\n LDFLAGS =\n ALL_CFLAGS = $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n-PERL_CFLAGS =\n-PERL_LDFLAGS =\n STRIP ?= strip\n \n prefix = $(HOME)\n@@ -158,9 +156,10 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powe\n ### --- END CONFIGURATION SECTION ---\n \n # Those must not be GNU-specific; they are shared with perl/ which may\n-# be built by a different compiler.\n-BASIC_CFLAGS = $(PERL_CFLAGS)\n-BASIC_LDFLAGS = $(PERL_LDFLAGS)\n+# be built by a different compiler. (Note that this is an artifact now\n+# but it still might be nice to keep that distinction.)\n+BASIC_CFLAGS = \n+BASIC_LDFLAGS = \n \n SCRIPT_SH = \\\n \tgit-bisect.sh git-branch.sh git-checkout.sh \\\n@@ -784,15 +783,9 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare\n \trm -f $@ && $(AR) rcs $@ $(XDIFF_OBJS)\n \n \n-PERL_DEFINE = $(BASIC_CFLAGS) -DGIT_VERSION='\"$(GIT_VERSION)\"'\n-PERL_DEFINE_SQ = $(subst ','\\'',$(PERL_DEFINE))\n-PERL_LIBS = $(BASIC_LDFLAGS) $(EXTLIBS)\n-PERL_LIBS_SQ = $(subst ','\\'',$(PERL_LIBS))\n perl/Makefile: perl/Git.pm perl/Makefile.PL GIT-CFLAGS\n \t(cd perl && $(PERL_PATH) Makefile.PL \\\n-\t\tPREFIX='$(prefix_SQ)' \\\n-\t\tDEFINE='$(PERL_DEFINE_SQ)' \\\n-\t\tLIBS='$(PERL_LIBS_SQ)')\n+\t\tPREFIX='$(prefix_SQ)')\n \n doc:\n \t$(MAKE) -C Documentation all\ndiff --git a/perl/.gitignore b/perl/.gitignore\nindex 6d778f3..e990cae 100644\n--- a/perl/.gitignore\n+++ b/perl/.gitignore\n@@ -1,7 +1,4 @@\n-Git.bs\n-Git.c\n Makefile\n blib\n blibdirs\n pm_to_blib\n-ppport.h\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 9ce9fcd..2b26b65 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -93,9 +93,6 @@ use Carp qw(carp croak); # but croak is \n use Error qw(:try);\n use Cwd qw(abs_path);\n \n-require XSLoader;\n-XSLoader::load('Git', $VERSION);\n-\n }\n \n \n@@ -413,12 +410,13 @@ sub command_noisy {\n \n Return the Git version in use.\n \n-Implementation of this function is very fast; no external command calls\n-are involved.\n-\n =cut\n \n-# Implemented in Git.xs.\n+sub version {\n+\tmy $verstr = command_oneline('--version');\n+\t$verstr =~ s/^git version //;\n+\t$verstr;\n+}\n \n \n =item exec_path ()\n@@ -426,12 +424,9 @@ # Implemented in Git.xs.\n Return path to the Git sub-command executables (the same as\n C<git --exec-path>). Useful mostly only internally.\n \n-Implementation of this function is very fast; no external command calls\n-are involved.\n-\n =cut\n \n-# Implemented in Git.xs.\n+sub exec_path { command_oneline('--exec-path') }\n \n \n =item repo_path ()\n@@ -572,41 +567,21 @@ sub ident_person {\n \n =item hash_object ( TYPE, FILENAME )\n \n-=item hash_object ( TYPE, FILEHANDLE )\n-\n Compute the SHA1 object id of the given C<FILENAME> (or data waiting in\n C<FILEHANDLE>) considering it is of the C<TYPE> object type (C<blob>,\n C<commit>, C<tree>).\n \n-In case of C<FILEHANDLE> passed instead of file name, all the data\n-available are read and hashed, and the filehandle is automatically\n-closed. The file handle should be freshly opened - if you have already\n-read anything from the file handle, the results are undefined (since\n-this function works directly with the file descriptor and internal\n-PerlIO buffering might have messed things up).\n-\n The method can be called without any instance or on a specified Git repository,\n it makes zero difference.\n \n The function returns the SHA1 hash.\n \n-Implementation of this function is very fast; no external command calls\n-are involved.\n-\n =cut\n \n+# TODO: Support for passing FILEHANDLE instead of FILENAME\n sub hash_object {\n \tmy ($self, $type, $file) = _maybe_self(@_);\n-\n-\t# hash_object_* implemented in Git.xs.\n-\n-\tif (ref($file) eq 'GLOB') {\n-\t\tmy $hash = hash_object_pipe($type, fileno($file));\n-\t\tclose $file;\n-\t\treturn $hash;\n-\t} else {\n-\t\thash_object_file($type, $file);\n-\t}\n+\tcommand_oneline('hash-object', '-t', $type, $file);\n }\n \n \n@@ -802,7 +777,7 @@ sub _cmd_exec {\n \n # Execute the given Git command ($_[0]) with arguments ($_[1..])\n # by searching for it at proper places.\n-# _execv_git_cmd(), implemented in Git.xs.\n+sub _execv_git_cmd { exec('git', @_); }\n \n # Close pipe to a subprocess.\n sub _cmd_close {\n@@ -821,39 +796,6 @@ sub _cmd_close {\n }\n \n \n-# Trickery for .xs routines: In order to avoid having some horrid\n-# C code trying to do stuff with undefs and hashes, we gate all\n-# xs calls through the following and in case we are being ran upon\n-# an instance call a C part of the gate which will set up the\n-# environment properly.\n-sub _call_gate {\n-\tmy $xsfunc = shift;\n-\tmy ($self, @args) = _maybe_self(@_);\n-\n-\tif (defined $self) {\n-\t\t# XXX: We ignore the WorkingCopy! To properly support\n-\t\t# that will require heavy changes in libgit.\n-\n-\t\t# XXX: And we ignore everything else as well. libgit\n-\t\t# at least needs to be extended to let us specify\n-\t\t# the $GIT_DIR instead of looking it up in environment.\n-\t\t#xs_call_gate($self->{opts}->{Repository});\n-\t}\n-\n-\t# Having to call throw from the C code is a sure path to insanity.\n-\tlocal $SIG{__DIE__} = sub { throw Error::Simple(\"@_\"); };\n-\t&$xsfunc(@args);\n-}\n-\n-sub AUTOLOAD {\n-\tmy $xsname;\n-\tour $AUTOLOAD;\n-\t($xsname = $AUTOLOAD) =~ s/.*:://;\n-\tthrow Error::Simple(\"&Git::$xsname not defined\") if $xsname =~ /^xs_/;\n-\t$xsname = 'xs_'.$xsname;\n-\t_call_gate(\\&$xsname, @_);\n-}\n-\n sub DESTROY { }\n \n \ndiff --git a/perl/Git.xs b/perl/Git.xs\ndeleted file mode 100644\nindex 2bbec43..0000000\n--- a/perl/Git.xs\n+++ /dev/null\n@@ -1,134 +0,0 @@\n-/* By carefully stacking #includes here (even if WE don't really need them)\n- * we strive to make the thing actually compile. Git header files aren't very\n- * nice. Perl headers are one of the signs of the coming apocalypse. */\n-#include <ctype.h>\n-/* Ok, it hasn't been so bad so far. */\n-\n-/* libgit interface */\n-#include \"../cache.h\"\n-#include \"../exec_cmd.h\"\n-\n-/* XS and Perl interface */\n-#include \"EXTERN.h\"\n-#include \"perl.h\"\n-#include \"XSUB.h\"\n-\n-\n-static char *\n-report_xs(const char *prefix, const char *err, va_list params)\n-{\n-\tstatic char buf[4096];\n-\tstrcpy(buf, prefix);\n-\tvsnprintf(buf + strlen(prefix), 4096 - strlen(prefix), err, params);\n-\treturn buf;\n-}\n-\n-static void NORETURN\n-die_xs(const char *err, va_list params)\n-{\n-\tchar *str;\n-\tstr = report_xs(\"fatal: \", err, params);\n-\tcroak(str);\n-}\n-\n-static void\n-error_xs(const char *err, va_list params)\n-{\n-\tchar *str;\n-\tstr = report_xs(\"error: \", err, params);\n-\twarn(str);\n-}\n-\n-\n-MODULE = Git\t\tPACKAGE = Git\n-\n-PROTOTYPES: DISABLE\n-\n-\n-BOOT:\n-{\n-\tset_error_routine(error_xs);\n-\tset_die_routine(die_xs);\n-}\n-\n-\n-# /* TODO: xs_call_gate(). See Git.pm. */\n-\n-\n-char *\n-xs_version()\n-CODE:\n-{\n-\tRETVAL = GIT_VERSION;\n-}\n-OUTPUT:\n-\tRETVAL\n-\n-\n-char *\n-xs_exec_path()\n-CODE:\n-{\n-\tRETVAL = (char *)git_exec_path();\n-}\n-OUTPUT:\n-\tRETVAL\n-\n-\n-void\n-xs__execv_git_cmd(...)\n-CODE:\n-{\n-\tconst char **argv;\n-\tint i;\n-\n-\targv = malloc(sizeof(const char *) * (items + 1));\n-\tif (!argv)\n-\t\tcroak(\"malloc failed\");\n-\tfor (i = 0; i < items; i++)\n-\t\targv[i] = strdup(SvPV_nolen(ST(i)));\n-\targv[i] = NULL;\n-\n-\texecv_git_cmd(argv);\n-\n-\tfor (i = 0; i < items; i++)\n-\t\tif (argv[i])\n-\t\t\tfree((char *) argv[i]);\n-\tfree((char **) argv);\n-}\n-\n-char *\n-xs_hash_object_pipe(type, fd)\n-\tchar *type;\n-\tint fd;\n-CODE:\n-{\n-\tunsigned char sha1[20];\n-\n-\tif (index_pipe(sha1, fd, type, 0))\n-\t\tcroak(\"Unable to hash given filehandle\");\n-\tRETVAL = sha1_to_hex(sha1);\n-}\n-OUTPUT:\n-\tRETVAL\n-\n-char *\n-xs_hash_object_file(type, path)\n-\tchar *type;\n-\tchar *path;\n-CODE:\n-{\n-\tunsigned char sha1[20];\n-\tint fd = open(path, O_RDONLY);\n-\tstruct stat st;\n-\n-\tif (fd < 0 ||\n-\t    fstat(fd, &st) < 0 ||\n-\t    index_fd(sha1, fd, &st, 0, type))\n-\t\tcroak(\"Unable to hash %s\", path);\n-\tclose(fd);\n-\n-\tRETVAL = sha1_to_hex(sha1);\n-}\n-OUTPUT:\n-\tRETVAL\ndiff --git a/perl/Makefile.PL b/perl/Makefile.PL\nindex ef9d82d..de73235 100644\n--- a/perl/Makefile.PL\n+++ b/perl/Makefile.PL\n@@ -3,12 +3,7 @@ use ExtUtils::MakeMaker;\n sub MY::postamble {\n \treturn <<'MAKE_FRAG';\n instlibdir:\n-\t@echo '$(INSTALLSITEARCH)'\n-\n-check:\n-\tperl -MDevel::PPPort -le 'Devel::PPPort::WriteFile(\".ppport.h\")' && \\\n-\tperl .ppport.h --compat-version=5.6.0 Git.xs && \\\n-\trm .ppport.h\n+\t@echo '$(INSTALLSITELIB)'\n \n MAKE_FRAG\n }\n@@ -29,7 +24,5 @@ WriteMakefile(\n \tNAME            => 'Git',\n \tVERSION_FROM    => 'Git.pm',\n \tPM\t\t=> \\%pm,\n-\tMYEXTLIB        => '../libgit.a',\n-\tINC             => '-I. -I..',\n \t%extra\n );\n"},{"id":"27537","messageId":"7vwt7ta6sh.fsf@assigned-by-dhcp.cox.net","threadId":"5665","inReplyTo":"20060923181849.28753.56984.stgit@machine.or.cz","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-24T10:38:22Z","receivedAt":"2006-09-24T10:38:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> This patch removes Git.xs from the repository for the time being. This\n> should hopefully enable Git.pm to finally make its way to master.\n\nIt's a bit sad, but I think this is a sane thing to do.\n\nI suspect Alex still has problem with Makefile.PL due to issues\naround ActiveState (slash vs bs and CRLF line ending if I recall\ncorrectly)?\n"},{"id":"27538","messageId":"20060924105016.GA5732@steel.home","threadId":"5665","inReplyTo":"7vwt7ta6sh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Alex Riesen","fromEmail":"fork0@t-online.de","sentAt":"2006-09-24T10:50:16Z","receivedAt":"2006-09-24T10:50:16Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Sun, Sep 24, 2006 12:38:22 +0200:\n> Petr Baudis <pasky@suse.cz> writes:\n> \n> > This patch removes Git.xs from the repository for the time being. This\n> > should hopefully enable Git.pm to finally make its way to master.\n> \n> It's a bit sad, but I think this is a sane thing to do.\n> \n> I suspect Alex still has problem with Makefile.PL due to issues\n> around ActiveState (slash vs bs and CRLF line ending if I recall\n> correctly)?\n> \n\nAbsolutely.\n"},{"id":"27540","messageId":"20060924110816.GK20017@pasky.or.cz","threadId":"5665","inReplyTo":"20060924105016.GA5732@steel.home","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-24T11:08:16Z","receivedAt":"2006-09-24T11:08:16Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, Sep 24, 2006 at 12:50:16PM CEST, I got a letter\nwhere Alex Riesen <fork0@t-online.de> said that...\n> Junio C Hamano, Sun, Sep 24, 2006 12:38:22 +0200:\n> > Petr Baudis <pasky@suse.cz> writes:\n> > \n> > > This patch removes Git.xs from the repository for the time being. This\n> > > should hopefully enable Git.pm to finally make its way to master.\n> > \n> > It's a bit sad, but I think this is a sane thing to do.\n> > \n> > I suspect Alex still has problem with Makefile.PL due to issues\n> > around ActiveState (slash vs bs and CRLF line ending if I recall\n> > correctly)?\n> > \n> \n> Absolutely.\n\nI'm sorry but I don't even have a Windows installation on my machine and\nI don't intend to personally work on Windows support. If you need to\nkeep ActiveState working seamlessly, please post patches. I hate to say\nit, but that's just how it should work - if someone cares about Git\nworking on a non-mainstream (for Git audience) platform, he needs to\nmaintain it, not ask the \"core\" developers do it. It's like if you\nwithheld random kernel features because they break m68k and noone is\naround to fix it.\n\nIf someone cares, we'll get patches.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27545","messageId":"20060924114834.GA5800@steel.home","threadId":"5665","inReplyTo":"20060924110816.GK20017@pasky.or.cz","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Alex Riesen","fromEmail":"fork0@t-online.de","sentAt":"2006-09-24T11:48:34Z","receivedAt":"2006-09-24T11:48:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Petr Baudis, Sun, Sep 24, 2006 13:08:16 +0200:\n> > > > This patch removes Git.xs from the repository for the time being. This\n> > > > should hopefully enable Git.pm to finally make its way to master.\n> > > \n> > > It's a bit sad, but I think this is a sane thing to do.\n> > > \n> > > I suspect Alex still has problem with Makefile.PL due to issues\n> > > around ActiveState (slash vs bs and CRLF line ending if I recall\n> > > correctly)?\n> > \n> > Absolutely.\n> \n> I'm sorry but I don't even have a Windows installation on my machine and\n> I don't intend to personally work on Windows support. If you need to\n> keep ActiveState working seamlessly, please post patches. I hate to say\n> it, but that's just how it should work - if someone cares about Git\n> working on a non-mainstream (for Git audience) platform, he needs to\n> maintain it, not ask the \"core\" developers do it. ...\n\nAin't asking you nor anyone. Ain't actually care, activetstate's perl\nbeing just another stupid windows program done badly. Can't promise,\nbut will try to post my workarounds to that particular combination\non list (never demanded to include them).\nThe original mail was more of a word of warning for anyone unhappy to\nwork with the sh%t.\n\nAside from that, any chance of making Git.xs optional (enabled by\ndefault, but disableable)? Disabling Git.xs will disable related\nscripts, sure, but also gives you an installation with one external\n(libperl) dependency less (probably with ability to install git and\nGit.xs separately).\n"},{"id":"27547","messageId":"20060924120724.GB5732@steel.home","threadId":"5665","inReplyTo":"20060924114834.GA5800@steel.home","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Alex Riesen","fromEmail":"fork0@t-online.de","sentAt":"2006-09-24T12:07:24Z","receivedAt":"2006-09-24T12:07:24Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Alex Riesen, Sun, Sep 24, 2006 13:48:34 +0200:\n> Aside from that, any chance of making Git.xs optional (enabled by\n> default, but disableable)? Disabling Git.xs will disable related\n> scripts, sure, but also gives you an installation with one external\n> (libperl) dependency less (probably with ability to install git and\n> Git.xs separately).\n\nI see that was already meantioned. Sorry, just have read the whole\ndiscussion.\n"},{"id":"27548","messageId":"20060924120902.GO20017@pasky.or.cz","threadId":"5665","inReplyTo":"20060924114834.GA5800@steel.home","subject":"Re: [PATCH] Git.pm: Kill Git.xs for now","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-24T12:09:02Z","receivedAt":"2006-09-24T12:09:02Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, Sep 24, 2006 at 01:48:34PM CEST, I got a letter\nwhere Alex Riesen <fork0@t-online.de> said that...\n> Aside from that, any chance of making Git.xs optional (enabled by\n> default, but disableable)? Disabling Git.xs will disable related\n> scripts, sure, but also gives you an installation with one external\n> (libperl) dependency less (probably with ability to install git and\n> Git.xs separately).\n\nAbsolutely.\n\nActually, as I noted in the patch comment, if we reintroduce Git.xs\nagain we should probably do it so that it is completely optional and the\nequivalent functionality is also implemented in Git.pm in pure perl +\nexecing Git. I have been originally opposed to that but after all the\nwoes we had to go through I have changed my opinion.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"}]}