Git development
 help / color / mirror / Atom feed
* [PATCH] nicer eye candies for pack-objects
From: Nicolas Pitre @ 2006-02-22 21:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This provides a stable and simpler progress reporting mechanism that 
updates progress as often as possible but accurately not updating more 
than once a second.  The deltification phase is also made more 
interesting to watch (since repacking a big repository and only seeing a 
dot appear once every many seconds is rather boring and doesn't provide 
much food for anticipation).

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/pack-objects.c b/pack-objects.c
index 0c9f4c9..7c89dc7 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -4,6 +4,7 @@
 #include "pack.h"
 #include "csum-file.h"
 #include <sys/time.h>
+#include <signal.h>
 
 static const char pack_usage[] = "git-pack-objects [-q] [--no-reuse-delta] [--non-empty] [--local] [--incremental] [--window=N] [--depth=N] {--stdout | base-name} < object-list";
 
@@ -661,17 +661,22 @@ static int try_delta(struct unpacked *cu
 	return 0;
 }
 
+static volatile int progress_update = 0;
+static void progress_interval(int signum)
+{
+	signal(SIGALRM, progress_interval);
+	progress_update = 1;
+}
+
 static void find_deltas(struct object_entry **list, int window, int depth)
 {
 	int i, idx;
 	unsigned int array_size = window * sizeof(struct unpacked);
 	struct unpacked *array = xmalloc(array_size);
-	int eye_candy;
 
 	memset(array, 0, array_size);
 	i = nr_objects;
 	idx = 0;
-	eye_candy = i - (nr_objects / 20);
 
 	while (--i >= 0) {
 		struct object_entry *entry = list[i];
@@ -680,9 +685,10 @@ static void find_deltas(struct object_en
 		char type[10];
 		int j;
 
-		if (progress && i <= eye_candy) {
-			eye_candy -= nr_objects / 20;
-			fputc('.', stderr);
+		if (progress_update || i == 0) {
+			fprintf(stderr, "Deltifying (%d %d%%)\r",
+				nr_objects-i, (nr_objects-i) * 100/nr_objects);
+			progress_update = 0;
 		}
 
 		if (entry->delta)
@@ -714,6 +720,9 @@ static void find_deltas(struct object_en
 			idx = 0;
 	}
 
+	if (progress)
+		fputc('\n', stderr);
+
 	for (i = 0; i < window; ++i)
 		free(array[i].data);
 	free(array);
@@ -721,17 +730,10 @@ static void find_deltas(struct object_en
 
 static void prepare_pack(int window, int depth)
 {
-	if (progress)
-		fprintf(stderr, "Packing %d objects", nr_objects);
 	get_object_details();
-	if (progress)
-		fputc('.', stderr);
-
 	sorted_by_type = create_sorted_list(type_size_sort);
 	if (window && depth)
 		find_deltas(sorted_by_type, window+1, depth);
-	if (progress)
-		fputc('\n', stderr);
 	write_pack_file();
 }
 
@@ -796,10 +798,6 @@ int main(int argc, char **argv)
 	int window = 10, depth = 10, pack_to_stdout = 0;
 	struct object_entry **list;
 	int i;
-	struct timeval prev_tv;
-	int eye_candy = 0;
-	int eye_candy_incr = 500;
-
 
 	setup_git_directory();
 
@@ -856,30 +854,25 @@ int main(int argc, char **argv)
 		usage(pack_usage);
 
 	prepare_packed_git();
+
 	if (progress) {
+		struct itimerval v;
+		v.it_interval.tv_sec = 1;
+		v.it_interval.tv_usec = 0;
+		v.it_value = v.it_interval;
+		signal(SIGALRM, progress_interval);
+		setitimer(ITIMER_REAL, &v, NULL);
 		fprintf(stderr, "Generating pack...\n");
-		gettimeofday(&prev_tv, NULL);
 	}
+
 	while (fgets(line, sizeof(line), stdin) != NULL) {
 		unsigned int hash;
 		char *p;
 		unsigned char sha1[20];
 
-		if (progress && (eye_candy <= nr_objects)) {
+		if (progress_update) {
 			fprintf(stderr, "Counting objects...%d\r", nr_objects);
-			if (eye_candy && (50 <= eye_candy_incr)) {
-				struct timeval tv;
-				int time_diff;
-				gettimeofday(&tv, NULL);
-				time_diff = (tv.tv_sec - prev_tv.tv_sec);
-				time_diff <<= 10;
-				time_diff += (tv.tv_usec - prev_tv.tv_usec);
-				if ((1 << 9) < time_diff)
-					eye_candy_incr += 50;
-				else if (50 < eye_candy_incr)
-					eye_candy_incr -= 50;
-			}
-			eye_candy += eye_candy_incr;
+			progress_update = 0;
 		}
 		if (get_sha1_hex(line, sha1))
 			die("expected sha1, got garbage:\n %s", line);

^ permalink raw reply related

* [PATCH] git-fetch: follow tag only when tracking remote branch.
From: Junio C Hamano @ 2006-02-22 21:12 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Jan Harkes
In-Reply-To: <7vacckt6vr.fsf@assigned-by-dhcp.cox.net>

Unless --no-tags flag was given, git-fetch tried to always
follow remote tags that point at the commits we picked up.

It is not very useful to pick up tags from remote unless storing
the fetched branch head in a local tracking branch.  This is
especially true if the fetch is done to merge the remote branch
into our current branch as one-shot basis (i.e. "please pull"),
and is even harmful if the remote repository has many irrelevant
tags.

This proposed update disables the automated tag following unless
we are storing the a fetched branch head in a local tracking
branch.

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

---

 * Likes, dislikes?

 git-fetch.sh |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

f41b73b4c2e0313d1638261ed87f0921e47904b2
diff --git a/git-fetch.sh b/git-fetch.sh
index b4325d9..fcc24f8 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -368,20 +368,25 @@ fetch_main "$reflist"
 # automated tag following
 case "$no_tags$tags" in
 '')
-	taglist=$(IFS=" " &&
-	git-ls-remote $upload_pack --tags "$remote" |
-	sed -ne 's|^\([0-9a-f]*\)[ 	]\(refs/tags/.*\)^{}$|\1 \2|p' |
-	while read sha1 name
-	do
-		test -f "$GIT_DIR/$name" && continue
-	  	git-check-ref-format "$name" || {
-			echo >&2 "warning: tag ${name} ignored"
-			continue
-		}
-		git-cat-file -t "$sha1" >/dev/null 2>&1 || continue
-		echo >&2 "Auto-following $name"
-		echo ".${name}:${name}"
-	done)
+	case "$reflist" in
+	*:refs/*)
+		# effective only when we are following remote branch
+		# using local tracking branch.
+		taglist=$(IFS=" " &&
+		git-ls-remote $upload_pack --tags "$remote" |
+		sed -ne 's|^\([0-9a-f]*\)[ 	]\(refs/tags/.*\)^{}$|\1 \2|p' |
+		while read sha1 name
+		do
+			test -f "$GIT_DIR/$name" && continue
+			git-check-ref-format "$name" || {
+				echo >&2 "warning: tag ${name} ignored"
+				continue
+			}
+			git-cat-file -t "$sha1" >/dev/null 2>&1 || continue
+			echo >&2 "Auto-following $name"
+			echo ".${name}:${name}"
+		done)
+	esac
 	case "$taglist" in
 	'') ;;
 	?*)
-- 
1.2.2.ga35e

^ permalink raw reply related

* Re: [PATCH] git-fetch: follow tag only when tracking remote branch.
From: Andreas Ericsson @ 2006-02-22 21:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git, Jan Harkes
In-Reply-To: <7vfymbm72x.fsf_-_@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Unless --no-tags flag was given, git-fetch tried to always
> follow remote tags that point at the commits we picked up.
> 
> It is not very useful to pick up tags from remote unless storing
> the fetched branch head in a local tracking branch.  This is
> especially true if the fetch is done to merge the remote branch
> into our current branch as one-shot basis (i.e. "please pull"),
> and is even harmful if the remote repository has many irrelevant
> tags.
> 
> This proposed update disables the automated tag following unless
> we are storing the a fetched branch head in a local tracking
> branch.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> 
> ---
> 
>  * Likes, dislikes?
> 

Likes a lot. This is a Good Thing.

-- 
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: Johannes Schindelin @ 2006-02-22 22:00 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Eric Wong, git
In-Reply-To: <81b0412b0602220835p4c4243edm145ee827eb706121@mail.gmail.com>

Hi,

On Wed, 22 Feb 2006, Alex Riesen wrote:

> IPC::Open2 works!

Note that there is a notable decrease in performance in my preliminary 
tests (about 10%).

Ciao,
Dscho

^ permalink raw reply

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 22 Feb 2006, Alex Riesen wrote:
>
>> IPC::Open2 works!
>
> Note that there is a notable decrease in performance in my preliminary 
> tests (about 10%).

Doesn't open(F, "| foo bar") or open(F, "foo bar |") with
careful shell quoting work?

^ permalink raw reply

* Re: [PATCH] nicer eye candies for pack-objects
From: Junio C Hamano @ 2006-02-22 22:27 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0602221549250.5606@localhost.localdomain>

I like this, but like the "every second or every percent
whichever comes first" unpack-objects does even better.  How
about something like this on top of your patch?

---
diff --git a/pack-objects.c b/pack-objects.c
index 5e1e14c..d05ab23 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -674,10 +674,14 @@ static void find_deltas(struct object_en
 	int i, idx;
 	unsigned int array_size = window * sizeof(struct unpacked);
 	struct unpacked *array = xmalloc(array_size);
+	unsigned processed = 0;
+	unsigned last_percent = 999;
 
 	memset(array, 0, array_size);
 	i = nr_objects;
 	idx = 0;
+	if (progress)
+		fprintf(stderr, "Deltifying %d objects.\n", nr_objects);
 
 	while (--i >= 0) {
 		struct object_entry *entry = list[i];
@@ -686,10 +690,15 @@ static void find_deltas(struct object_en
 		char type[10];
 		int j;
 
-		if (progress_update || i == 0) {
-			fprintf(stderr, "Deltifying (%d %d%%)\r",
-				nr_objects-i, (nr_objects-i) * 100/nr_objects);
-			progress_update = 0;
+		processed++;
+		if (progress) {
+			unsigned percent = processed * 100 / nr_objects;
+			if (percent != last_percent || progress_update) {
+				fprintf(stderr, "%4u%% (%u/%u) done\r",
+					percent, processed, nr_objects);
+				progress_update = 0;
+				last_percent = percent;
+			}
 		}
 
 		if (entry->delta)

^ permalink raw reply related

* Re: [PATCH] nicer eye candies for pack-objects
From: Nicolas Pitre @ 2006-02-22 22:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy803kp1n.fsf@assigned-by-dhcp.cox.net>

On Wed, 22 Feb 2006, Junio C Hamano wrote:

> I like this, but like the "every second or every percent
> whichever comes first" unpack-objects does even better.  How
> about something like this on top of your patch?

Well... my concern is (if I'm right) that this status is generated 
remotely and sent over the network when performing a fetch.  The "every 
percent" might in this case generate quite some significant overhead if 
the pack is small.

Also (personal opinion) such progress numbers are harder to read when 
they change too fast.


Nicolas

^ permalink raw reply

* [PATCH] also adds progress when actually writing a pack
From: Nicolas Pitre @ 2006-02-22 22:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If that pack is big, it takes significant time to write and might 
benefit from some more eye candies as well.  This is however disabled 
when the pack is written to stdout since in that case the output is 
usually piped into unpack_objects which already does its own progress 
reporting.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/pack-objects.c b/pack-objects.c
index 7c89dc7..7784d32 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -53,6 +53,7 @@ static int nr_objects = 0, nr_alloc = 0;
 static const char *base_name;
 static unsigned char pack_file_sha1[20];
 static int progress = 1;
+static volatile int progress_update = 0;
 
 /*
  * The object names in objects array are hashed with this hashtable,
@@ -333,8 +334,14 @@ static void write_pack_file(void)
 	hdr.hdr_entries = htonl(nr_objects);
 	sha1write(f, &hdr, sizeof(hdr));
 	offset = sizeof(hdr);
-	for (i = 0; i < nr_objects; i++)
+	for (i = 0; i < nr_objects; i++) {
 		offset = write_one(f, objects + i, offset);
+		if (progress_update) {
+			fprintf(stderr, "Writing (%d %d%%)\r",
+				i+1, (i+1) * 100/nr_objects);
+			progress_update = 0;
+		}
+	}
 
 	sha1close(f, pack_file_sha1, 1);
 }
@@ -661,7 +668,6 @@ static int try_delta(struct unpacked *cu
 	return 0;
 }
 
-static volatile int progress_update = 0;
 static void progress_interval(int signum)
 {
 	signal(SIGALRM, progress_interval);
@@ -734,7 +740,6 @@ static void prepare_pack(int window, int
 	sorted_by_type = create_sorted_list(type_size_sort);
 	if (window && depth)
 		find_deltas(sorted_by_type, window+1, depth);
-	write_pack_file();
 }
 
 static int reuse_cached_pack(unsigned char *sha1, int pack_to_stdout)
@@ -904,6 +909,14 @@ int main(int argc, char **argv)
 		;
 	else {
 		prepare_pack(window, depth);
+		if (progress && pack_to_stdout) {
+			/* the other end usually displays progress itself */
+			struct itimerval v = {{0,},};
+			setitimer(ITIMER_REAL, &v, NULL);
+			signal(SIGALRM, SIG_IGN );
+			progress_update = 0;
+		}
+		write_pack_file();
 		if (!pack_to_stdout) {
 			write_index_file();
 			puts(sha1_to_hex(object_list_sha1));

^ permalink raw reply related

* Re: [PATCH] nicer eye candies for pack-objects
From: Linus Torvalds @ 2006-02-22 23:05 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602221733030.5606@localhost.localdomain>



On Wed, 22 Feb 2006, Nicolas Pitre wrote:

> On Wed, 22 Feb 2006, Junio C Hamano wrote:
> 
> > I like this, but like the "every second or every percent
> > whichever comes first" unpack-objects does even better.  How
> > about something like this on top of your patch?
> 
> Well... my concern is (if I'm right) that this status is generated 
> remotely and sent over the network when performing a fetch.  The "every 
> percent" might in this case generate quite some significant overhead if 
> the pack is small.

Well, my thinking behind the original unpack-objects behaviour was that we 
don't really care about the max 100 extra packets. 

The days of 150 baud line printers are gone. I cannot imagine any valid 
situation where you can't send a hundred small packets to update the 
screen.. And it did make a nice visual difference.

(In fact, over a network, if the line really is slow, you'll find that 
Nagle will fix things, and you'll just see one extra - but obviously 
slightly bigger - packet).

		Linus

^ permalink raw reply

* Re: [PATCH] nicer eye candies for pack-objects
From: Andreas Ericsson @ 2006-02-22 23:19 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602221733030.5606@localhost.localdomain>

Nicolas Pitre wrote:
> On Wed, 22 Feb 2006, Junio C Hamano wrote:
> 
> 
>>I like this, but like the "every second or every percent
>>whichever comes first" unpack-objects does even better.  How
>>about something like this on top of your patch?
> 
> 
> Well... my concern is (if I'm right) that this status is generated 
> remotely and sent over the network when performing a fetch.  The "every 
> percent" might in this case generate quite some significant overhead if 
> the pack is small.
> 

But if the pack is small it won't matter. It's when it's big we want to 
know about it (and then the each-percent method is better, really).

> Also (personal opinion) such progress numbers are harder to read when 
> they change too fast.
> 

I don't know about the rest of the world, but when I see numbers 
counting up with a percent-sign behind them I do some mental math to see 
how many brain-ticks go between each increment and then multiply with 
100 to see if I need to get a beer while waiting. I don't really care 
what number it shows if it flashes too fast to read.

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

^ permalink raw reply

* Re: [PATCH] nicer eye candies for pack-objects
From: Nicolas Pitre @ 2006-02-22 23:40 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602221502410.3771@g5.osdl.org>

On Wed, 22 Feb 2006, Linus Torvalds wrote:

> Well, my thinking behind the original unpack-objects behaviour was that we 
> don't really care about the max 100 extra packets. 

Obviously, the "every percent" has at most 100 additional packets.

And if it updates too fast to be readable, that means it'll be over in a 
snap anyway.

So I don't have any objections left.


Nicolas

^ permalink raw reply

* [PATCH] git-rm: Fix to properly handle files with spaces, tabs, newlines, etc.
From: Carl Worth @ 2006-02-23  0:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Krzysiek Pawlik, git
In-Reply-To: <7vaccjst3x.fsf@assigned-by-dhcp.cox.net>

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

New tests are added to the git-rm test case to cover this as well.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

On Wed, 22 Feb 2006 00:19:46 -0800, Junio C Hamano wrote:
>
> Note you are not using -z, which means we will c-quote the funny
> characters in the output...

Oh, I didn't expect C-language-style quoting. That's definitely not
going to work.

> > +*)
> > +	[[ "$remove_files" = "true" ]] && rm -- $files
> 
> Same here. What happens to filenames with IFS letters in them?
> "git-add" does not use -z and xargs -0 without a good reason.

Yeah, this is just me unleashing my shell-programming incompetence on
the world.

The attached patch addresses that problem with a rather blunt
hammer. Let me know if anyone has a more elegant approach than what I
did here.

One thing I've lost is that the previous version had a sort|uniq on
the output of git-ls-files which is useful in the case of failed
merges and other ways in which git-ls-files reports the same file
multiple times. What might be nice is a --unique flag to git-ls-files
that git-rm could use, (but on first glance it doesn't look trivial to
implement as git-ls-files doesn't ever store or sort its entire list).

> Even if rm -- $files were quoted correctly, and tried to remove
> the right files, if some of the files failed to disappear for
> whatever reason, what happens?

My intent with the previous patch was that, when git-rm is given -f,
and the rm fails to remove a file, that the file is then not removed
from the index. Of course, this was hopelessly broken due to two major
typos in the same line:

	index_remote_option=--force
instead of:
	index_remove_option=--remove

which my tests were insufficient to catch.

This behavior should now work in the current patch as well as the
proper handling of files with funny characters, (tests are included
for both).

The desired behavior when rm fails is debatable, so I'm open to
opinions. One reason I liked this was that in the previous patch, rm
would prompt the user before deleting a read-only file, and if the
user said no, then git-rm would also not remove it from the index.

This did cause another minor problem in that there would then be no
way to get git-rm to use "rm -f" when desired.

In the current patch, with my blunt hammer, there's another sub-shell
before the rm which apparently steals its tty and causes it to not
prompt at all. So that aspect may be moot.

-Carl

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 seem seems inelegant.

 git-rm.sh     |   37 ++++++++++++++++++++-----------------
 t/t3600-rm.sh |   30 ++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 25 deletions(-)

3d52c6d60047390d434f8737368adea77fa26310
diff --git a/git-rm.sh b/git-rm.sh
index 0a3f546..fa361bd 100755
--- a/git-rm.sh
+++ b/git-rm.sh
@@ -4,7 +4,6 @@ USAGE='[-f] [-n] [-v] [--] <file>...'
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
-index_remove_option=--force-remove
 remove_files=
 show_only=
 verbose=
@@ -12,7 +11,6 @@ while : ; do
   case "$1" in
     -f)
 	remove_files=true
-	index_remote_option=--force
 	;;
     -n)
 	show_only=true
@@ -45,23 +43,28 @@ case "$#" in
 	;;
 esac
 
-files=$(
-    if test -f "$GIT_DIR/info/exclude" ; then
-	git-ls-files \
-	    --exclude-from="$GIT_DIR/info/exclude" \
-	    --exclude-per-directory=.gitignore -- "$@"
-    else
-	git-ls-files \
+if test -f "$GIT_DIR/info/exclude"
+then
+	git-ls-files -z \
+	--exclude-from="$GIT_DIR/info/exclude" \
 	--exclude-per-directory=.gitignore -- "$@"
-    fi | sort | uniq
-)
-
-case "$show_only" in
-true)
-	echo $files
+else
+	git-ls-files -z \
+	--exclude-per-directory=.gitignore -- "$@"
+fi |
+case "$show_only,remove_files" in
+true,*)
+	xargs -0 echo
+	;;
+*,true)
+	xargs -0 sh -c "
+		while [ \$# -gt 0 ]; do
+			file=\$1; shift
+			rm -- \"\$file\" && git-update-index --remove $verbose \"\$file\"
+		done
+	" inline
 	;;
 *)
-	[[ "$remove_files" = "true" ]] && rm -- $files
-	git-update-index $index_remove_option $verbose $files
+	git-update-index --force-remove $verbose -z --stdin
 	;;
 esac
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 8415732..b87beb0 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -7,13 +7,12 @@ test_description='Test of the various op
 
 . ./test-lib.sh
 
-# Setup some files to be removed
-touch foo bar
-git-add foo bar
-# Need one to test --
-touch -- -q
-git update-index --add -- -q
-git-commit -m "add foo, bar, and -q"
+# Setup some files to be removed, some with funny characters
+touch -- foo bar baz 'space embedded' 'tab	embedded' 'newline
+embedded' -q
+git-add -- foo bar baz 'space embedded' 'tab	embedded' 'newline
+embedded' -q
+git-commit -m "add files"
 
 test_expect_success \
     'Pre-check that foo is in index before git-rm foo' \
@@ -36,7 +35,22 @@ test_expect_failure \
     '[ -f bar ]'
 
 test_expect_success \
-    'Test that "git-rm -- -q" works to delete a file named -q' \
+    'Test that "git-rm -- -q" works to delete a file that looks like an option' \
     'git-rm -- -q'
 
+test_expect_success \
+    "Test that \"git-rm -f\" can remove files with embedded space, tab, or newline characters." \
+    "git-rm 'space embedded' 'tab	embedded' 'newline
+embedded"
+
+chmod u-w .
+test_expect_failure \
+    'Test that "git-rm -f" fails if its rm fails' \
+    'git-rm -f baz'
+chmod u+w .
+
+test_expect_success \
+    'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
+    'git-ls-files --error-unmatch baz'
+
 test_done
-- 
1.2.2.g01a2-dirty



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Jonas Fonseca @ 2006-02-23  0:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Carl Worth, git
In-Reply-To: <7vr75vmcod.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote Wed, Feb 22, 2006:
> @@ -552,7 +563,9 @@ static void prepare_packed_git_one(char 
>  	len = strlen(path);
>  	dir = opendir(path);
>  	if (!dir) {
> -		fprintf(stderr, "unable to open object pack directory: %s: %s\n", path, strerror(errno));
> +		if (errno != ENOENT)
> +			error("unable to open object pack directory: %s: %s\n",
> +			      path, strerror(errno));

No need for the ending \n.

-- 
Jonas Fonseca

^ permalink raw reply

* Re: [PATCH] git-rm: Fix to properly handle files with spaces, tabs, newlines, etc.
From: Carl Worth @ 2006-02-23  0:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Krzysiek Pawlik, git
In-Reply-To: <8764n7rl6s.wl%cworth@cworth.org>

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

New tests are added to the git-rm test case to cover this as well.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

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

 (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?)

 -Carl

 git-rm.sh     |   37 ++++++++++++++++++++-----------------
 t/t3600-rm.sh |   52 +++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 55 insertions(+), 34 deletions(-)

3bd80bae8dc2b004a3109018f0efb0007804b79d
diff --git a/git-rm.sh b/git-rm.sh
index 0a3f546..fda4541 100755
--- a/git-rm.sh
+++ b/git-rm.sh
@@ -4,7 +4,6 @@ USAGE='[-f] [-n] [-v] [--] <file>...'
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
-index_remove_option=--force-remove
 remove_files=
 show_only=
 verbose=
@@ -12,7 +11,6 @@ while : ; do
   case "$1" in
     -f)
 	remove_files=true
-	index_remote_option=--force
 	;;
     -n)
 	show_only=true
@@ -45,23 +43,28 @@ case "$#" in
 	;;
 esac
 
-files=$(
-    if test -f "$GIT_DIR/info/exclude" ; then
-	git-ls-files \
-	    --exclude-from="$GIT_DIR/info/exclude" \
-	    --exclude-per-directory=.gitignore -- "$@"
-    else
-	git-ls-files \
+if test -f "$GIT_DIR/info/exclude"
+then
+	git-ls-files -z \
+	--exclude-from="$GIT_DIR/info/exclude" \
 	--exclude-per-directory=.gitignore -- "$@"
-    fi | sort | uniq
-)
-
-case "$show_only" in
-true)
-	echo $files
+else
+	git-ls-files -z \
+	--exclude-per-directory=.gitignore -- "$@"
+fi |
+case "$show_only,$remove_files" in
+true,*)
+	xargs -0 echo
+	;;
+*,true)
+	xargs -0 sh -c "
+		while [ \$# -gt 0 ]; do
+			file=\$1; shift
+			rm -- \"\$file\" && git-update-index --remove $verbose \"\$file\"
+		done
+	" inline
 	;;
 *)
