threads / discuss / 4860

remove perl from git-commit.sh

Subject: remove perl from git-commit.sh

## tl;dr

9 messages between Jul 12, 2006 and Jul 13, 2006.

replies: 8people: 4as markdown or json

Alex Riesen· Jul 12, 2006, 11:15 UTC · lore

It is really annoying when the essentials do not work. I think we could improve at least them, by minimizing their dependencies to external tools.

---

git-commit.sh has only ohne place where perl is used and it can actually quite trivially be done in sh. git-ls-files without "-z" produces quoted output, even if is different from that produced by perl code it could be enough. Otherwise I'd better suggest to add another quoting style (replacing only \t, \n and backslash) than having to constantly patch git-commit.sh for binmode or perl path (Windows has no stable tool set, and perl for instance often resides "somewhere").

From 69bf41df4ef69d0f1e4ab52942c59bb3fd568cb8 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed, 12 Jul 2006 13:02:23 +0200
Subject: remove perl from git-commit.sh
---
 git-commit.sh |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index 802dd72..4cf3fab 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -138,32 +138,26 @@ #'
         if test -z "$untracked_files"; then
             option="--directory --no-empty-directory"
         fi
+	hdr_shown=
 	if test -f "$GIT_DIR/info/exclude"
 	then
-	    git-ls-files -z --others $option \
+	    git-ls-files --others $option \
 		--exclude-from="$GIT_DIR/info/exclude" \
 		--exclude-per-directory=.gitignore
 	else
-	    git-ls-files -z --others $option \
+	    git-ls-files --others $option \
 		--exclude-per-directory=.gitignore
 	fi |
-	@@PERL@@ -e '$/ = "\0";
-	    my $shown = 0;
-	    while (<>) {
-		chomp;
-		s|\\|\\\\|g;
-		s|\t|\\t|g;
-		s|\n|\\n|g;
-		s/^/#	/;
-		if (!$shown) {
-		    print "#\n# Untracked files:\n";
-		    print "#   (use \"git add\" to add to commit)\n";
-		    print "#\n";
-		    $shown = 1;
-		}
-		print "$_\n";
-	    }
-	'
+	while read line; do
+	    if [ -z "$hdr_shown" ]; then
+		echo '#'
+		echo '# Untracked files:'
+		echo '#   (use "git add" to add to commit)'
+		echo '#'
+		hdr_shown=1
+	    fi
+	    echo "#	$line"
+	done
 
 	if test -n "$verbose" -a -z "$IS_INITIAL"
 	then
-- 
1.4.1.gb4adf
Nikolai Weibull· Jul 12, 2006, 11:17 UTC · re: Alex Riesen · lore

Re: remove perl from git-commit.sh

On 7/12/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> Otherwise I'd better suggest to add another
> quoting style (replacing only \t, \n and backslash)
One could always do the quoting with sed, or am I missing something?
  nikolai
Alex Riesen· Jul 12, 2006, 11:23 UTC · re: Nikolai Weibull · lore

Re: remove perl from git-commit.sh

On 7/12/06, Nikolai Weibull <now@bitwi.se> wrote:
Show 5 quoted lines
> > Otherwise I'd better suggest to add another
> > quoting style (replacing only \t, \n and backslash)
>
> One could always do the quoting with sed, or am I missing something?
>

No, but if look closely at this list you'll find a lot of incompatibilities in sed too. So the less, the better

Nikolai Weibull· Jul 12, 2006, 11:35 UTC · re: Alex Riesen · lore

Re: remove perl from git-commit.sh

On 7/12/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 7/12/06, Nikolai Weibull <now@bitwi.se> wrote:
> > > Otherwise I'd better suggest to add another
> > > quoting style (replacing only \t, \n and backslash)
> > One could always do the quoting with sed, or am I missing something?
> No, but if look closely at this list you'll find a lot of incompatibilities
> in sed too. So the less, the better

Well, that's true, of course, but something as "simple" as what is needed in this case should be possible. Anyway, I was just wondering what the motives were, not questioning them ;-).

  nikolai
Alex Riesen· Jul 12, 2006, 11:41 UTC · re: Nikolai Weibull · lore

Re: remove perl from git-commit.sh

On 7/12/06, Nikolai Weibull <now@bitwi.se> wrote:
> Anyway, I was just wondering
> what the motives were, not questioning them ;-).

Being forced to use ActiveState Perl on a W2K in a corporate environment.

Petr Baudis· Jul 12, 2006, 11:48 UTC · re: Alex Riesen · lore

Re: remove perl from git-commit.sh

Dear diary, on Wed, Jul 12, 2006 at 01:15:03PM CEST, I got a letter where Alex Riesen <raa.lkml@gmail.com> said that...

> ---

BTW, why people so often want to keep stuff like the following from commit messages?

Show 6 quoted lines
> git-commit.sh has only ohne place where perl is used
> and it can actually quite trivially be done in sh.
> git-ls-files without "-z" produces quoted output, even if
> is different from that produced by perl code it could be
> enough. Otherwise I'd better suggest to add another
> quoting style (replacing only \t, \n and backslash)
Of course getting rid of Perl from the "essentials" is an option...
> than having to constantly patch git-commit.sh for binmode
> or perl path (Windows has no stable tool set, and perl
> for instance often resides "somewhere").

