Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-rm: Fix to properly handle files with spaces, tabs, newlines, etc.
From: Junio C Hamano @ 2006-02-23  1:07 UTC (permalink / raw)
  To: Carl Worth; +Cc: git
In-Reply-To: <873biasyew.wl%cworth@cworth.org>

Carl Worth <cworth@cworth.org> writes:

>  Please ignore the previous patch. This is what I intended to send.

Ahh.  I was wondering...

>  (For as useful as the index is---and yes, I have found it very
>  useful---I still find it easy to inadvertently commit stale data with
>  it. I guess what might help me is a command to update into the index
>  all files that are currently in the "updated but not checked in (will
>  commit)" state as reported by git status. Does such a command exist?)

No.  I do not do this myself, but this one-liner should work:

	git diff --name-only "$@" | git update-index --stdin

[from another message]

> PS. What's the syntax/tool support for just replying to an existing
> message, and at the end inserting a patch with its own subject and
> commit message? Here I've manually whacked the subject and put the
> commit message above my reply (in the style of git-format-patch) but
> that seems inelegant.

YMMV depending on the MUA you use, of course.

I start [REPLY], have my MUA quote the original and write
response while trimming excess quote, just as usual.  When I
need to add a patch, then I remove all that with \C-w
(kill-region), read a format-patch output into the same mail
buffer, and then \C-y (yank) to paste the "usual correspondence"
part below the three-dash lines.  Yes, it's all manual.  I
presume it would be easy to write a few-liner Emacs macro to do
this though...

^ permalink raw reply

* [PATCH] Give no terminating LF to error() function.
From: Junio C Hamano @ 2006-02-23  1:48 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060223003400.GA27800@diku.dk>


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

  Jonas Fonseca <fonseca@diku.dk> writes:

  > Junio C Hamano <junkio@cox.net> wrote Wed, Feb 22, 2006:
  >> +			error("unable to open object pack directory: %s: %s\n",
  >> +			      path, strerror(errno));
  >
  > No need for the ending \n.

  True.

 commit.c       |    3 ++-
 http-fetch.c   |    8 ++++----
 read-tree.c    |    4 ++--
 receive-pack.c |    4 ++--
 refs.c         |    2 +-
 sha1_file.c    |    7 ++++---
 6 files changed, 15 insertions(+), 13 deletions(-)