-	[[ "$remove_files" = "true" ]] && rm -- $files
-	git-update-index $index_remove_option $verbose $files
+	git-update-index --force-remove $verbose -z --stdin
 	;;
 esac
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 8415732..cabfadd 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -7,36 +7,54 @@ test_description='Test of the various op
 
 . ./test-lib.sh
 
-# Setup some files to be removed
-touch foo bar
-git-add foo bar
-# Need one to test --
-touch -- -q
-git update-index --add -- -q
-git-commit -m "add foo, bar, and -q"
+# Setup some files to be removed, some with funny characters
+touch -- foo bar baz 'space embedded' 'tab	embedded' 'newline
+embedded' -q
+git-add -- foo bar baz 'space embedded' 'tab	embedded' 'newline
+embedded' -q
+git-commit -m "add files"
 
 test_expect_success \
-    'Pre-check that foo is in index before git-rm foo' \
-    'git-ls-files --error-unmatch foo'
+    'Pre-check that foo exists and is in index before git-rm foo' \
+    '[ -f foo ] && git-ls-files --error-unmatch foo'
 
 test_expect_success \
     'Test that git-rm foo succeeds' \
     'git-rm foo'
 
-test_expect_failure \
-    'Post-check that foo is not in index after git-rm foo' \
-    'git-ls-files --error-unmatch foo'
+test_expect_success \
+    'Post-check that foo exists but is not in index after git-rm foo' \
+    '[ -f foo ] && ! git-ls-files --error-unmatch foo'
+
+test_expect_success \
+    'Pre-check that bar exists and is in index before "git-rm -f bar"' \
+    '[ -f bar ] && git-ls-files --error-unmatch bar'
 
 test_expect_success \
-    'Test that "git-rm -f bar" works' \
+    'Test that "git-rm -f bar" succeeds' \
     'git-rm -f bar'
 
-test_expect_failure \
-    'Post-check that bar no longer exists' \
-    '[ -f bar ]'
+test_expect_success \
+    'Post-check that bar does not exist and is not in index after "git-rm -f bar"' \
+    '! [ -f bar ] && ! git-ls-files --error-unmatch bar'
 
 test_expect_success \
-    'Test that "git-rm -- -q" works to delete a file named -q' \
+    'Test that "git-rm -- -q" succeeds (remove a file that looks like an option)' \
     'git-rm -- -q'
 
+test_expect_success \
+    "Test that \"git-rm -f\" succeeds with embedded space, tab, or newline characters." \
+    "git-rm -f 'space embedded' 'tab	embedded' 'newline
+embedded'"
+
+chmod u-w .
+test_expect_failure \
+    'Test that "git-rm -f" fails if its rm fails' \
+    'git-rm -f baz'
+chmod u+w .
+
+test_expect_success \
+    'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
+    'git-ls-files --error-unmatch baz'
+
 test_done
-- 
1.2.2.g3d52-dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* 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


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