...but I would prefer much better if we could actually fix the problem. I'm not sure what "patching for binmode" takes but it sounds that we should do it for the other Perl scripts anyway. If Cygwin usually really doesn't have stable Perl path, you could special-case PERL_PATH in Makefile for Cygwin to be just "perl" (or perhaps that's the issue only for non-source packages so you might want to simply set PERL_PATH to perl when building those).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam
Alex Riesen· Jul 12, 2006, 12:23 UTC · re: Petr Baudis · lore

Re: remove perl from git-commit.sh

On 7/12/06, Petr Baudis <pasky@suse.cz> wrote:
> BTW, why people so often want to keep stuff like the following from
> commit messages?
Well, strictly speaking it isn't the description of what the patch does.
Show 6 quoted lines
> > than having to constantly patch git-commit.sh for binmode
> > or perl path (Windows has no stable tool set, and perl
> > for instance often resides "somewhere").
>
> ...but I would prefer much better if we could actually fix the problem.
> I'm not sure what "patching for binmode" takes but it sounds that we

ActiveState Perl normally replaces \n in the output with \r\n, which is annoying, as the rest of git does output \n verbatim.

> should do it for the other Perl scripts anyway. If Cygwin usually really
> doesn't have stable Perl path, you could special-case PERL_PATH in

It's not cygwin. It's an organization typical for windows environements: stinky trash can.

> Makefile for Cygwin to be just "perl" (or perhaps that's the issue only
> for non-source packages so you might want to simply set PERL_PATH to
> perl when building those).

I did. I didn't mention it because the patch was about the binmode. sh is from cygwin here, so just removing this one dependency helps a lot. I will change other scripts as soon as they get problematic.

Another small problem is with windows pathnames (activestate perl expects _windows_ path, but allows forward slashes in it: c:/a/b/c). I have calls to cygpath (converts cygwin path to windows and vice versa) all over (not everywhere, just clone, rerere, mv, shortlog and fmt-merge-msg. The tools that were important to me) just to deal with this. It's a horribly unstable and poorly tested and breaks from time to time but it mostly works. As I saw no complaints here about the same problem I have never submitted the changes. Maybe I am just unlucky to have to deal with windows so often.

Junio C Hamano· Jul 13, 2006, 04:38 UTC · re: Alex Riesen · lore

Re: remove perl from git-commit.sh

"Alex Riesen" <raa.lkml@gmail.com> writes:
Show 15 quoted lines
> It is really annoying when the essentials do not work.
> I think we could improve at  least them, by minimizing
> their dependencies to external tools.
>
> ---
>
> git-commit.sh has only ohne place where perl is used
> and it can actually quite trivially be done in sh.
> git-ls-files without "-z" produces quoted output, even if
> is different from that produced by perl code it could be
> enough. Otherwise I'd better suggest to add another
> quoting style (replacing only \t, \n and backslash) than
> having to constantly patch git-commit.sh for binmode
> or perl path (Windows has no stable tool set, and perl
> for instance often resides "somewhere").
(1) Sign-off?
(2) I think the cover letter comment talks more apporpirate
    things than your proposed commit message.  The commit log is
    not a place to vent your frustration.  It's where you
    justify why that change was needed for people who will want
    to figure out why your patch broke their workflow later.

I do not think anybody parses git-status output to figure out the list of paths that are touched/untracked/etc., so I am not opposed to this change itself.

Alex Riesen· Jul 13, 2006, 08:30 UTC · re: Junio C Hamano · lore

git-commit.sh has the only one place where perl is used and there it can quite trivially be done in sh.

git-ls-files without "-z" produces quoted output, even if is different from that produced by perl code it could be enough. Otherwise I'd better suggest to add another quoting style (replacing only \t, \n and backslash).

This system is an ugly combination of cygwin and activestate perl. The combination has some quirks (like the perl producing \r\n by default, expecting windows pathnames instead of cygwin fakes, or ignoring environment variables under some hard to reproduce circumstances), so reducing number of this interactions reduces number of hacks one has to put in core code to make things work. I used to patch git-commit.sh to put binmode in perl output, and git-clone.sh still has these calls to cygpath.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

It is really annoying when the essentials do not work. I think we could improve at least them, by minimizing their dependencies to external tools.

Junio C Hamano wrote:
> (1) Sign-off?
done
Show 5 quoted lines
> (2) I think the cover letter comment talks more apporpirate
>    things than your proposed commit message.  The commit log is
>    not a place to vent your frustration.  It's where you
>    justify why that change was needed for people who will want
>    to figure out why your patch broke their workflow later.
done.
From 69bf41df4ef69d0f1e4ab52942c59bb3fd568cb8 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed, 12 Jul 2006 13:02:23 +0200
Subject: remove perl from git-commit.sh
---
 git-commit.sh |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index 802dd72..4cf3fab 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -138,32 +138,26 @@ #'
         if test -z "$untracked_files"; then
             option="--directory --no-empty-directory"
         fi
+	hdr_shown=
 	if test -f "$GIT_DIR/info/exclude"
 	then
-	    git-ls-files -z --others $option \
+	    git-ls-files --others $option \
 		--exclude-from="$GIT_DIR/info/exclude" \
 		--exclude-per-directory=.gitignore
 	else
-	    git-ls-files -z --others $option \
+	    git-ls-files --others $option \
 		--exclude-per-directory=.gitignore
 	fi |
-	@@PERL@@ -e '$/ = "\0";
-	    my $shown = 0;
-	    while (<>) {
-		chomp;
-		s|\\|\\\\|g;
-		s|\t|\\t|g;
-		s|\n|\\n|g;
-		s/^/#	/;
-		if (!$shown) {
-		    print "#\n# Untracked files:\n";
-		    print "#   (use \"git add\" to add to commit)\n";
-		    print "#\n";
-		    $shown = 1;
-		}
-		print "$_\n";
-	    }
-	'
+	while read line; do
+	    if [ -z "$hdr_shown" ]; then
+		echo '#'
+		echo '# Untracked files:'
+		echo '#   (use "git add" to add to commit)'
+		echo '#'
+		hdr_shown=1
+	    fi
+	    echo "#	$line"
+	done
 
 	if test -n "$verbose" -a -z "$IS_INITIAL"
 	then
-- 
1.4.1.gb4adf

← back to recent threads