8cd486dcab93856c7eafe33338a3df85b79e4a3e
diff --git a/commit.c b/commit.c
index c550a00..06d5439 100644
--- a/commit.c
+++ b/commit.c
@@ -212,7 +212,8 @@ int parse_commit_buffer(struct commit *i
 	if (memcmp(bufptr, "tree ", 5))
 		return error("bogus commit object %s", sha1_to_hex(item->object.sha1));
 	if (get_sha1_hex(bufptr + 5, parent) < 0)
-		return error("bad tree pointer in commit %s\n", sha1_to_hex(item->object.sha1));
+		return error("bad tree pointer in commit %s",
+			     sha1_to_hex(item->object.sha1));
 	item->tree = lookup_tree(parent);
 	if (item->tree)
 		n_refs++;
diff --git a/http-fetch.c b/http-fetch.c
index ce3df5f..8fd9de0 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -130,7 +130,7 @@ static void start_object_request(struct 
 
 	if (obj_req->local < 0) {
 		obj_req->state = ABORTED;
-		error("Couldn't create temporary file %s for %s: %s\n",
+		error("Couldn't create temporary file %s for %s: %s",
 		      obj_req->tmpfile, obj_req->filename, strerror(errno));
 		return;
 	}
@@ -830,9 +830,9 @@ static int fetch_object(struct alt_base 
 				    obj_req->errorstr, obj_req->curl_result,
 				    obj_req->http_code, hex);
 	} else if (obj_req->zret != Z_STREAM_END) {
-		ret = error("File %s (%s) corrupt\n", hex, obj_req->url);
+		ret = error("File %s (%s) corrupt", hex, obj_req->url);
 	} else if (memcmp(obj_req->sha1, obj_req->real_sha1, 20)) {
-		ret = error("File %s has bad hash\n", hex);
+		ret = error("File %s has bad hash", hex);
 	} else if (obj_req->rename < 0) {
 		ret = error("unable to write sha1 filename %s",
 			    obj_req->filename);
@@ -854,7 +854,7 @@ int fetch(unsigned char *sha1)
 		fetch_alternates(alt->base);
 		altbase = altbase->next;
 	}
-	return error("Unable to find %s under %s\n", sha1_to_hex(sha1),
+	return error("Unable to find %s under %s", sha1_to_hex(sha1),
 		     alt->base);
 }
 
diff --git a/read-tree.c b/read-tree.c
index 52f06e3..8be307e 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -564,7 +564,7 @@ static int twoway_merge(struct cache_ent
 	struct cache_entry *oldtree = src[1], *newtree = src[2];
 
 	if (merge_size != 2)
-		return error("Cannot do a twoway merge of %d trees\n",
+		return error("Cannot do a twoway merge of %d trees",
 			     merge_size);
 
 	if (current) {
@@ -616,7 +616,7 @@ static int oneway_merge(struct cache_ent
 	struct cache_entry *a = src[1];
 
 	if (merge_size != 1)
-		return error("Cannot do a oneway merge of %d trees\n",
+		return error("Cannot do a oneway merge of %d trees",
 			     merge_size);
 
 	if (!a)
diff --git a/receive-pack.c b/receive-pack.c
index eae31e3..2a3db16 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -92,7 +92,7 @@ static int run_update_hook(const char *r
 	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
 		return error("waitpid is confused");
 	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-		return error("%s died of signal\n", update_hook);
+		return error("%s died of signal", update_hook);
 	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
 		return error("%s died strangely", update_hook);
 	default:
@@ -158,7 +158,7 @@ static int update(struct command *cmd)
 	if (run_update_hook(name, old_hex, new_hex)) {
 		unlink(lock_name);
 		cmd->error_string = "hook declined";
-		return error("hook declined to update %s\n", name);
+		return error("hook declined to update %s", name);
 	}
 	else if (rename(lock_name, name) < 0) {
 		unlink(lock_name);
diff --git a/refs.c b/refs.c
index d01fc39..826ae7a 100644
--- a/refs.c
+++ b/refs.c
@@ -268,7 +268,7 @@ static int write_ref_file(const char *fi
 	char term = '\n';
 	if (write(fd, hex, 40) < 40 ||
 	    write(fd, &term, 1) < 1) {
-		error("Couldn't write %s\n", filename);
+		error("Couldn't write %s", filename);
 		close(fd);
 		return -1;
 	}
diff --git a/sha1_file.c b/sha1_file.c
index 1fd5b79..a80d849 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -564,7 +564,7 @@ static void prepare_packed_git_one(char 
 	dir = opendir(path);
 	if (!dir) {
 		if (errno != ENOENT)
-			error("unable to open object pack directory: %s: %s\n",
+			error("unable to open object pack directory: %s: %s",
 			      path, strerror(errno));
 		return;
 	}
@@ -1513,7 +1513,8 @@ int write_sha1_from_fd(const unsigned ch
 
 	local = mkstemp(tmpfile);
 	if (local < 0)
-		return error("Couldn't open %s for %s\n", tmpfile, sha1_to_hex(sha1));
+		return error("Couldn't open %s for %s",
+			     tmpfile, sha1_to_hex(sha1));
 
 	memset(&stream, 0, sizeof(stream));
 
@@ -1561,7 +1562,7 @@ int write_sha1_from_fd(const unsigned ch
 	}
 	if (memcmp(sha1, real_sha1, 20)) {
 		unlink(tmpfile);
-		return error("File %s has bad hash\n", sha1_to_hex(sha1));
+		return error("File %s has bad hash", sha1_to_hex(sha1));
 	}
 
 	return move_temp_to_file(tmpfile, sha1_file_name(sha1));
-- 
1.2.3.g5978

^ permalink raw reply related

* [ANNOUNCE] GIT 1.2.3
From: Junio C Hamano @ 2006-02-23  1:59 UTC (permalink / raw)
  To: git; +Cc: linux-kernel

The latest maintenance release GIT 1.2.3 is available at the
usual places:

	http://www.kernel.org/pub/software/scm/git/

	git-1.2.3.tar.{gz,bz2}			(tarball)
	RPMS/$arch/git-*-1.2.3-1.$arch.rpm	(RPM)


This contains some documentation updates, and a fix for the
"empty ident not allowed" problem that bit too many people.

Breaking the tradition, this is _not_ a pure bugfix release,
however.  It contains backports of the much talked about "reuse
data from existing pack" optimization from the master branch.

Hopefully this would help downloading from the kernel.org
servers over git native protocol.

----------------------------------------------------------------

Changes since v1.2.2 are as follows:

Carl Worth:
      git-add: Add support for --, documentation, and test.
      git-push: Update documentation to describe the no-refspec behavior.

Junio C Hamano:
      format-patch: pretty-print timestamp correctly.
      detect broken alternates.
      pack-objects: reuse data from existing packs.
      pack-objects: finishing touches.
      git-repack: allow passing a couple of flags to pack-objects.
      pack-objects: avoid delta chains that are too long.
      Make "empty ident" error message a bit more helpful.
      Delay "empty ident" errors until they really matter.
      Keep Porcelainish from failing by broken ident after making changes.
      pack-objects eye-candy: finishing touches.
      git-fetch: follow tag only when tracking remote branch.

Nicolas Pitre:
      nicer eye candies for pack-objects
      also adds progress when actually writing a pack

^ permalink raw reply

* What's in git.git
From: Junio C Hamano @ 2006-02-23  2:05 UTC (permalink / raw)
  To: git


* The 'master' branch has these since the last announcement.

Junio C Hamano:
      gitview: ls-remote invocation shellquote safety.
      detect broken alternates.
      git-fetch: follow tag only when tracking remote branch.
      Merge fixes up to GIT 1.2.3

Nicolas Pitre:
      nicer eye candies for pack-objects
      also adds progress when actually writing a pack


* The 'next' branch, in addition, has these (not much changed).

 - git-rm from Carl Worth.  I think this is ready.

 - git-cvsserver from Martin.  I think this is ready.

 - git-annotate and git-blame from Ryan and Friedrik.

 - "thin pack" pack transfer optimization.

 - delta optimization from Nicolas Pitre.


* The 'pu' branch, in addition, has these.

 - "bind commit"

 - delta optimization bits from Nicolas.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23  8:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Eric Wong, git
In-Reply-To: <Pine.LNX.4.63.0602222259480.6682@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/22/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > IPC::Open2 works!
>
> Note that there is a notable decrease in performance in my preliminary
> tests (about 10%).

I'll keep that in mind. But there are places where a safe pipe is unavoidable
(filenames. No amount of careful quoting will save you).

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Junio C Hamano @ 2006-02-23  8:45 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0602230000t58a88af6na1aa7e323dc0179d@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> I'll keep that in mind. But there are places where a safe pipe is unavoidable
> (filenames. No amount of careful quoting will save you).

Huh?

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23  9:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtfmihts.fsf@assigned-by-dhcp.cox.net>

On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
>
> > I'll keep that in mind. But there are places where a safe pipe is unavoidable
> > (filenames. No amount of careful quoting will save you).
>
> Huh?
>

Because you never know what did the next interpreter took for unquoting:
$SHELL, /bin/sh cmd /c, or something else.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23  9:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <81b0412b0602230135w472aa6f3v72980f6f63bb355f@mail.gmail.com>

On 2/23/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
> > "Alex Riesen" <raa.lkml@gmail.com> writes:
> >
> > > I'll keep that in mind. But there are places where a safe pipe is unavoidable
> > > (filenames. No amount of careful quoting will save you).
> >
> > Huh?
>
> Because you never know what did the next interpreter took for unquoting:
> $SHELL, /bin/sh cmd /c, or something else.
>
And that stupid activestate thing actually doesn't use any. Just tried:

  perl -e '$,=" ";open(F, "sleep 1000 ; # @ARGV |") and print <F>'

It passed the whole string "1000 ; # @ARGV" to sleep from $PATH.
It failed to sleep at all, of course. The same code works perfectly on
almost any UNIX system.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Andreas Ericsson @ 2006-02-23  9:48 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <81b0412b0602230141g46dbfaev6baa5083dee2d42@mail.gmail.com>

Alex Riesen wrote:
> On 2/23/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> 
>>On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
>>
>>>"Alex Riesen" <raa.lkml@gmail.com> writes:
>>>
>>>
>>>>I'll keep that in mind. But there are places where a safe pipe is unavoidable
>>>>(filenames. No amount of careful quoting will save you).
>>>
>>>Huh?
>>
>>Because you never know what did the next interpreter took for unquoting:
>>$SHELL, /bin/sh cmd /c, or something else.
>>
> 
> And that stupid activestate thing actually doesn't use any. Just tried:
> 
>   perl -e '$,=" ";open(F, "sleep 1000 ; # @ARGV |") and print <F>'
> 
> It passed the whole string "1000 ; # @ARGV" to sleep from $PATH.
> It failed to sleep at all, of course. The same code works perfectly on
> almost any UNIX system.


Not to be unhelpful or anything, but activestate perl seems to be quite 
a lot of bother. Is it worth supporting it?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23 10:10 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <43FD84EB.3040704@op5.se>

On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> Not to be unhelpful or anything, but activestate perl seems to be quite
> a lot of bother. Is it worth supporting it?

It's not activestate perl actually. It's only one platform it also
_has_ to support.
Is it worth supporting Windows?

^ permalink raw reply

* PATCH: simplify calls to git programs in git-fmt-merge-msg
From: Alex Riesen @ 2006-02-23 10:26 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

It also makes it work on ActiveState Perl.

[-- Attachment #2: 0001-fix-git-fmt-merge-msg.perl-for-activestate-perl.txt --]
[-- Type: text/plain, Size: 1470 bytes --]

---

 git-fmt-merge-msg.perl |   31 +++++--------------------------
 1 files changed, 5 insertions(+), 26 deletions(-)

1c0dd861fa32017cf7bc4226bfa54d390a3fb91e
diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
index c13af48..dae383f 100755
--- a/git-fmt-merge-msg.perl
+++ b/git-fmt-merge-msg.perl
@@ -28,28 +28,13 @@ sub andjoin {
 }
 
 sub repoconfig {
-	my $val;
-	eval {
-		my $pid = open(my $fh, '-|');
-		if (!$pid) {
-			exec('git-repo-config', '--get', 'merge.summary');
-		}
-		($val) = <$fh>;
-		close $fh;
-	};
+	my ($val) = qx{git-repo-config --get merge.summary};
 	return $val;
 }
 
 sub current_branch {
-	my $fh;
-	my $pid = open($fh, '-|');
-	die "$!" unless defined $pid;
-	if (!$pid) {
-	    exec('git-symbolic-ref', 'HEAD') or die "$!";
-	}
-	my ($bra) = <$fh>;
+	my ($bra) = qx{git-symbolic-ref HEAD};
 	chomp($bra);
-	close $fh or die "$!";
 	$bra =~ s|^refs/heads/||;
 	if ($bra ne 'master') {
 		$bra = " into $bra";
@@ -61,18 +46,12 @@ sub current_branch {
 
 sub shortlog {
 	my ($tip) = @_;
-	my ($fh, @result);
-	my $pid = open($fh, '-|');
-	die "$!" unless defined $pid;
-	if (!$pid) {
-	    exec('git-log', '--topo-order',
-		 '--pretty=oneline', $tip, '^HEAD') or die "$!";
-	}
-	while (<$fh>) {
+	my @result;
+	foreach ( qx{git-log --topo-order --pretty=oneline $tip ^HEAD} ) {
 		s/^[0-9a-f]{40}\s+//;
 		push @result, $_;
 	}
-	close $fh or die "$!";
+	die "git-log failed\n" if $?;
 	return @result;
 }
 
-- 
1.2.3.g6ae0e


^ permalink raw reply related

* Re: PATCH: simplify calls to git programs in git-fmt-merge-msg
From: Junio C Hamano @ 2006-02-23 10:35 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0602230226j12e88682h303d466a273bec09@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> It also makes it work on ActiveState Perl.
>
> ---

ActiveState or not, this simplification is pretty much welcomed.
The only problem I _might_ have later is with shortlog, though I
have not looked at it closely yet.

Thanks.

^ permalink raw reply

* Re: PATCH: simplify calls to git programs in git-fmt-merge-msg
From: Alex Riesen @ 2006-02-23 10:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vslqaicqj.fsf@assigned-by-dhcp.cox.net>

On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
>
> > It also makes it work on ActiveState Perl.
> >
> > ---
>
> ActiveState or not, this simplification is pretty much welcomed.
> The only problem I _might_ have later is with shortlog, though I
> have not looked at it closely yet.

I tested it a bit.
I suppose you can have two following concerns there:
- $tip - it seem to be always an sha1
- perl can decide to fetch whole output of git-log into memory.
  I heard it optimizes such cases into reading only when needed...

^ permalink raw reply

* fix t5600-clone-fail-cleanup.sh on windows
From: Alex Riesen @ 2006-02-23 11:25 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 174 bytes --]

In windows you cannot remove current or opened directory,
an opened file, a running program, a loaded library, etc...

---
I still wonder how they managed to survive...

[-- Attachment #2: 0001-fix-t5600-clone-fail-cleanup.sh-on-windows-canot-remove-current-dir.txt --]
[-- Type: text/plain, Size: 706 bytes --]

>From nobody Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Thu Feb 23 11:49:31 2006 +0100
Subject: [PATCH] fix t5600-clone-fail-cleanup.sh on windows

---

 git-clone.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

3453c0df0d815580c5e7aaeb67bfc54b5b0954c6
diff --git a/git-clone.sh b/git-clone.sh
index dc0ad55..0c60e0b 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -118,7 +118,7 @@ dir="$2"
 [ -e "$dir" ] && echo "$dir already exists." && usage
 mkdir -p "$dir" &&
 D=$(cd "$dir" && pwd) &&
-trap 'err=$?; rm -r $D; exit $err' exit
+trap 'err=$?; cd ..; rm -r $D; exit $err' exit
 case "$bare" in
 yes) GIT_DIR="$D" ;;
 *) GIT_DIR="$D/.git" ;;
-- 
1.2.3.g6ae0e


^ permalink raw reply related

* Re: PATCH: simplify calls to git programs in git-fmt-merge-msg
From: Johannes Schindelin @ 2006-02-23 12:36 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Git Mailing List
In-Reply-To: <81b0412b0602230226j12e88682h303d466a273bec09@mail.gmail.com>

Hi,

On Thu, 23 Feb 2006, Alex Riesen wrote:

> It also makes it work on ActiveState Perl.

Thank you for teaching me this very valuable construct: qx{ ... }.

Ciao,
Dscho

^ permalink raw reply

* Re: PATCH: simplify calls to git programs in git-fmt-merge-msg
From: Alex Riesen @ 2006-02-23 12:40 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0602231335420.3726@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/23/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > It also makes it work on ActiveState Perl.
>
> Thank you for teaching me this very valuable construct: qx{ ... }.

You're welcome. That's the same as shells old `` (backquote).

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Andreas Ericsson @ 2006-02-23 13:29 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <81b0412b0602230210r3ffe6e2dta5dc86d6516692b9@mail.gmail.com>

Alex Riesen wrote:
> On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> 
>>Not to be unhelpful or anything, but activestate perl seems to be quite
>>a lot of bother. Is it worth supporting it?
> 
> 
> It's not activestate perl actually. It's only one platform it also
> _has_ to support.
> Is it worth supporting Windows?


With or without cygwin? With cygwin, I'd say "yes, unless it makes 
things terribly difficult to maintain and so long as we don't take 
performance hits on unices". Without cygwin, I'd say "What? It runs on 
windows?".

If we claim to support windows but do a poor job of it, no-one else will 
start working on a windows-port. If we don't claim to support windows 
but say that "it's known to work with cygwin, although be aware of these 
performance penalties...", eventually someone will come along with their 
shiny Visual Express and hack up support for it, even if some tools will 
be missing and others unnecessarily complicated.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] Port git-annotate to qx{} syntax, and make default output more like CVS
From: Johannes Schindelin @ 2006-02-23 13:53 UTC (permalink / raw)
  To: git, junkio


This uses the qx{} syntax Alex digged up, and it makes output without "-l"
quite similar to CVS's annotate output.

Oh, and it ignores a dirty work file. In fact, it even ignores a clean 
work file, but fetches a clean version from the repository.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 git-annotate.perl |   82 +++++++++++++++++++++++++----------------------------
 1 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/git-annotate.perl b/git-annotate.perl
index 3800c46..e9e5a3d 100755
--- a/git-annotate.perl
+++ b/git-annotate.perl
@@ -37,14 +37,14 @@ my @stack = (
 
 our (@lineoffsets, @pendinglineoffsets);
 our @filelines = ();
-open(F,"<",$filename)
-	or die "Failed to open filename: $!";
 
-while(<F>) {
+our $blob_sha1 = qx{git-ls-tree HEAD "$filename"};
+$blob_sha1 =~ s/^.*blob (\S*).*$/$1/;
+foreach (qx{git-cat-file blob $blob_sha1}) {
 	chomp;
 	push @filelines, $_;
 }
-close(F);
+!$? or exit $?;
 our $leftover_lines = @filelines;
 our %revs;
 our @revqueue;
@@ -91,13 +91,12 @@ foreach my $l (@filelines) {
 		if (!$opt_l && length($rev) > 8) {
 			$rev = substr($rev,0,8);
 		}
+		printf("%s\t(%10s\t%s):%s\n", $rev, $committer,
+			format_date($date), $output);
 	} else {
-		$output = $l;
-		($rev, $committer, $date) = ('unknown', 'unknown', 'unknown');
+		printf("%s\t(%10s\t%10s\t%d)%s\n", $rev, $committer,
+			format_date($date), $i++, $output);
 	}
-
-	printf("%s\t(%10s\t%10s\t%d)%s\n", $rev, $committer,
-		format_date($date), $i++, $output);
 }
 
 sub init_claim {
@@ -143,20 +142,24 @@ sub handle_rev {
 sub git_rev_list {
 	my ($rev, $file) = @_;
 
+	my @revs;
 	if ($opt_S) {
 		open(P, '<' . $opt_S);
+		while(my $line = <P>) {
+			chomp $line;
+			my ($rev, @parents) = split /\s+/, $line;
+			push @revs, [ $rev, @parents ];
+		}
+		close(P);
 	} else {
-		open(P,"-|","git-rev-list","--parents","--remove-empty",$rev,"--",$file)
-			or die "Failed to exec git-rev-list: $!";
-	}
-
-	my @revs;
-	while(my $line = <P>) {
-		chomp $line;
-		my ($rev, @parents) = split /\s+/, $line;
-		push @revs, [ $rev, @parents ];
+		foreach (qx{git-rev-list --parents --remove-empty $rev -- $file}
+) {
+			chomp $_;
+			my ($rev, @parents) = split /\s+/, $_;
+			push @revs, [ $rev, @parents ];
+		}
+		!$? or exit $?;
 	}
-	close(P);
 
 	printf("0 revs found for rev %s (%s)\n", $rev, $file) if (@revs == 0);
 	return @revs;
@@ -165,13 +168,11 @@ sub git_rev_list {
 sub find_parent_renames {
 	my ($rev, $file) = @_;
 
-	open(P,"-|","git-diff-tree", "-M50", "-r","--name-status", "-z","$rev")
-		or die "Failed to exec git-diff: $!";
-
 	local $/ = "\0";
 	my %bound;
 	my $junk = <P>;
-	while (my $change = <P>) {
+	foreach (qx{git-diff-tree -M50 -r --name-status -z $rev}) {
+		my $change = $_;
 		chomp $change;
 		my $filename = <P>;
 		chomp $filename;
@@ -189,7 +190,7 @@ sub find_parent_renames {
 			}
 		}
 	}
-	close(P);
+	!$? or exit $?;
 
 	return \%bound;
 }
@@ -198,15 +199,12 @@ sub find_parent_renames {
 sub git_find_parent {
 	my ($rev, $filename) = @_;
 
-	open(REVPARENT,"-|","git-rev-list","--remove-empty", "--parents","--max-count=1","$rev","--",$filename)
-		or die "Failed to open git-rev-list to find a single parent: $!";
+	my $parentline = qx{git-rev-list --remove-empty --parents --max-count=1 $rev -- $filename};
+	!$? or die "Failed to open git-rev-list to find a single parent: $!";
 
-	my $parentline = <REVPARENT>;
 	chomp $parentline;
 	my ($revfound,$parent) = split m/\s+/, $parentline;
 
-	close(REVPARENT);
-
 	return $parent;
 }
 
@@ -217,17 +215,13 @@ sub git_diff_parse {
 	my ($parent, $rev, %revinfo) = @_;
 
 	my ($ri, $pi) = (0,0);
-	open(DIFF,"-|","git-diff-tree","-M","-p",$rev,$parent,"--",
-			$revs{$rev}{'filename'}, $revs{$parent}{'filename'})
-		or die "Failed to call git-diff for annotation: $!";
-
 	my $slines = $revs{$rev}{'lines'};
 	my @plines;
 
 	my $gotheader = 0;
 	my ($remstart, $remlength, $addstart, $addlength);
 	my ($hunk_start, $hunk_index, $hunk_adds);
-	while(<DIFF>) {
+	foreach (qx{git-diff-tree -M -p $rev $parent -- $revs{$rev}{'filename'} $revs{$parent}{'filename'}}) {
 		chomp;
 		if (m/^@@ -(\d+),(\d+) \+(\d+),(\d+)/) {
 			($remstart, $remlength, $addstart, $addlength) = ($1, $2, $3, $4);
@@ -279,7 +273,7 @@ sub git_diff_parse {
 		}
 		$hunk_index++;
 	}
-	close(DIFF);
+	!$? or exit $?;
 	for (my $i = $ri; $i < @{$slines} ; $i++) {
 		push @plines, $slines->[$ri++];
 	}
@@ -300,15 +294,12 @@ sub git_cat_file {
 	my $blobline = `git-ls-tree $parent $filename`;
 	my ($mode, $type, $blob, $tfilename) = split(/\s+/, $blobline, 4);
 
-	open(C,"-|","git-cat-file", "blob", $blob)
-		or die "Failed to git-cat-file blob $blob (rev $parent, file $filename): " . $!;
-
 	my @lines;
-	while(<C>) {
+	foreach (qx{git-cat-file blob $blob}) {
 		chomp;
 		push @lines, $_;
 	}
-	close(C);
+	!$? or die "Failed to git-cat-file blob $blob (rev $parent, file $filename).";
 
 	return @lines;
 }
@@ -325,11 +316,9 @@ sub claim_line {
 
 sub git_commit_info {
 	my ($rev) = @_;
-	open(COMMIT, "-|","git-cat-file", "commit", $rev)
-		or die "Failed to call git-cat-file: $!";
 
 	my %info;
-	while(<COMMIT>) {
+	foreach (qx{git-cat-file commit $rev}) {
 		chomp;
 		last if (length $_ == 0);
 
@@ -343,7 +332,7 @@ sub git_commit_info {
 			$info{'committer_date'} = $3;
 		}
 	}
-	close(COMMIT);
+	!$? or exit $?;
 
 	return %info;
 }
@@ -351,6 +340,11 @@ sub git_commit_info {
 sub format_date {
 	my ($timestamp, $timezone) = split(' ', $_[0]);
 
+	if (!$opt_l) {
+		return strftime("%Y-%b-%d", gmtime($timestamp));
+	}
+
 	return strftime("%Y-%m-%d %H:%M:%S " . $timezone, gmtime($timestamp));
 }
 
+

^ permalink raw reply related

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23 14:07 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <43FDB8CC.5000503@op5.se>

On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> >
> >>Not to be unhelpful or anything, but activestate perl seems to be quite
> >>a lot of bother. Is it worth supporting it?
> >
> >
> > It's not activestate perl actually. It's only one platform it also
> > _has_ to support.
> > Is it worth supporting Windows?
>
> With or without cygwin? With cygwin, I'd say "yes, unless it makes
> things terribly difficult to maintain and so long as we don't take
> performance hits on unices". Without cygwin, I'd say "What? It runs on
> windows?".

There not much difference with or without cygwin. The penalties of
doing any kind of support for it will pile up (as they started to do
with pipes).
Someday we'll have to start dropping features on Windows or restrict them
beyond their usefullness. The fork emulation in cygwin isn't perfect,
signals do not work reliably (if at all), filesystem is slow and locked down,
and exec-attribute is NOT really useful even on NTFS (it is somehow related
to execute permission and open files. I still cannot figure out how exactly
are they related).

> If we claim to support windows but do a poor job of it, no-one else will
> start working on a windows-port. If we don't claim to support windows
> but say that "it's known to work with cygwin, although be aware of these
> performance penalties...", eventually someone will come along with their
> shiny Visual Express and hack up support for it, even if some tools will
> be missing and others unnecessarily complicated.

That seem to be the case, except for shiny.
(I really don't know what could possibly mean by that. It stinks, smears,
and sometimes bounces. Never saw it shining).

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Andreas Ericsson @ 2006-02-23 14:22 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <81b0412b0602230607n22146a77k36929f0ad9e44d53@mail.gmail.com>

Alex Riesen wrote:
> On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> 
>>If we claim to support windows but do a poor job of it, no-one else will
>>start working on a windows-port. If we don't claim to support windows
>>but say that "it's known to work with cygwin, although be aware of these
>>performance penalties...", eventually someone will come along with their
>>shiny Visual Express and hack up support for it, even if some tools will
>>be missing and others unnecessarily complicated.
> 
> 
> That seem to be the case, except for shiny.
> (I really don't know what could possibly mean by that. It stinks, smears,
> and sometimes bounces. Never saw it shining).
> 

The logo has a little glint thing on it, like those things that go 
'ting' on a front tooth in commercials for toothpaste and particularly 
healthy chewing gum.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] Convert open("-|") to qx{} calls
From: Johannes Schindelin @ 2006-02-23 14:33 UTC (permalink / raw)
  To: git, junkio


Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Since of these 4, I only use cvsimport myself, I could only test
	that. Could someone who uses the others give them a hard beating?

 git-cvsimport.perl  |   64 +++++++++++++++++++--------------------------------
 git-rerere.perl     |    9 ++-----
 git-send-email.perl |    9 ++-----
 git-svnimport.perl  |   62 ++++++++++++++-----------------------------------
 4 files changed, 46 insertions(+), 98 deletions(-)

b37d21c223fdc0ef7fc6af889432f6b51ac82992
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index b46469a..da009f2 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -478,9 +478,9 @@ unless(-d $git_dir) {
 		       "Either use the correct '-o branch' option,\n".
 		       "or import to a new repository.\n";
 
-	open(F, "git-symbolic-ref HEAD |") or
-		die "Cannot run git-symbolic-ref: $!\n";
-	chomp ($last_branch = <F>);
+	$last_branch = qx{git-symbolic-ref HEAD};
+	!$? or exit $?;
+	chomp ($last_branch);
 	$last_branch = basename($last_branch);
 	close(F);
 	unless($last_branch) {
@@ -516,13 +516,12 @@ EOM
 			or die "Bad head branch: $head: $!\n";
 		chomp(my $ftag = <F>);
 		close(F);
-		open(F,"git-cat-file commit $ftag |");
-		while(<F>) {
+		foreach (qx{git-cat-file commit $ftag}) {
 			next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;
 			$branch_date{$head} = $1;
 			last;
 		}
-		close(F);
+		!$? or exit $?;
 	}
 	closedir(D);
 }
@@ -538,24 +537,21 @@ if ($opt_A) {
 	write_author_info("$git_dir/cvs-authors");
 }
 
-my $pid = open(CVS,"-|");
-die "Cannot fork: $!\n" unless defined $pid;
-unless($pid) {
-	my @opt;
-	@opt = split(/,/,$opt_p) if defined $opt_p;
-	unshift @opt, '-z', $opt_z if defined $opt_z;
-	unshift @opt, '-q'         unless defined $opt_v;
-	unless (defined($opt_p) && $opt_p =~ m/--no-cvs-direct/) {
-		push @opt, '--cvs-direct';
-	}
-	if ($opt_P) {
-	    exec("cat", $opt_P);
-	} else {
-	    exec("cvsps","--norc",@opt,"-u","-A",'--root',$opt_d,$cvs_tree);
-	    die "Could not start cvsps: $!\n";
-	}
+my @opt;
+@opt = split(/,/,$opt_p) if defined $opt_p;
+unshift @opt, '-z', $opt_z if defined $opt_z;
+unshift @opt, '-q'         unless defined $opt_v;
+unless (defined($opt_p) && $opt_p =~ m/--no-cvs-direct/) {
+	push @opt, '--cvs-direct';
 }
 
+my @input;
+if ($opt_P) {
+    @input = qx{cat $opt_P};
+} else {
+    @input = qx{cvsps --norc opt -u -A --root $opt_d $cvs_tree};
+    !$? or exit $?;
+}
 
 ## cvsps output:
 #---------------------
@@ -603,17 +599,11 @@ my $commit = sub {
 		die "Cannot add files: $?\n" if $?;
 	}
 
-	$pid = open(C,"-|");
-	die "Cannot fork: $!" unless defined $pid;
-	unless($pid) {
-		exec("git-write-tree");
-		die "Cannot exec git-write-tree: $!\n";
-	}
-	chomp(my $tree = <C>);
+	my $tree = qx{git-write-tree};
+	!$? or exit $?;
+	chomp($tree);
 	length($tree) == 40
 		or die "Cannot get tree id ($tree): $!\n";
-	close(C)
-		or die "Error running git-write-tree: $?\n";
 	print "Tree ID $tree\n" if $opt_v;
 
 	my $parent = "";
@@ -734,7 +724,7 @@ my $commit = sub {
 	}
 };
 
-while(<CVS>) {
+foreach (@input) {
 	chomp;
 	if($state == 0 and /^-+$/) {
 		$state = 1;
@@ -846,15 +836,9 @@ while(<CVS>) {
 			print "Drop $fn\n" if $opt_v;
 		} else {
 			print "".($init ? "New" : "Update")." $fn: $size bytes\n" if $opt_v;
-			my $pid = open(my $F, '-|');
-			die $! unless defined $pid;
-			if (!$pid) {
-			    exec("git-hash-object", "-w", $tmpname)
-				or die "Cannot create object: $!\n";
-			}
-			my $sha = <$F>;
+			my $sha = qx{git-hash-object -w $tmpname};
+			!$? or exit $?;
 			chomp $sha;
-			close $F;
 			my $mode = pmode($cvs->{'mode'});
 			push(@new,[$mode, $sha, $fn]); # may be resurrected!
 		}
diff --git a/git-rerere.perl b/git-rerere.perl
index d3664ff..0dd04c5 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -131,20 +131,15 @@ sub record_preimage {
 sub find_conflict {
 	my $in;
 	local $/ = "\0";
-	my $pid = open($in, '-|');
-	die "$!" unless defined $pid;
-	if (!$pid) {
-		exec(qw(git ls-files -z -u)) or die "$!: ls-files";
-	}
 	my %path = ();
 	my @path = ();
-	while (<$in>) {
+	foreach (qx{git-ls-files -z -u}) {
 		chomp;
 		my ($mode, $sha1, $stage, $path) =
 		    /^([0-7]+) ([0-9a-f]{40}) ([123])\t(.*)$/s;
 		$path{$path} |= (1 << $stage);
 	}
-	close $in;
+	!$? or exit $?;
 	while (my ($path, $status) = each %path) {
 		if ($status == 14) { push @path, $path; }
 	}
diff --git a/git-send-email.perl b/git-send-email.perl
index b0d095b..bd8fae6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -62,13 +62,8 @@ my $rc = GetOptions("from=s" => \$from,
 sub gitvar {
     my ($var) = @_;
     my $fh;
-    my $pid = open($fh, '-|');
-    die "$!" unless defined $pid;
-    if (!$pid) {
-	exec('git-var', $var) or die "$!";
-    }
-    my ($val) = <$fh>;
-    close $fh or die "$!";
+    my ($val) = qx{git-var $var};
+    !$? or exit $?;
     chomp($val);
     return $val;
 }
diff --git a/git-svnimport.perl b/git-svnimport.perl
index ee2940f..6094a11 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -218,11 +218,10 @@ unless(-d $git_dir) {
 	-f "$git_dir/svn2git"
 		or die "'$git_dir/svn2git' does not exist.\n".
 		       "You need that file for incremental imports.\n";
-	open(F, "git-symbolic-ref HEAD |") or
-		die "Cannot run git-symbolic-ref: $!\n";
-	chomp ($last_branch = <F>);
+	$last_brach = qx{git-symbolic-ref HEAD};
+	!$? or exit $?;
+	chomp $last_branch;
 	$last_branch = basename($last_branch);
-	close(F);
 	unless($last_branch) {
 		warn "Cannot read the last branch name: $! -- assuming 'master'\n";
 		$last_branch = "master";
@@ -321,15 +320,9 @@ sub get_file($$$) {
 		return undef unless defined $name;
 	}
 
-	my $pid = open(my $F, '-|');
-	die $! unless defined $pid;
-	if (!$pid) {
-	    exec("git-hash-object", "-w", $name)
-		or die "Cannot create object: $!\n";
-	}
-	my $sha = <$F>;
+	my $sha = qx{git-hash-object -w $name};
+	!$? or exit $?;
 	chomp $sha;
-	close $F;
 	unlink $name;
 	my $mode = "0644"; # SV does not seem to store any file modes
 	return [$mode, $sha, $path];
@@ -401,14 +394,8 @@ sub copy_path($$$$$$$$) {
 			$srcpath =~ s#/*$#/#;
 	}
 	
-	my $pid = open my $f,'-|';
-	die $! unless defined $pid;
-	if (!$pid) {
-		exec("git-ls-tree","-r","-z",$gitrev,$srcpath)
-			or die $!;
-	}
 	local $/ = "\0";
-	while(<$f>) {
+	foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
 		chomp;
 		my($m,$p) = split(/\t/,$_,2);
 		my($mode,$type,$sha1) = split(/ /,$m);
@@ -420,8 +407,7 @@ sub copy_path($$$$$$$$) {
 		}
 		push(@$new,[$mode,$sha1,$p]);	
 	}
-	close($f) or
-		print STDERR "$newrev:$newbranch: could not list files in $oldpath \@ $rev\n";
+	!$? or exit $?;
 }
 
 sub commit {
@@ -472,9 +458,8 @@ sub commit {
 
 	my $rev;
 	if($revision > $opt_s and defined $parent) {
-		open(H,"git-rev-parse --verify $parent |");
-		$rev = <H>;
-		close(H) or do {
+		$rev = qx{git-rev-parse --verify $parent};
+		!$? or do {
 			print STDERR "$revision: cannot find commit '$parent'!\n";
 			return;
 		};
@@ -555,25 +540,20 @@ sub commit {
 		}
 
 		while(@old) {
-			my @o1;
+			my @o2;
 			if(@old > 55) {
-				@o1 = splice(@old,0,50);
+				@o2 = splice(@old,0,50);
 			} else {
-				@o1 = @old;
+				@o2 = @old;
 				@old = ();
 			}
-			my $pid = open my $F, "-|";
-			die "$!" unless defined $pid;
-			if (!$pid) {
-				exec("git-ls-files", "-z", @o1) or die $!;
-			}
-			@o1 = ();
+			my @o1 = ();
 			local $/ = "\0";
-			while(<$F>) {
+			foreach (qx{git-ls-files -z @o1}) {
 				chomp;
 				push(@o1,$_);
 			}
-			close($F);
+			!$? or exit $?;
 
 			while(@o1) {
 				my @o2;
@@ -600,17 +580,11 @@ sub commit {
 			die "Cannot add files: $?\n" if $?;
 		}
 
-		my $pid = open(C,"-|");
-		die "Cannot fork: $!" unless defined $pid;
-		unless($pid) {
-			exec("git-write-tree");
-			die "Cannot exec git-write-tree: $!\n";
-		}
-		chomp(my $tree = <C>);
+		my $tree = qx{git-write-tree};
+		!$? or exit $?;
+		chomp($tree);
 		length($tree) == 40
 			or die "Cannot get tree id ($tree): $!\n";
-		close(C)
-			or die "Error running git-write-tree: $?\n";
 		print "Tree ID $tree\n" if $opt_v;
 
 		my $pr = IO::Pipe->new() or die "Cannot open pipe: $!\n";
-- 
1.2.3.gb37d

^ permalink raw reply related

* Re: [PATCH] Port git-annotate to qx{} syntax, and make default output more like CVS
From: Alex Riesen @ 2006-02-23 15:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0602231442390.24946@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/23/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> This uses the qx{} syntax Alex digged up, and it makes output without "-l"
> quite similar to CVS's annotate output.
>

Works here.

^ permalink raw reply

* FYI: git-am allows creation of empty commits.
From: Eric W. Biederman @ 2006-02-23 15:33 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano


Currently I am using: git version 1.2.2.g709a.

I have been rebasing and refactoring a number of patchsets using git.

Frequently because of prior edits I will git a patch that does not
apply at all.  So all I get is the creation of the .dotest directory.
The automatic merge logic does not kick in, so I have to manually
call patch -p1 and resolve the differences but hand.  There everything
is expected and fine.  Although it might have been nice to have one
of those failed merge indicators in the index.

After fixing it up and doing all of my edits I occasionally forget
the git-update-index step, before calling git-am --resolved.  This
proceeds along it's merry way and creates an empty commit.

Is this the expected correct behavior or should git-am complain about
a situation like that?

Eric

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Alex Riesen @ 2006-02-23 15:38 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0602231532470.29635@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/23/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>       Since of these 4, I only use cvsimport myself, I could only test
>       that. Could someone who uses the others give them a hard beating?

I can't really test them (no svn and cvs, and locked down network), but I took
a look at the patches. Hope it helps.

git-cvsimport:

> -               open(F,"git-cat-file commit $ftag |");
> -               while(<F>) {
> +               foreach (qx{git-cat-file commit $ftag}) {
>                         next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;

Are you sure you don't need quoting/safe pipe here?
Or is it a CVS tag?

> +} else {
> +    @input = qx{cvsps --norc opt -u -A --root $opt_d $cvs_tree};
> +    !$? or exit $?;

Same here. $cvs_tree can contain any filesystem-allowed character.

git-svnimport:

> -                       my $sha = <$F>;
> +                       my $sha = qx{git-hash-object -w $tmpname};
> +                       !$? or exit $?;

Is $tmpname safe?

> -       my $sha = <$F>;
> +       my $sha = qx{git-hash-object -w $name};
> +       !$? or exit $?;

Is $name safe?

> -       while(<$f>) {
> +       foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
>                 chomp;

Is $srcpath safe?

> -                       while(<$F>) {
> +                       foreach (qx{git-ls-files -z @o1}) {

@o1 must contain filenames. Can be dangerous

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Randal L. Schwartz @ 2006-02-23 16:07 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Schindelin, git, junkio
In-Reply-To: <81b0412b0602230738s3445bd86h2d1d670e0ef5daed@mail.gmail.com>

>>>>> "Alex" == Alex Riesen <raa.lkml@gmail.com> writes:

Alex> Is $tmpname safe?

>> -       my $sha = <$F>;
>> +       my $sha = qx{git-hash-object -w $name};
>> +       !$? or exit $?;

Alex> Is $name safe?

>> -       while(<$f>) {
>> +       foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
>> chomp;

Alex> Is $srcpath safe?

>> -                       while(<$F>) {
>> +                       foreach (qx{git-ls-files -z @o1}) {

Alex> @o1 must contain filenames. Can be dangerous

Convert all of these to use "safe_qx" (perl 5.6 compatible):

    sub safe_qx {
      defined (my $pid = open my $kid, "-|") or die "Cannot fork: $!";
      unless ($pid) { # child does:
        exec @_;
        die "Cannot exec @_: $!";
      }
      my $result = do { local $/; <$kid> };
      close $kid;                   # sets $?
      return $result;
    }

my $result = safe_qx('some shell command');
my $other_result = safe_qx('git-ls-tree', '-r', '-z', $gitrev, $srcpath);

Args are safe, as if being passed to system/exec, so a single arg
can be a shell command, multiargs are passed arg-by-arg to a single
exec target.  $? is set correctly.

-- 
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!

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox