* [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] 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
* 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: 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
* [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: 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
* 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] 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] 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] 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
* [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
* [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
* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Junio C Hamano @ 2006-02-22 19:54 UTC (permalink / raw)
To: Sam Vilain; +Cc: git
In-Reply-To: <43FCC0D0.5050307@vilain.net>
Sam Vilain <sam@vilain.net> writes:
> Checking in Module::CoreList, that module goes right back to the Perl
> 5.0 release, so every normal Perl 5 distribution should have it.
Good digging, but IIRC this thread started because something
that _claims_ to be 5.8 does not grok open(F, '-|') correctly,
so...
^ permalink raw reply
* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Sam Vilain @ 2006-02-22 19:51 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0602220835p4c4243edm145ee827eb706121@mail.gmail.com>
Alex Riesen wrote:
>>I guess what I'm saying is that if you want to limit the modules that
>>Perl script uses, you end up either impacting on the portability of the
>>script or rediscovering problems with early wheel designs.
> IPC::Open{2,3} seem to be installed on every system I have access to.
Checking in Module::CoreList, that module goes right back to the Perl
5.0 release, so every normal Perl 5 distribution should have it.
Sam.
^ permalink raw reply
* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Johannes Schindelin @ 2006-02-22 19:44 UTC (permalink / raw)
To: Alex Riesen; +Cc: Sam Vilain, Junio C Hamano, Eric Wong, git
In-Reply-To: <81b0412b0602220835p4c4243edm145ee827eb706121@mail.gmail.com>
Hi,
On Wed, 22 Feb 2006, Alex Riesen wrote:
> IPC::Open{2,3} seem to be installed on every system I have access to.
I can confirm that the platforms I usually work on also provide it
(Linux, Linux, old IRIX, old macosx, MinGW32).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Carl Worth @ 2006-02-22 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr75vmcod.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: text/plain, Size: 213 bytes --]
On Wed, 22 Feb 2006 11:11:14 -0800, Junio C Hamano wrote:
>
> Updated patch. The root cause of the problem you had was that
> alternates was dangling, so we catch that.
Looks perfect. Thanks for the fix.
-Carl
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Junio C Hamano @ 2006-02-22 19:11 UTC (permalink / raw)
To: Carl Worth; +Cc: git
In-Reply-To: <87bqwzs07h.wl%cworth@cworth.org>
Carl Worth <cworth@cworth.org> writes:
> In the case in which I hit the original bug, this message is necessary
> to provide information about where the actual problem is. Here is what
> that scenario looks like with the message:
>
> $ mkdir original;
> $ (cd original; git-init-db; touch foo; git add foo; git commit -m "original")
> defaulting to local storage area
> Committing initial tree 4d5fcadc293a348e88f777dc0920f11e7d71441c
> $ git clone -l -s original clone
> $ mv original moved
> $ git clone clone again
> unable to open object pack directory: /tmp/original/.git/objects/pack: No such file or directory
> fatal: git-upload-pack: cannot find object 0153d496df669cbe5cecb665dbe6f95b20461917:
> fatal: unexpected EOF
> clone-pack from '/tmp/clone/.git' failed.
>
> Here the "cannot find object" message doesn't point to the core
> problem, but the "unable to open object pack directory" does contain
> the "/tmp/original" path of interest.
Updated patch. The root cause of the problem you had was that
alternates was dangling, so we catch that.
-- >8 --
diff --git a/sha1_file.c b/sha1_file.c
index f08b1d6..c08da35 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -247,6 +247,7 @@ static void link_alt_odb_entries(const c
for ( ; cp < ep && *cp != sep; cp++)
;
if (last != cp) {
+ struct stat st;
struct alternate_object_database *alt;
/* 43 = 40-byte + 2 '/' + terminating NUL */
int pfxlen = cp - last;
@@ -269,9 +270,19 @@ static void link_alt_odb_entries(const c
}
else
memcpy(ent->base, last, pfxlen);
+
ent->name = ent->base + pfxlen + 1;
- ent->base[pfxlen] = ent->base[pfxlen + 3] = '/';
- ent->base[entlen-1] = 0;
+ ent->base[pfxlen + 3] = '/';
+ ent->base[pfxlen] = ent->base[entlen-1] = 0;
+
+ /* Detect cases where alternate disappeared */
+ if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
+ error("object directory %s does not exist; "
+ "check .git/objects/info/alternates.",
+ ent->base);
+ goto bad;
+ }
+ ent->base[pfxlen] = '/';
/* Prevent the common mistake of listing the same
* thing twice, or object directory itself.
@@ -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));
return;
}
path[len++] = '/';
^ permalink raw reply related
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Junio C Hamano @ 2006-02-22 18:50 UTC (permalink / raw)
To: Carl Worth; +Cc: git, Andrew Vasquez
In-Reply-To: <87bqwzs07h.wl%cworth@cworth.org>
I have this in mind...
---
diff --git a/sha1_file.c b/sha1_file.c
index f08b1d6..dae77fc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -552,7 +552,8 @@ 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)
+ fprintf(stderr, "unable to open object pack directory: %s: %s\n", path, strerror(errno));
return;
}
path[len++] = '/';
^ permalink raw reply related
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Carl Worth @ 2006-02-22 18:44 UTC (permalink / raw)
To: Andrew Vasquez; +Cc: git
In-Reply-To: <20060222181758.GH3355@andrew-vasquezs-powerbook-g4-15.local>
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
On Wed, 22 Feb 2006 10:17:58 -0800, Andrew Vasquez wrote:
> Commit:
>
> b5b16990f8b074bd0481ced047b8f8bf66eee6dc
> Prevent git-upload-pack segfault if object cannot be found
>
> is causing some really annoying noise being sent to stderr on some of
> my older non-packed repositories:
>
> $ git status
> # On branch refs/heads/b4
> unable to open object pack directory: .git/objects/pack: No such file or directory
> nothing to commit
Hmm... I can see that would be really annoying.
> > - if (!dir)
> > + if (!dir) {
> > + fprintf(stderr, "unable to open object pack directory: %s: %s\n", path, strerror(errno));
> > return;
> > + }
>
> Could we drop this fprintf to stderr?
In the case in which I hit the original bug, this message is necessary
to provide information about where the actual problem is. Here is what
that scenario looks like with the message:
$ mkdir original;
$ (cd original; git-init-db; touch foo; git add foo; git commit -m "original")
defaulting to local storage area
Committing initial tree 4d5fcadc293a348e88f777dc0920f11e7d71441c
$ git clone -l -s original clone
$ mv original moved
$ git clone clone again
unable to open object pack directory: /tmp/original/.git/objects/pack: No such file or directory
fatal: git-upload-pack: cannot find object 0153d496df669cbe5cecb665dbe6f95b20461917:
fatal: unexpected EOF
clone-pack from '/tmp/clone/.git' failed.
Here the "cannot find object" message doesn't point to the core
problem, but the "unable to open object pack directory" does contain
the "/tmp/original" path of interest.
One could be a bit more careful about not complaining if <objdir>
actually does exist, even if <objdir>/pack does not.
Or a workaround would be to just run git-init-db in old repositories
to create the pack directory:
$ rmdir .git/objects/pack/
$ git status
unable to open object pack directory: .git/objects/pack: No such file or directory
nothing to commit
$ git-init-db
defaulting to local storage area
$ git status
nothing to commit
-Carl
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] send-pack: do not give up when remote has insanely large number of refs.
From: Junio C Hamano @ 2006-02-22 18:42 UTC (permalink / raw)
To: Stephen C. Tweedie; +Cc: git
In-Reply-To: <1140633034.3385.5.camel@orbit.scot.redhat.com>
"Stephen C. Tweedie" <sct@redhat.com> writes:
> Adding more ^refs up to the limit of 900 should be possible, too, and
> should catch more already-present objects --- while the refs count for
> this repo was under 900, push still worked fine for me, so we don't
> necessarily have to cut it hard to as low a number as 16.
Actually it was done on purpose. My first round did not have
that 16 limit, and then it turned out that rev-list, given extra
work to cull those 900 negative refs, were spending a lot of
time. I created a repository with 1500 tags for this test ;-).
Unless we were talking about a repository that happens to host
1000 unrelated projects, each tracked with separate sets of
heads and tags, those negative refs should be related with each
other in the ancestry graph. We should be able to eliminate
most of them by using the latest handful.
> Perhaps ultimately we may want to simply send the refs list to
> git-rev-list via a pipe or similar if we want this to scale? We'll need
> this for edge cases such as sending >900 new tags to an old repository
> at once, as we'll exhaust the size of the positive refs list in that
> case.
True. We should support unbounded number of positive refs.
^ permalink raw reply
* Re: [PATCH] send-pack: do not give up when remote has insanely large number of refs.
From: Stephen C. Tweedie @ 2006-02-22 18:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wxvsovj.fsf_-_@assigned-by-dhcp.cox.net>
Hi,
On Wed, 2006-02-22 at 01:51 -0800, Junio C Hamano wrote:
> + for (ref = refs, j = i + 16;
> + i < 900 && i < j && ref;
Looks like it's now sending just 16 additional negative refs instead of
940 for this repo. Definitely an improvement --- push (both full and
with an explicit refspec) is now working properly, thanks!
Adding more ^refs up to the limit of 900 should be possible, too, and
should catch more already-present objects --- while the refs count for
this repo was under 900, push still worked fine for me, so we don't
necessarily have to cut it hard to as low a number as 16.
Perhaps ultimately we may want to simply send the refs list to
git-rev-list via a pipe or similar if we want this to scale? We'll need
this for edge cases such as sending >900 new tags to an old repository
at once, as we'll exhaust the size of the positive refs list in that
case.
Thanks again,
Stephen
^ permalink raw reply
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Junio C Hamano @ 2006-02-22 18:26 UTC (permalink / raw)
To: Andrew Vasquez; +Cc: git
In-Reply-To: <20060222181758.GH3355@andrew-vasquezs-powerbook-g4-15.local>
Andrew Vasquez <andrew.vasquez@qlogic.com> writes:
> Commit:
>
> b5b16990f8b074bd0481ced047b8f8bf66eee6dc
> Prevent git-upload-pack segfault if object cannot be found
>
> is causing some really annoying noise being sent to stderr on some of
> my older non-packed repositories:
>...
> Could we drop this fprintf to stderr?
Thanks for noticing.
^ permalink raw reply
* Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found
From: Andrew Vasquez @ 2006-02-22 18:17 UTC (permalink / raw)
To: git
Commit:
b5b16990f8b074bd0481ced047b8f8bf66eee6dc
Prevent git-upload-pack segfault if object cannot be found
is causing some really annoying noise being sent to stderr on some of
my older non-packed repositories:
$ git status
# On branch refs/heads/b4
unable to open object pack directory: .git/objects/pack: No such file or directory
nothing to commit
$ git-rev-list --pretty=oneline v8.01.04b3..
unable to open object pack directory: .git/objects/pack: No such file or directory
a7401b7109fb2cba7de41a0e30dfe6aa41690ea8 No ZIO.
...
Here's the relevant hunk:
> diff --git a/sha1_file.c b/sha1_file.c
> index 3d11a9b..f08b1d6 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -551,8 +551,10 @@ static void prepare_packed_git_one(char
> sprintf(path, "%s/pack", objdir);
> len = strlen(path);
> dir = opendir(path);
> - if (!dir)
> + if (!dir) {
> + fprintf(stderr, "unable to open object pack directory: %s: %s\n", path, strerror(errno));
> return;
> + }
> path[len++] = '/';
> while ((de = readdir(dir)) != NULL) {
> int namelen = strlen(de->d_name);
Could we drop this fprintf to stderr?
Thanks,
Andrew
^ permalink raw reply
* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-22 16:35 UTC (permalink / raw)
To: Sam Vilain; +Cc: Junio C Hamano, Johannes Schindelin, Eric Wong, git
In-Reply-To: <43FB9656.8050308@vilain.net>
On 2/21/06, Sam Vilain <sam@vilain.net> wrote:
> Alex Riesen wrote:
> >>>Does not work here (ActiveState Build 811, Perl 5.8.6):
> >>>$ perl -e 'open(F, "-|")'
> >>>'-' is not recognized as an internal or external command,
> >>>operable program or batch file.
> >>Portability, Ease of Coding, Few CPAN Module Dependencies. Pick any two.
> > Sometimes an upgrade is just out of question. Besides, that'd mean an
> > upgrade to another operating system, because very important scripts
> > over here a just not portable to anything else but
> > "ActiveState Perl on Windows (TM)"
> > I just have no choice.
>
> Sure, but perhaps IPC::Open2 or some other CPAN module has solved this
> problem already.
IPC::Open2 works! Well "kind of": there are still strange segfaults regarding
stack sometimes. And I don't know yet whether and how the arguments are escaped
(Windows has no argument array. It has that bloody stupid one-line command line)
> I guess what I'm saying is that if you want to limit the modules that
> Perl script uses, you end up either impacting on the portability of the
> script or rediscovering problems with early wheel designs.
IPC::Open{2,3} seem to be installed on every system I have access to.
^ permalink raw reply
* gitview: Use git ls-remote to find the tag and branch details
From: Aneesh Kumar K.V @ 2006-02-22 16:07 UTC (permalink / raw)
To: git, Junio C Hamano, aneesh.kumar
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: 0002-gitview-Use-git-ls-remote-to-find-the-tag-and-branch-details.txt --]
[-- Type: text/plain, Size: 2707 bytes --]
From: Junio C Hamano <junkio@cox.net>
This fix the below bug
Junio C Hamano <junkio@cox.net> writes:
>
> It does not work in my repository, since you do not seem to
> handle branch and tag names with slashes in them. All of my
> topic branches live in directories with two-letter names
> (e.g. ak/gitview).
Also use ${GIT_DIR} directly so that it works with below environment setup
GIT_DIR=/home/opensource/Test\ Output/git-devel/.git
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>
---
contrib/gitview/gitview | 54 ++++++++++++-----------------------------------
1 files changed, 14 insertions(+), 40 deletions(-)
ecd82bdd8399b84ada1f4fc0c720a88ae0735a94
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 0e52c78..76a1b67 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -56,20 +56,6 @@ def show_date(epoch, tz):
return time.strftime("%Y-%m-%d %H:%M:%S", time.gmtime(secs))
-def get_sha1_from_tags(line):
- fp = os.popen("git cat-file -t " + line)
- entry = string.strip(fp.readline())
- fp.close()
- if (entry == "commit"):
- return line
- elif (entry == "tag"):
- fp = os.popen("git cat-file tag "+ line)
- entry = string.strip(fp.readline())
- fp.close()
- obj = re.split(" ", entry)
- if (obj[0] == "object"):
- return obj[1]
- return None
class CellRendererGraph(gtk.GenericCellRenderer):
"""Cell renderer for directed graph.
@@ -467,32 +453,20 @@ class GitView:
respective sha1 details """
self.bt_sha1 = { }
- git_dir = os.getenv("GIT_DIR")
- if (git_dir == None):
- git_dir = ".git"
-
- #FIXME the path seperator
- ref_files = os.listdir(git_dir + "/refs/tags")
- for file in ref_files:
- fp = open(git_dir + "/refs/tags/"+file)
- sha1 = get_sha1_from_tags(string.strip(fp.readline()))
- try:
- self.bt_sha1[sha1].append(file)
- except KeyError:
- self.bt_sha1[sha1] = [file]
- fp.close()
-
-
- #FIXME the path seperator
- ref_files = os.listdir(git_dir + "/refs/heads")
- for file in ref_files:
- fp = open(git_dir + "/refs/heads/" + file)
- sha1 = get_sha1_from_tags(string.strip(fp.readline()))
- try:
- self.bt_sha1[sha1].append(file)
- except KeyError:
- self.bt_sha1[sha1] = [file]
- fp.close()
+ ls_remote = re.compile('^(.{40})\trefs/([^^]+)(?:\\^(..))?$');
+ fp = os.popen('git ls-remote "${GIT_DIR:-.git}"')
+ while 1:
+ line = string.strip(fp.readline())
+ if line == '':
+ break
+ m = ls_remote.match(line)
+ if not m:
+ continue
+ (sha1, name) = (m.group(1), m.group(2))
+ if not self.bt_sha1.has_key(sha1):
+ self.bt_sha1[sha1] = []
+ self.bt_sha1[sha1].append(name)
+ fp.close()
def construct(self):
--
1.2.0-dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox