threads / patch / 6572

patchReplace perl code with pure shell code

Subject: [PATCH] Replace perl code with pure shell code

## tl;dr

12 messages between Jan 29, 2007 and Jan 30, 2007. Diffs are folded; open one to read it.

replies: 11people: 6as markdown or json

Simon 'corecode' Schubert· Jan 29, 2007, 08:09 UTC · lore
Signed-off-by: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
---
 git-clone.sh |   65 ++++++++++++++++++---------------------------------------
 1 files changed, 21 insertions(+), 44 deletions(-)
Show changes to git-clone.sh +21 −44
diff --git a/git-clone.sh b/git-clone.sh
index ced7dfb..b3c6fa4 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -66,48 +66,6 @@ Perhaps git-update-server-info needs to be run there?"
 	rm -f "$GIT_DIR/REMOTE_HEAD"
 }
 
-# Read git-fetch-pack -k output and store the remote branches.
-copy_refs='
-use File::Path qw(mkpath);
-use File::Basename qw(dirname);
-my $git_dir = $ARGV[0];
-my $use_separate_remote = $ARGV[1];
-my $origin = $ARGV[2];
-
-my $branch_top = ($use_separate_remote ? "remotes/$origin" : "heads");
-my $tag_top = "tags";
-
-sub store {
-	my ($sha1, $name, $top) = @_;
-	$name = "$git_dir/refs/$top/$name";
-	mkpath(dirname($name));
-	open O, ">", "$name";
-	print O "$sha1\n";
-	close O;
-}
-
-open FH, "<", "$git_dir/CLONE_HEAD";
-while (<FH>) {
-	my ($sha1, $name) = /^([0-9a-f]{40})\s(.*)$/;
-	next if ($name =~ /\^\173/);
-	if ($name eq "HEAD") {
-		open O, ">", "$git_dir/REMOTE_HEAD";
-		print O "$sha1\n";
-		close O;
-		next;
-	}
-	if ($name =~ s/^refs\/heads\///) {
-		store($sha1, $name, $branch_top);
-		next;
-	}
-	if ($name =~ s/^refs\/tags\///) {
-		store($sha1, $name, $tag_top);
-		next;
-	}
-}
-close FH;
-'
-
 quiet=
 local=no
 use_local=no
@@ -332,8 +290,27 @@ test -d "$GIT_DIR/refs/reference-tmp" && rm -fr "$GIT_DIR/refs/reference-tmp"
 if test -f "$GIT_DIR/CLONE_HEAD"
 then
 	# Read git-fetch-pack -k output and store the remote branches.
-	@@PERL@@ -e "$copy_refs" "$GIT_DIR" "$use_separate_remote" "$origin" ||
-	exit
+	if [ -n "$use_separate_remote" ]
+	then
+		branch_top="remotes/$origin"
+	else
+		branch_top="heads"
+	fi
+	tag_top="tags"
+	while read sha1 name
+	do
+		case "$name" in
+		HEAD)
+			destname="REMOTE_HEAD" ;;
+		refs/heads/*)
+			destname="refs/$branch_top/${name#refs/heads/}" ;;
+		refs/tags/*)
+			destname="refs/$tag_top/${name#refs/tags/}" ;;
+		*)
+			continue ;;
+		esac
+		git-update-ref -m "clone: from $repo" "$destname" "$sha1" ""
+	done < "$GIT_DIR/CLONE_HEAD"
 fi
 
 cd "$D" || exit
-- 
1.5.0.rc1.196.geebfb
Junio C Hamano· Jan 29, 2007, 08:56 UTC · re: Simon 'corecode' Schubert · lore

Re: [PATCH] Replace perl code with pure shell code

Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
Show 14 quoted lines
> Signed-off-by: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
> ...
> diff --git a/git-clone.sh b/git-clone.sh
> index ced7dfb..b3c6fa4 100755
> --- a/git-clone.sh
> +++ b/git-clone.sh
> @@ -66,48 +66,6 @@ Perhaps git-update-server-info needs to be run there?"
> ...
> -open FH, "<", "$git_dir/CLONE_HEAD";
> -while (<FH>) {
> -	my ($sha1, $name) = /^([0-9a-f]{40})\s(.*)$/;
> -	next if ($name =~ /\^\173/);
> -	if ($name eq "HEAD") {
> ...
Thanks.  I like the general direction, but not quite.

You exposed one outstanding bug, which is a hint about what is not quite right with your patch.

-- >8 -- [PATCH] update-ref: do not accept malformatted refs.

We used to use lock_any_ref_for_update() because the command needs to also update HEAD (which is not under refs/, so lock_ref_sha1() cannot be used). The function however did not check for refs with illegal characters in them.

Use check_ref_format() to catch malformed refs. For this check, we specifically do not want to say having less than two levels in the name is illegal to allow HEAD (and perhaps other special refs in the future).

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
Show changes to 2 files +4 −4

builtin-update-ref.c, refs.c

diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 1461937..5ee960b 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -61,10 +61,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 
 	lock = lock_any_ref_for_update(refname, oldval ? oldsha1 : NULL);
 	if (!lock)
-		return 1;
+		die("%s: cannot lock the ref", refname);
 	if (write_ref_sha1(lock, sha1, msg) < 0)
-		return 1;
-
-	/* write_ref_sha1 always unlocks the ref, no need to do it explicitly */
+		die("%s: cannot update the ref", refname);
 	return 0;
 }
diff --git a/refs.c b/refs.c
index 12e46b8..3db444c 100644
--- a/refs.c
+++ b/refs.c
@@ -710,6 +710,8 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
 
 struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1)
 {
+	if (check_ref_format(ref) == -1)
+		return NULL;
 	return lock_ref_sha1_basic(ref, old_sha1, NULL);
 }
 
Simon 'corecode' Schubert· Jan 29, 2007, 09:18 UTC · re: Junio C Hamano · lore
Signed-off-by: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
---
Show 8 quoted lines
>> -	next if ($name =~ /\^\173/);
>> -	if ($name eq "HEAD") {
>> ...
> 
> Thanks.  I like the general direction, but not quite.
> 
> You exposed one outstanding bug, which is a hint about what is
> not quite right with your patch.
I already wondered.  What's those ^{} tags, and why is CLONE_HEAD littered with them?
 git-clone.sh |   67 ++++++++++++++++++++--------------------------------------
 1 files changed, 23 insertions(+), 44 deletions(-)
Show changes to git-clone.sh +23 −44
diff --git a/git-clone.sh b/git-clone.sh
index ced7dfb..869caf9 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -66,48 +66,6 @@ Perhaps git-update-server-info needs to be run there?"
 	rm -f "$GIT_DIR/REMOTE_HEAD"
 }
 
-# Read git-fetch-pack -k output and store the remote branches.
-copy_refs='
-use File::Path qw(mkpath);
-use File::Basename qw(dirname);
-my $git_dir = $ARGV[0];
-my $use_separate_remote = $ARGV[1];
-my $origin = $ARGV[2];
-
-my $branch_top = ($use_separate_remote ? "remotes/$origin" : "heads");
-my $tag_top = "tags";
-
-sub store {
-	my ($sha1, $name, $top) = @_;
-	$name = "$git_dir/refs/$top/$name";
-	mkpath(dirname($name));
-	open O, ">", "$name";
-	print O "$sha1\n";
-	close O;
-}
-
-open FH, "<", "$git_dir/CLONE_HEAD";
-while (<FH>) {
-	my ($sha1, $name) = /^([0-9a-f]{40})\s(.*)$/;
-	next if ($name =~ /\^\173/);
-	if ($name eq "HEAD") {
-		open O, ">", "$git_dir/REMOTE_HEAD";
-		print O "$sha1\n";
-		close O;
-		next;
-	}
-	if ($name =~ s/^refs\/heads\///) {
-		store($sha1, $name, $branch_top);
-		next;
-	}
-	if ($name =~ s/^refs\/tags\///) {
-		store($sha1, $name, $tag_top);
-		next;
-	}
-}
-close FH;
-'
-
 quiet=
 local=no
 use_local=no
@@ -332,8 +290,29 @@ test -d "$GIT_DIR/refs/reference-tmp" && rm -fr "$GIT_DIR/refs/reference-tmp"
 if test -f "$GIT_DIR/CLONE_HEAD"
 then
 	# Read git-fetch-pack -k output and store the remote branches.
-	@@PERL@@ -e "$copy_refs" "$GIT_DIR" "$use_separate_remote" "$origin" ||
-	exit
+	if [ -n "$use_separate_remote" ]
+	then
+		branch_top="remotes/$origin"
+	else
+		branch_top="heads"
+	fi
+	tag_top="tags"
+	while read sha1 name
+	do
+		case "$name" in
+		*^{*)
+			continue ;;
+		HEAD)
+			destname="REMOTE_HEAD" ;;
+		refs/heads/*)
+			destname="refs/$branch_top/${name#refs/heads/}" ;;
+		refs/tags/*)
+			destname="refs/$tag_top/${name#refs/tags/}" ;;
+		*)
+			continue ;;
+		esac
+		git-update-ref -m "clone: from $repo" "$destname" "$sha1" ""
+	done < "$GIT_DIR/CLONE_HEAD"
 fi
 
 cd "$D" || exit
-- 
1.5.0.rc1.196.geebfb
Shawn O. Pearce· Jan 29, 2007, 09:28 UTC · re: Simon 'corecode' Schubert · lore

Re: [PATCH] Replace perl code with pure shell code

Simon 'corecode' Schubert <corecode@fs.ei.tum.de> wrote:
> I already wondered.  What's those ^{} tags, and why is CLONE_HEAD littered 
> with them?
They are the deref of the thing without them.
As in, "foo" is a tag pointing at some object (probably a commit
but not necessarily) then "foo^{}" is whatever "foo"'s tag points at.
 
> +		case "$name" in
> +		*^{*)
> +			continue ;;
Probably could just be:
		case "$name" in
		*^{})
			continue ;;

This is common in Git. ^{} on the end of a ref name shows up in the peek-remote/ls-remote output, but certainly is *not* a ref.

Sorry I missed that case eariler when I reviewed the patch. I thought about it and why it wasn't handled here, but then thought maybe it wasn't actually occuring in the input (that someone else higher up had filtered them out).

-- 
Shawn.
Randal L. Schwartz· Jan 29, 2007, 11:41 UTC · re: Simon 'corecode' Schubert · lore

Re: [PATCH] Replace perl code with pure shell code

>>>>> "Simon" == Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
Simon> +			destname="refs/$branch_top/${name#refs/heads/}" ;;

I don't think this is portable shell. At least Perl is the same everywhere. Ignore me if this is a shell syntax on something other than bash.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
Nikolai Weibull· Jan 29, 2007, 12:38 UTC · re: Randal L. Schwartz · lore

Re: [PATCH] Replace perl code with pure shell code

On 1/29/07, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
Show 6 quoted lines
> >>>>> "Simon" == Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
>
> Simon> +                        destname="refs/$branch_top/${name#refs/heads/}" ;;
>
> I don't think this is portable shell.  At least Perl is the same everywhere.
> Ignore me if this is a shell syntax on something other than bash.
According to "The Open Group Base Specifications Issue 6" [1]:
${parameter#word}
    Remove Smallest Prefix Pattern. The word shall be expanded to
produce a pattern. The parameter expansion shall then result in
parameter, with the smallest portion of the prefix matched by the
pattern deleted.
We do require a POSIX-compliant version of sh already, right?
  nikolai
[1] http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02
Randal L. Schwartz· Jan 29, 2007, 12:53 UTC · re: Nikolai Weibull · lore

Re: [PATCH] Replace perl code with pure shell code

>>>>> "Nikolai" == Nikolai Weibull <now@bitwi.se> writes:
Nikolai> We do require a POSIX-compliant version of sh already, right?

OK, sorry for raising the flag. I just know that writing portable shell is far trickier than writing portable Perl, so I get overly cautious on anything that wasn't in Unix V7.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
Nikolai Weibull· Jan 29, 2007, 13:17 UTC · re: Randal L. Schwartz · lore

Re: [PATCH] Replace perl code with pure shell code

On 1/29/07, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
Show 7 quoted lines
> >>>>> "Nikolai" == Nikolai Weibull <now@bitwi.se> writes:
>
> Nikolai> We do require a POSIX-compliant version of sh already, right?
>
> OK, sorry for raising the flag.  I just know that writing portable shell
> is far trickier than writing portable Perl, so I get overly cautious
> on anything that wasn't in Unix V7.

No problem; I wasn't sure on the current requirements myself :-). It's too bad that not all the great stuff one can find in Bash and Zsh were in the original...

  nikolai
Randal L. Schwartz· Jan 29, 2007, 13:21 UTC · re: Nikolai Weibull · lore

Re: [PATCH] Replace perl code with pure shell code

>>>>> "Nikolai" == Nikolai Weibull <now@bitwi.se> writes:
Nikolai> On 1/29/07, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
>> >>>>> "Nikolai" == Nikolai Weibull <now@bitwi.se> writes:
>> 
Nikolai> We do require a POSIX-compliant version of sh already, right?
>> 
>> OK, sorry for raising the flag.  I just know that writing portable shell
>> is far trickier than writing portable Perl, so I get overly cautious
>> on anything that wasn't in Unix V7.

Nikolai> No problem; I wasn't sure on the current requirements myself :-). Nikolai> It's too bad that not all the great stuff one can find in Bash and Zsh Nikolai> were in the original...

I'll probably seem like a traitor to my tribe for saying this, but if zsh or gnu awk had been around at the time Perl was created, we wouldn't have seen Perl. Of course, Perl has gone far beyond the "awk replacement" as originally planned, but who knows what might have happened next?

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
Junio C Hamano· Jan 30, 2007, 10:57 UTC · re: Randal L. Schwartz · lore

Re: [PATCH] Replace perl code with pure shell code

merlyn@stonehenge.com (Randal L. Schwartz) writes:
Show 7 quoted lines
>>>>>> "Nikolai" == Nikolai Weibull <now@bitwi.se> writes:
>
> Nikolai> We do require a POSIX-compliant version of sh already, right?
>
> OK, sorry for raising the flag.  I just know that writing portable shell
> is far trickier than writing portable Perl, so I get overly cautious
> on anything that wasn't in Unix V7.

You seem to be as old fashioned as I am (I grew up with V7 shell). People complained often enough how antiquated my shell script (hence many but not all git Porcelain-ish) style is.

I have tried very hard to reject "Hey, it's in POSIX -- we'll use it and screw platforms that are not exactly POSIX". I only use POSIX as a yardstick to decide what to stay away from, as in "Let's not use ${parameter//pattern/string} -- it is not _even_ in POSIX".

Jakub Narebski· Jan 29, 2007, 12:43 UTC · re: Randal L. Schwartz · lore

Re: [PATCH] Replace perl code with pure shell code

Randal L. Schwartz wrote:
Show 6 quoted lines
>>>>>> "Simon" == Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
> 
> Simon> +                      destname="refs/$branch_top/${name#refs/heads/}" ;;
> 
> I don't think this is portable shell.  At least Perl is the same everywhere.
> Ignore me if this is a shell syntax on something other than bash.
I think ${var#pattern} is a POSIX shell syntax.
Nevertheless Perl embedded in shell script is (a bit) horrible.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Randal L. Schwartz· Jan 29, 2007, 12:54 UTC · re: Jakub Narebski · lore

Re: [PATCH] Replace perl code with pure shell code

>>>>> "Jakub" == Jakub Narebski <jnareb@gmail.com> writes:
Jakub> Nevertheless Perl embedded in shell script is (a bit) horrible.

I will agree with that. Either all Perl, or all shell, unless there's an operation that begs for portability for which Perl can be an asset.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

← back to recent threads