* Re: Bug: git-rebase goofs up \n in commit messages
From: Junio C Hamano @ 2007-05-26 3:59 UTC (permalink / raw)
To: Jeff King; +Cc: Herbert Xu, Szekeres Istvan, git
In-Reply-To: <20070526034236.GA18169@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, May 26, 2007 at 11:10:36AM +1000, Herbert Xu wrote:
>
>> If you need to echo something that may have escapes in it, the portable
>> way to do it is
>>
>> printf '%s\n' "$test"
>
> Ah, I see. I had thought the problem was coming from some dash
> interpolation magic, but yes, it's just echo doing the conversion. And
> POSIX is very clear that this is an implementation defined behavior.
> Thanks very much for the response, Herbert.
>
> Junio, patch is below. I have no idea how prevalent this issue is within
> our scripts, but this at least fixes the reported bug.
Gaah. Ok, dash uses "echo -e" behaviour by default.
I guess we need to hunt allmost all "echo", as I suspect most of
them (except the ones we use to do "echo $SHA1") have user
strings somewhere.
What a mess, but that is not your fault nor Herbert's.
Thanks.
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Junio C Hamano @ 2007-05-26 4:59 UTC (permalink / raw)
To: Jeff King; +Cc: Herbert Xu, Szekeres Istvan, git
In-Reply-To: <20070526034236.GA18169@coredump.intra.peff.net>
It turns out that git-commit also shares the same problem under
dash.
-- >8 --
git-commit: use printf "%s\n" instead of echo on user-supplied strings
This adds a test to verify the earlier fix to git-am by Jeff
King, and fixes the same issue in git-commit that is exposed by
the new test while at it.
Cleverly enough, this commit's log message is a good test case
at the same time.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* I suspect we would declare either "war on echo" or "harder push
for builtins" triggered by these.
git-commit.sh | 8 ++++----
t/t4014-format-patch.sh | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index 292cf96..f3f9a35 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -376,12 +376,12 @@ t,)
rm -f "$TMP_INDEX"
fi || exit
- echo "$commit_only" |
+ printf "%s\n" "$commit_only" |
GIT_INDEX_FILE="$TMP_INDEX" \
git-update-index --add --remove --stdin &&
save_index &&
- echo "$commit_only" |
+ printf "%s\n" "$commit_only" |
(
GIT_INDEX_FILE="$NEXT_INDEX"
export GIT_INDEX_FILE
@@ -432,7 +432,7 @@ fi
if test "$log_message" != ''
then
- echo "$log_message"
+ printf "%s\n" "$log_message"
elif test "$logfile" != ""
then
if test "$logfile" = -
@@ -475,7 +475,7 @@ if test -f "$GIT_DIR/MERGE_HEAD" && test -z "$no_edit"; then
echo "#"
echo "# It looks like you may be committing a MERGE."
echo "# If this is not correct, please remove the file"
- echo "# $GIT_DIR/MERGE_HEAD"
+ printf "%s\n" "# $GIT_DIR/MERGE_HEAD"
echo "# and try again"
echo "#"
fi >>"$GIT_DIR"/COMMIT_EDITMSG
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 4795872..42aa9e4 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -16,16 +16,16 @@ test_expect_success setup '
for i in 1 2 5 6 A B C 7 8 9 10; do echo "$i"; done >file &&
git update-index file &&
- git commit -m "Side change #1" &&
+ git commit -m "Side changes #1" &&
for i in D E F; do echo "$i"; done >>file &&
git update-index file &&
- git commit -m "Side change #2" &&
+ git commit -m "Side changes #2" &&
git tag C2 &&
for i in 5 6 1 2 3 A 4 B C 7 8 9 10 D E F; do echo "$i"; done >file &&
git update-index file &&
- git commit -m "Side change #3" &&
+ git commit -m "Side changes #3 with \\n backslash-n in it." &&
git checkout master &&
git diff-tree -p C2 | git apply --index &&
^ permalink raw reply related
* [PATCH 1/3] Lazily open pack index files on demand
From: Shawn O. Pearce @ 2007-05-26 5:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Dana How
In some repository configurations the user may have many packfiles,
but all of the recent commits/trees/tags/blobs are likely to
be in the most recent packfile (the one with the newest mtime).
It is therefore common to be able to complete an entire operation
by accessing only one packfile, even if there are 25 packfiles
available to the repository.
Rather than opening and mmaping the corresponding .idx file for
every pack found, we now only open and map the .idx when we suspect
there might be an object of interest in there.
Of course we cannot known in advance which packfile contains an
object, so we still need to scan the entire packed_git list to
locate anything. But odds are users want to access objects in the
most recently created packfiles first, and that may be all they
ever need for the current operation.
Junio observed in b867092f that placing recent packfiles before
older ones can slightly improve access times for recent objects,
without degrading it for historical object access.
This change improves upon Junio's observations by trying even harder
to avoid the .idx files that we won't need.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
This conflicts (in a subtle way) with Dana How's
"sha1_file.c:rearrange_packed_git() should consider packs' object
sizes" patch as we now have num_objects = 0 for any indexes we
have not opened. In the case of Dana's patch this would cause
those packfiles to have very high ranks, possibly sorting much
later than they should have.
builtin-count-objects.c | 2 ++
cache.h | 3 ++-
pack-check.c | 9 +++++++--
pack-redundant.c | 3 +++
sha1_file.c | 38 +++++++++++++++++++++++++++++++++++---
5 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/builtin-count-objects.c b/builtin-count-objects.c
index ff90ebd..ac65e03 100644
--- a/builtin-count-objects.c
+++ b/builtin-count-objects.c
@@ -111,6 +111,8 @@ int cmd_count_objects(int ac, const char **av, const char *prefix)
for (p = packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
+ if (!p->index_data && open_pack_index(p))
+ continue;
packed += p->num_objects;
num_pack++;
}
diff --git a/cache.h b/cache.h
index cd875bc..0f4a05b 100644
--- a/cache.h
+++ b/cache.h
@@ -485,10 +485,11 @@ extern struct packed_git *find_sha1_pack(const unsigned char *sha1,
struct packed_git *packs);
extern void pack_report(void);
+extern int open_pack_index(struct packed_git *);
extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t, unsigned int *);
extern void unuse_pack(struct pack_window **);
extern struct packed_git *add_packed_git(const char *, int, int);
-extern const unsigned char *nth_packed_object_sha1(const struct packed_git *, uint32_t);
+extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);
extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
diff --git a/pack-check.c b/pack-check.c
index d04536b..7475348 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -129,12 +129,17 @@ static void show_pack_info(struct packed_git *p)
int verify_pack(struct packed_git *p, int verbose)
{
- off_t index_size = p->index_size;
- const unsigned char *index_base = p->index_data;
+ off_t index_size;
+ const unsigned char *index_base;
SHA_CTX ctx;
unsigned char sha1[20];
int ret;
+ if (open_pack_index(p))
+ return error("packfile %s index not opened", p->pack_name);
+ index_size = p->index_size;
+ index_base = p->index_data;
+
ret = 0;
/* Verify SHA1 sum of the index file */
SHA1_Init(&ctx);
diff --git a/pack-redundant.c b/pack-redundant.c
index 87077e1..0617320 100644
--- a/pack-redundant.c
+++ b/pack-redundant.c
@@ -550,6 +550,9 @@ static struct pack_list * add_pack(struct packed_git *p)
l.pack = p;
llist_init(&l.all_objects);
+ if (!p->index_data && open_pack_index(p))
+ return NULL;
+
base = p->index_data;
base += 256 * 4 + ((p->index_version < 2) ? 4 : 8);
step = (p->index_version < 2) ? 24 : 20;
diff --git a/sha1_file.c b/sha1_file.c
index 12d2ef2..6a5ba63 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -530,6 +530,21 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
return 0;
}
+int open_pack_index (struct packed_git *p)
+{
+ char *idx_name;
+ int ret;
+
+ if (p->index_data)
+ return 0;
+
+ idx_name = xstrdup(p->pack_name);
+ strcpy(idx_name + strlen(idx_name) - strlen(".pack"), ".idx");
+ ret = check_packed_git_idx(idx_name, p);
+ free(idx_name);
+ return ret;
+}
+
static void scan_windows(struct packed_git *p,
struct packed_git **lru_p,
struct pack_window **lru_w,
@@ -605,6 +620,9 @@ static int open_packed_git_1(struct packed_git *p)
unsigned char *idx_sha1;
long fd_flag;
+ if (!p->index_data && open_pack_index(p))
+ return error("packfile %s index unavailable", p->pack_name);
+
p->pack_fd = open(p->pack_name, O_RDONLY);
if (p->pack_fd < 0 || fstat(p->pack_fd, &st))
return -1;
@@ -757,8 +775,7 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
return NULL;
memcpy(p->pack_name, path, path_len);
strcpy(p->pack_name + path_len, ".pack");
- if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode) ||
- check_packed_git_idx(path, p)) {
+ if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
free(p);
return NULL;
}
@@ -766,6 +783,10 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
/* ok, it looks sane as far as we can check without
* actually mapping the pack file.
*/
+ p->index_version = 0;
+ p->index_data = NULL;
+ p->index_size = 0;
+ p->num_objects = 0;
p->pack_size = st.st_size;
p->next = NULL;
p->windows = NULL;
@@ -1572,10 +1593,15 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
return data;
}
-const unsigned char *nth_packed_object_sha1(const struct packed_git *p,
+const unsigned char *nth_packed_object_sha1(struct packed_git *p,
uint32_t n)
{
const unsigned char *index = p->index_data;
+ if (!index) {
+ if (open_pack_index(p))
+ return NULL;
+ index = p->index_data;
+ }
if (n >= p->num_objects)
return NULL;
index += 4 * 256;
@@ -1612,6 +1638,12 @@ off_t find_pack_entry_one(const unsigned char *sha1,
const unsigned char *index = p->index_data;
unsigned hi, lo;
+ if (!index) {
+ if (open_pack_index(p))
+ return 0;
+ level1_ofs = p->index_data;
+ index = p->index_data;
+ }
if (p->index_version > 1) {
level1_ofs += 2;
index += 8;
--
1.5.2.789.g8ee1
^ permalink raw reply related
* [PATCH 2/3] Micro-optimize prepare_alt_odb
From: Shawn O. Pearce @ 2007-05-26 5:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Calling getenv() is not that expensive, but its also not free,
and its certainly not cheaper than testing to see if alt_odb_tail
is not null.
Because we are calling prepare_alt_odb() from within find_sha1_file
every time we cannot find an object file locally we want to skip out
of prepare_alt_odb() as early as possible once we have initialized
our alternate list.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_file.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 6a5ba63..a3637d7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -376,11 +376,12 @@ void prepare_alt_odb(void)
{
const char *alt;
+ if (alt_odb_tail)
+ return;
+
alt = getenv(ALTERNATE_DB_ENVIRONMENT);
if (!alt) alt = "";
- if (alt_odb_tail)
- return;
alt_odb_tail = &alt_odb_list;
link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL, 0);
--
1.5.2.789.g8ee1
^ permalink raw reply related
* [PATCH 3/3] Attempt to delay prepare_alt_odb during get_sha1
From: Shawn O. Pearce @ 2007-05-26 5:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Not every input value passed to get_sha1 is an abbreviated SHA-1.
Its actually quite common for refs to be passed and for those
refs to resolve to full SHA-1s, in which case we may not need to
initialize the alternate object database list in this process.
I'm relocating the call to prepare_alt_odb closer to the code
that actually needs it to maintain the fix first introduced by
Junio in 99a19b43 (to avoid ambiguous SHA-1 abbreviations from
being accepted). This allows us to avoid the alt_odb list setup
if we won't actually need it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_name.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 55f25a2..8dfceb2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -133,6 +133,7 @@ static int find_unique_short_object(int len, char *canonical,
int has_unpacked, has_packed;
unsigned char unpacked_sha1[20], packed_sha1[20];
+ prepare_alt_odb();
has_unpacked = find_short_object_filename(len, canonical, unpacked_sha1);
has_packed = find_short_packed_object(len, res, packed_sha1);
if (!has_unpacked && !has_packed)
@@ -654,7 +655,6 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
const char *cp;
*mode = S_IFINVALID;
- prepare_alt_odb();
ret = get_sha1_1(name, namelen, sha1);
if (!ret)
return ret;
--
1.5.2.789.g8ee1
^ permalink raw reply related
* Re: Bug: git-rebase goofs up \n in commit messages
From: Jeff King @ 2007-05-26 6:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Herbert Xu, Szekeres Istvan, git
In-Reply-To: <7vps4onps0.fsf@assigned-by-dhcp.cox.net>
On Fri, May 25, 2007 at 09:59:43PM -0700, Junio C Hamano wrote:
> * I suspect we would declare either "war on echo" or "harder push
> for builtins" triggered by these.
Cry havoc! More fixes below (just a diff -- maybe we want to aggregate
these into a single commit?).
These are the ones I noticed that use commit messages (which are
probably the most likely to use backslash). There are _tons_ of uses for
heads and filenames. I think we either should stop with commit messages,
or go all-out and simply remove all uses of echo (because there are
literally hundreds otherwise).
> - echo "$commit_only" |
> + printf "%s\n" "$commit_only" |
Is "\n" portable to all shells (i.e., do you need '\n')? It works with
bash and dash, which are by far the most common, but who knows what evil
lurks in the heart of Sun?
---
diff --git a/git-am.sh b/git-am.sh
index 543efd0..8b57129 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -18,7 +18,7 @@ stop_here () {
stop_here_user_resolve () {
if [ -n "$resolvemsg" ]; then
- echo "$resolvemsg"
+ printf '%s\n' "$resolvemsg"
stop_here $1
fi
cmdline=$(basename $0)
@@ -146,7 +146,7 @@ do
git_apply_opt="$git_apply_opt $1"; shift ;;
--resolvemsg=*)
- resolvemsg=$(echo "$1" | sed -e "s/^--resolvemsg=//"); shift ;;
+ resolvemsg=${1#--resolvemsg=}; shift ;;
--)
shift; break ;;
diff --git a/git-commit.sh b/git-commit.sh
diff --git a/git-merge.sh b/git-merge.sh
index 44e9b70..981d69d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -335,7 +335,7 @@ f,*)
then
echo "Wonderful."
result_commit=$(
- echo "$merge_msg" |
+ printf '%s\n' "$merge_msg" |
git-commit-tree $result_tree -p HEAD -p "$1"
) || exit
finish "$result_commit" "In-index merge"
@@ -440,7 +440,7 @@ done
if test '' != "$result_tree"
then
parents=$(git-show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
- result_commit=$(echo "$merge_msg" | git-commit-tree $result_tree $parents) || exit
+ result_commit=$(printf '%s\n' "$merge_msg" | git-commit-tree $result_tree $parents) || exit
finish "$result_commit" "Merge made by $wt_strategy."
dropsave
exit 0
@@ -479,7 +479,7 @@ else
do
echo $remote
done >"$GIT_DIR/MERGE_HEAD"
- echo "$merge_msg" >"$GIT_DIR/MERGE_MSG"
+ printf '%s\n' "$merge_msg" >"$GIT_DIR/MERGE_MSG"
fi
if test "$merge_was_ok" = t
diff --git a/git-tag.sh b/git-tag.sh
index 4a0a7b6..6f0b7a7 100755
--- a/git-tag.sh
+++ b/git-tag.sh
@@ -126,7 +126,7 @@ if [ "$annotate" ]; then
echo "#" ) > "$GIT_DIR"/TAG_EDITMSG
${VISUAL:-${EDITOR:-vi}} "$GIT_DIR"/TAG_EDITMSG || exit
else
- echo "$message" >"$GIT_DIR"/TAG_EDITMSG
+ printf '%s\n' "$message" >"$GIT_DIR"/TAG_EDITMSG
fi
grep -v '^#' <"$GIT_DIR"/TAG_EDITMSG |
^ permalink raw reply related
* Re: Bug: git-rebase goofs up \n in commit messages
From: Junio C Hamano @ 2007-05-26 6:13 UTC (permalink / raw)
To: Jeff King; +Cc: Herbert Xu, Szekeres Istvan, git
In-Reply-To: <20070526060748.GA20715@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, May 25, 2007 at 09:59:43PM -0700, Junio C Hamano wrote:
>
>> * I suspect we would declare either "war on echo" or "harder push
>> for builtins" triggered by these.
>
> Cry havoc! More fixes below (just a diff -- maybe we want to aggregate
> these into a single commit?).
>
> These are the ones I noticed that use commit messages (which are
> probably the most likely to use backslash). There are _tons_ of uses for
> heads and filenames. I think we either should stop with commit messages,
> or go all-out and simply remove all uses of echo (because there are
> literally hundreds otherwise).
>
>> - echo "$commit_only" |
>> + printf "%s\n" "$commit_only" |
>
> Is "\n" portable to all shells (i.e., do you need '\n')? It works with
> bash and dash, which are by far the most common, but who knows what evil
> lurks in the heart of Sun?
Gaah, you are right.
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Herbert Xu @ 2007-05-26 6:19 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Szekeres Istvan, git
In-Reply-To: <20070526060748.GA20715@coredump.intra.peff.net>
On Sat, May 26, 2007 at 02:07:48AM -0400, Jeff King wrote:
>
> > - echo "$commit_only" |
> > + printf "%s\n" "$commit_only" |
>
> Is "\n" portable to all shells (i.e., do you need '\n')? It works with
> bash and dash, which are by far the most common, but who knows what evil
> lurks in the heart of Sun?
You mean the "\n" in printf? Yes that is specified in POSIX.
Without the "\n" printf will act like echo -n (which incidentally
is forbidden by POSIX).
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Jeff King @ 2007-05-26 6:27 UTC (permalink / raw)
To: Herbert Xu; +Cc: Junio C Hamano, Szekeres Istvan, git
In-Reply-To: <20070526061942.GA5986@gondor.apana.org.au>
On Sat, May 26, 2007 at 04:19:42PM +1000, Herbert Xu wrote:
> > Is "\n" portable to all shells (i.e., do you need '\n')? It works with
> > bash and dash, which are by far the most common, but who knows what evil
> > lurks in the heart of Sun?
>
> You mean the "\n" in printf? Yes that is specified in POSIX.
> Without the "\n" printf will act like echo -n (which incidentally
> is forbidden by POSIX).
No, I meant would the shell, while interpolating a double-quoted string
"\n", always preserve the string and pass the backslash and 'n' to
printf? Clearly \\ and \" get interpolated, but I don't know the rules
for "unrecognized" backslash sequences.
-Peff
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Herbert Xu @ 2007-05-26 7:38 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Szekeres Istvan, git
In-Reply-To: <20070526062748.GA21229@coredump.intra.peff.net>
On Sat, May 26, 2007 at 02:27:48AM -0400, Jeff King wrote:
>
> No, I meant would the shell, while interpolating a double-quoted string
> "\n", always preserve the string and pass the backslash and 'n' to
> printf? Clearly \\ and \" get interpolated, but I don't know the rules
> for "unrecognized" backslash sequences.
Yep. Section 2.2.3 says that the backslash within double quotes will
only serve as an escape character for $, `, ", \ and <newline>. Of
course, you can always use a single quote instead, i.e.,
printf '%s\n' "$VAR"
Then there can be no doubt :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Jeff King @ 2007-05-26 7:47 UTC (permalink / raw)
To: Herbert Xu; +Cc: Junio C Hamano, Szekeres Istvan, git
In-Reply-To: <20070526073855.GA6395@gondor.apana.org.au>
On Sat, May 26, 2007 at 05:38:55PM +1000, Herbert Xu wrote:
> Yep. Section 2.2.3 says that the backslash within double quotes will
> only serve as an escape character for $, `, ", \ and <newline>. Of
> course, you can always use a single quote instead, i.e.,
>
> printf '%s\n' "$VAR"
>
> Then there can be no doubt :)
Right, I wasn't sure if the single quotes were necessary, but it looks
like Junio's version (with doublequotes) is also fine according to
POSIX. Thanks again.
-Peff
^ permalink raw reply
* Re: Bug: git-rebase goofs up \n in commit messages
From: Junio C Hamano @ 2007-05-26 7:47 UTC (permalink / raw)
To: Jeff King; +Cc: Herbert Xu, Szekeres Istvan, git
In-Reply-To: <20070526060748.GA20715@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, May 25, 2007 at 09:59:43PM -0700, Junio C Hamano wrote:
>
>> * I suspect we would declare either "war on echo" or "harder push
>> for builtins" triggered by these.
>
> Cry havoc! More fixes below (just a diff -- maybe we want to aggregate
> these into a single commit?).
>
> These are the ones I noticed that use commit messages (which are
> probably the most likely to use backslash). There are _tons_ of uses for
> heads and filenames. I think we either should stop with commit messages,
> or go all-out and simply remove all uses of echo (because there are
> literally hundreds otherwise).
At least the ones you did look very sane to me. Will apply with
appropriate log message, credit to you.
Thanks.
I do not think we need to do all the uses of 'echo'. Many of
them are clearly fixed string we know about, object names we
parsed out of plumbing output, refnames and refspecs, all of
which should be safe.
Other worrisome ones are pathnames, but (1) I do not think
anybody is insane enough to have slashed funnies in their
pathname components, (2) half the pathnames we deal with come
from plumbing output which use '/' as path component separator
even on Windows, (3) users can use forward slash as path
component separator in their input even on Windows, and (4) even
though we try to use -z output from plumbing and read it with -0
capable downstream in some of our pipelines, many pure-shell
scripts read non-z output using shell built-in "read" and do not
unquote c-quoted ones, so they do not work correctly if you have
HT or LF in your pathnames anyway (notable exception is that
pipelines between git plumbing, e.g. "ls-files | update-index
--stdin", are safe without -z, as the downstream knows how to
unquote c-quoted paths).
I would expect that by the time we run out of more important
things to worry about and start worrying about truly funny
pathnames, we would have rewritten more of the remaining
Porcelains shell scripts in C, which automatically would make
this problem go away.
^ permalink raw reply
* Re: [PATCH] Fix mishandling of $Id$ expanded in the repository copy in convert.c
From: Junio C Hamano @ 2007-05-26 8:09 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
In-Reply-To: <200705251150.09439.andyparkins@gmail.com>
Andy Parkins <andyparkins@gmail.com> writes:
> I've included the comments I wrote while debugging in this patch, which
> I'm sure will annoy you, because you'd rather the fix and the comments
> separately. I'll supply that if you wish - just holler.
Actually I like well commented code, although some of your
comments feel a tad too much at places. For example,
> for (dst = buf; size; size--) {
> const char *cp;
> + /* Fetch next source character, move the pointer on */
> char ch = *src++;
> + /* Copy the current character to the destination */
> *dst++ = ch;
These are too much.
> + /* If the current character is "$" or there are less than three
> + * remaining bytes or the two bytes following this one are not
> + * "Id", then simply read the next character */
> if ((ch != '$') || (size < 3) || memcmp("Id", src, 2))
> continue;
> + /*
> + * Here when
> + * - There are more than 2 bytes remaining
> + * - The current three bytes are "$Id$"
> + * with
> + * - ch == "$"
> + * - src[0] == "I"
> + */
But this is very good, if you fix it to read the current 3 are
"$Id" ;-).
> + /*
> + * It's possible that an expanded Id has crept its way into the
> + * repository, we cope with that by stripping the expansion out
> + */
So are all the other comments.
Thanks for the fix. It would be very nice for the patch to be
accompanied with a new test to expose the bug and demonstrate
that the patch fixes it.
^ permalink raw reply
* Re: [PATCH] Move refspec pattern matching to match_refs().
From: Junio C Hamano @ 2007-05-26 8:20 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705250111200.9778@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> This means that send-pack and http-push will support pattern refspecs,
> so builtin-push.c doesn't have to expand them, and also git push can
> just turn --tags into "refs/tags/*", further simplifying
> builtin-push.c
>
> check_ref_format() gets a third "conditionally okay" result for
> something that's valid as a pattern but not as a particular ref.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> On top of my "remote" series. Shouldn't change any significant behavior,
> and simplifies a lot of logic. This version takes into account the
> comments from the previous round (assuming that the ruling on coding style
> is that:
>
> if (condition)
> /* Comment */
> statement;
>
> shouldn't have braces).
>
> builtin-push.c | 133 +++++++++----------------------------------------------
> refs.c | 27 ++++++++---
> remote.c | 31 ++++++++++++-
> send-pack.c | 1 +
> 4 files changed, 70 insertions(+), 122 deletions(-)
Whee. Removes lots more code than it adds. Will queue.
^ permalink raw reply
* Re: [PATCH 1/3] Lazily open pack index files on demand
From: Junio C Hamano @ 2007-05-26 8:29 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Dana How
In-Reply-To: <20070526052419.GA11957@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> This conflicts (in a subtle way) with Dana How's
> "sha1_file.c:rearrange_packed_git() should consider packs' object
> sizes" patch as we now have num_objects = 0 for any indexes we
> have not opened. In the case of Dana's patch this would cause
> those packfiles to have very high ranks, possibly sorting much
> later than they should have.
I am keeping that rearrange stuff on hold, partly because I am
moderately hesitant to do the fp, which feels overkill at that
low level of code.
Also, I am hoping that we can discard that the object density
criteria altogether by making the default repack behaviour
friendlier to the pathological cases, e.g. by emitting huge
blobs at the end of the packstream, potentially pushing it out
to later parts of split packs by themselves and automatically
marking them with the .keep flag. Until that kind of
improvements materialize, people with pathological cases could
(1) handcraft a pack that contains only megablob, (2) place that
on central alternate, (3) touch it with artificially old
timestamp, which hopefully is a good enough workaround.
^ permalink raw reply
* [RFH] QGit: how to cram a patch in a crowded screen
From: Marco Costalba @ 2007-05-26 8:46 UTC (permalink / raw)
To: Git Mailing List; +Cc: Pavel Roskin
Me and Pavel were discussing about usability improvements in the new
qgit (Qt4 based) that is going to replace the current Qt3 based one.
Currently there are no released tarballs, just a git repo:
git://git.kernel.org/pub/scm/qgit/qgit4.git
but it's already quite stable and feature complete and works also under Windows.
The biggest issue Pavel pointed out is that the main view does not
show patch content.
Currently qgit is tab based, so you have the main view tab with
revision header info:
http://digilander.libero.it/mcostalba/qgit4_1.png
Or without header info ('toggle 'h' key) for bigger screen estate
given to the revisions list:
http://digilander.libero.it/mcostalba/qgit4_2.png
Then to see the patch you have to switch to 'patch tab' ('p' key and
'r' key to go back to revisions list tab):
http://digilander.libero.it/mcostalba/qgit4_3.png
In case you need to give a deep look at the patch maybe it's worth
toggling the split view with 's' key:
http://digilander.libero.it/mcostalba/qgit4_4.png
Form any tab you can navigate with 'i' (move up one) and 'n', 'k'
(move down one) so that if you are interested in patch content only
you can avoid going back ('r') to main tab and stay in patch tab while
browsing the repo.
But for Pavel this is not enough, and I agree with him, because you
cannot see both the list and the patch content in one view.
Long time ago qgit was using independent overlapping windows to show
each kind of content, something similar to what git-gui uses now, then
I switched to tabs because I found myself spending more time in
arranging windows then in browsing contents.
The legacy way to do it is the gitk way: patch content below revision
log messages.
I tried to study that approach and to understand why it is practical
and handy, at least if you don't need to see the patch at full screen,
as I need in case I really want understand a difficult patch, but
probably this is a my limitation.
What I found, the 'secret' of space screen saving also with patch
content, is that when you look at the patch, scrolling down the
bottom-left pane, the revision logs and messages automatically fade
away because are scrolled out of the pane.
So it seems that you can see revision list + log messages + patch
content, but indeed what you see is revision list + log messages + *a
couple of patch lines* _OR_ revision list + patch content.
A natural consequence of this could be the introduction of another
shortcut to toggle messages and patch content in main view bottom left
pane....
After all this long introduction here we come to the subject of the e-mail.
We need some help, in terms of ideas, to better arrange the
information to be shown in main view so to improve repo browsability.
I could implement almost anything good comes up in this thread. But
still I don't figure out myself what is the best solution.
Any suggestion is greatly appreciated.
Marco
P.S: To be a clone of gitk is not necessarily a design spec.
^ permalink raw reply
* Re: [PATCH] Fix mishandling of $Id$ expanded in the repository copy in convert.c
From: Andy Parkins @ 2007-05-26 9:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7vlkfcm2eu.fsf@assigned-by-dhcp.cox.net>
On Saturday 2007, May 26, Junio C Hamano wrote:
> > for (dst = buf; size; size--) {
> > const char *cp;
> > + /* Fetch next source character, move the pointer on */
> > char ch = *src++;
> > + /* Copy the current character to the destination */
> > *dst++ = ch;
>
> These are too much.
Absolutely. I always find when bug hunting though that I like to
comment every block, sometimes to excess, as reminder that I've
understood what its doing.
I should have done the final pass once I'd found it to clear up the
overkill ones :-)
> But this is very good, if you fix it to read the current 3 are
> "$Id" ;-).
"and in the ability to count competition, Andy comes second... let's
have a big hand for our runner up" :-)
> Thanks for the fix. It would be very nice for the patch to be
> accompanied with a new test to expose the bug and demonstrate
> that the patch fixes it.
I had to jump through quite a few hoops to get the expanded $Id$ into a
repository (originally it was because I used an older version of git in
one place, and a newer one in another).
I'll see what I can do to make a test case though.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* Re: [RFH] QGit: how to cram a patch in a crowded screen
From: Andy Parkins @ 2007-05-26 9:34 UTC (permalink / raw)
To: git; +Cc: Marco Costalba, Pavel Roskin
In-Reply-To: <e5bfff550705260146q51350f40q1c80cfe8079f47c6@mail.gmail.com>
On Saturday 2007, May 26, Marco Costalba wrote:
> We need some help, in terms of ideas, to better arrange the
> information to be shown in main view so to improve repo browsability.
>
> I could implement almost anything good comes up in this thread. But
> still I don't figure out myself what is the best solution.
>
> Any suggestion is greatly appreciated.
While reading this I was reminded of something Linus said recently. The
default form for exchanging patches is _not_ the diff; it's the email.
The reason for this is that emails contain author information, log
information and patch information. I've often thought that one thing
we can be reasonably sure of is that the log message will be small.
Perhaps then, the best thing would be to show each revision in a
similar manner to how one would view the patch as an email as Pavel
suggests: all in one. My patches that made the header information
appear in a box were a stumble in that direction, making the header
look a bit like an email header, what about going one step further and
including the patch in that window as well?
For example, the log view widget would show:
<Header>
<Log Message>
<Patch>
All visually distinct to improve searching by eye (perhaps including
clear separators between files patched). Then the file list could
include a "<header>" psuedo-file that would jump back to the top of the
viewer.
I think a key feature would be that the log message shows up fast, and
then the patch is loaded in the background - sometimes patches are big,
but you still want to hop around revisions without waiting for the
whole patch to load before you leap to the next view point.
Here's another option, keeping the patch tab, but putting the tab widget
in the log view window. That way the list would be visible and you
would just switch between the log and the patch.
And another idea: make the log window a tree widget, of sorts, a bit
like kate in KDE is, each section would begin collapsed (perhaps), it
would look like:
+ <Header>
+ <Log message>
+ <Patch>
And clicking on the + would expand that section; then for the patch
itself:
+ <Patch>
+ <File1>
+ <File2>
+ <File3>
In this way the user could get an overview of all the changed files, and
could view the changes for whatever subset they wanted. It would
probably be nice to default the header and log message to expanded,
with the option to collapse them.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* Re: t9107-git-svn-migrate.sh fails
From: Eric Wong @ 2007-05-26 9:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705242315550.4648@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> I recently wanted to use git-svn, and installed the svn perl bindings.
> Since then, t9107 is failing:
>
> -- snip --
> [...]
> Rebuilding .git/svn/git-svn/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> ...
> r1 = b9b82a419abdbb54f51a41bc8a3118b28c791ac1
> Done rebuilding
> .git/svn/git-svn/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> diff --git
> a/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> b/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db
> index d3f1b6e..01d8afd 100644
> ---
> a/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> +++ b/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db
> @@ -1,2 +1 @@
> -0000000000000000000000000000000000000000
> -6aa651a66730888e854a8de54199d62ffa402739
> +.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> \ No newline at end of file
> * FAIL 7: .rev_db auto-converted to .rev_db.UUID
>
> git-svn fetch -i trunk &&
> [...]
> -- snap --
>
> Usually I try to fix things like this myself, but I really have to get
> some dinner now. Besides, other people than me seem to be way more clever
> with perl code.
.git/svn/trunk/.rev_db should be
a symlink to
.git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
I keep .rev_db around as a symlink for backwards compatibility in case
the user wants to downgrade.
I'm running cmp(1) to compare the file and symlink. Are you running
diff2[1] replacements in your tree and it's not understanding symlinks?
> Anybody knows how to fix this?
Works for me(TM).
[1] - I think that's what you called it. I have had trouble keeping
up-to-date with git things lately.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] Add git-submodule command
From: Lars Hjemli @ 2007-05-26 9:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Linus Torvalds, git
In-Reply-To: <7v1wh4ped4.fsf@assigned-by-dhcp.cox.net>
On 5/26/07, Junio C Hamano <junkio@cox.net> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > With this entry in .gitmodules (and a commit reference in the index entry for
> > the path "git"), the command 'git submodule init' will clone the repository
> > at kernel.org into the directory "git".
> >
> > Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> > ---
> >
> > On 5/26/07, Lars Hjemli <hjemli@gmail.com> wrote:
> >> I'll redo the patch, removing the branch-specific things, and try to
> >> shut up :)
>
> Hey, don't shut up. Starting small and covering corner cases
> incrementally is really the right approach.
Ok, my meds have kicked in and I'm ready for more ;-)
>
> > +status::
> > + Show the status of the submodules. This will print the sha1 of the
> > + currently checked out commit for each submodule, along with the
> > + submodule path and the output of gitlink:git-describe[1] for the
> > + sha1. Each sha1 will be prefixed with '-' if the submodule is not
> > + initialized and '+' if the currently checked out submodule commit
> > + does not match the sha1 found in the index of the containing
> > + repository. This command is the default command for git-submodule.
>
> (markup) probably you would want `` there for typewriter face.
Ok, will fix
>
> (wording) didn't we have "the name of the hash function is
> SHA-1" patch earlier? I'd personally prefer calling them
> "object names", though...
Some quick grepping wasn't helpfull:
~/src/git/Documentation$ git grep sha1 | wc -l
98
~/src/git/Documentation$ git grep SHA1 | wc -l
80
~/src/git/Documentation$ git grep SHA-1 | wc -l
22
Would you prefer SHA-1?
>
> Other than that, the command description is very nicely done,
> which means the design of the command set hence the semantics is
> cleanly done. Good job.
Thanks!
>
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > new file mode 100755
> > index 0000000..247b1ee
> > --- /dev/null
> > +++ b/git-submodule.sh
> > @@ -0,0 +1,172 @@
> > ...
> > +#
> > +# print stuff on stdout unless -q was specified
> > +#
> > +say()
> > +{
> > + if test -z "$quiet"
> > + then
> > + echo -e "$@"
> > + fi
> > +}
>
> We tend to avoid "echo -e" (not POSIX). I do not see any string
> you feed to this function that you would _want_ backslash
> escaped sequences (actually I would suspect you would not want
> them).
I do use \t between submodule path and the result of git-describe, but
it's not really needed. I'll drop it.
>
> > +
> > +#
> > +# Run clone + checkout on missing submodules
> > +#
> > +# $@ = requested paths (default to all)
> > +#
> > +modules_init()
> > +{
> > + git ls-files --stage -- $@ | grep -e '^160000 ' |
>
> Did you mean "$@", i.e. inside double-quotes?
That looks right, yes
> > + test -d "$path/.git" && continue
> > +
> > + if test -d "$path"
> > + then
> > + rmdir "$path" 2>/dev/null ||
> > + die "Directory '$path' exist, but not as a submodule"
> > + fi
>
> Could the currently checked-out $path be a symlink to another
> directory, and what does the code do in such a case?
This I will have to test, but I _suspect_ it would fail on "test -e
path" _unless_ the "test -d $path/.git" kicks in.
>
> > +
> > + test -e "$path" &&
> > + die "A file already exist at path '$path'"
>
> "test -e" is a relatively new invention and I tended to stay
> away from it, but it should be safe to use these days...
Would you prefer separate -f and -l instead? Hmm, that would match up
nicely with your concern about symlinks ;-)
>
> > + url=$(GIT_CONFIG=.gitmodules git-config module."$path".url)
> > + test -z "$url" &&
> > + die "No url found for submodule '$path' in .gitmodules"
> > +
> > + git-clone "$url" "$path" ||
> > + die "Clone of submodule '$path' failed"
>
> "git-clone -n" please, as you will checkout something different
> in the next step anyway.
Nice, I hadn't noticed -n, will fix
>
> > +
> > + $(unset GIT_DIR && cd "$path" && git-checkout -q "$sha1") ||
> > + die "Checkout of submodule '$path' failed"
>
> Lose $() around this, as it is not producing a string which is
> the name of the command to run. You do want a subshell here
> because of chdir, so instead of losing $(), replace them with
> ().
Heh, there's always something new to learn on this list, thanks.
--
larsh
^ permalink raw reply
* Re: t9107-git-svn-migrate.sh fails
From: Johannes Schindelin @ 2007-05-26 10:30 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20070526093534.GB12639@muzzle>
Hi,
On Sat, 26 May 2007, Eric Wong wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > I recently wanted to use git-svn, and installed the svn perl bindings.
> > Since then, t9107 is failing:
> >
> > -- snip --
> > [...]
> > Rebuilding .git/svn/git-svn/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> > ...
> > r1 = b9b82a419abdbb54f51a41bc8a3118b28c791ac1
> > Done rebuilding
> > .git/svn/git-svn/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> > diff --git
> > a/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> > b/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db
> > index d3f1b6e..01d8afd 100644
> > ---
> > a/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> > +++ b/home/gene099/my/git/t/trash/.git/svn/trunk/.rev_db
> > @@ -1,2 +1 @@
> > -0000000000000000000000000000000000000000
> > -6aa651a66730888e854a8de54199d62ffa402739
> > +.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
> > \ No newline at end of file
> > * FAIL 7: .rev_db auto-converted to .rev_db.UUID
> >
> > git-svn fetch -i trunk &&
> > [...]
> > -- snap --
> >
> > Usually I try to fix things like this myself, but I really have to get
> > some dinner now. Besides, other people than me seem to be way more clever
> > with perl code.
>
> .git/svn/trunk/.rev_db should be
> a symlink to
> .git/svn/trunk/.rev_db.f944dda1-3e31-0410-9475-8f268450faf6
>
> I keep .rev_db around as a symlink for backwards compatibility in case
> the user wants to downgrade.
>
> I'm running cmp(1) to compare the file and symlink. Are you running
> diff2[1] replacements in your tree and it's not understanding symlinks?
D'oh!
Yes, I did a wholesale replacement of all cmp and diff invocations to
git-diff.
Two thoughts:
- why don't you check it with readlink?
- git diff should really output something when a file is compared
to a symlink...
I guess the second is something for my TODO list.
> > Anybody knows how to fix this?
>
> Works for me(TM).
Changing the check back to cmp make it work here, too.
> [1] - I think that's what you called it. I have had trouble keeping
> up-to-date with git things lately.
Yep, I called it diff2. But we found out that we can DWIM git-diff to do
it; if in doubt, just say "git diff --no-index".
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add git-submodule command
From: Johannes Schindelin @ 2007-05-26 10:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lars Hjemli, Linus Torvalds, git
In-Reply-To: <7v1wh4ped4.fsf@assigned-by-dhcp.cox.net>
Hi,
On Fri, 25 May 2007, Junio C Hamano wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > +
> > +#
> > +# Run clone + checkout on missing submodules
> > +#
> > +# $@ = requested paths (default to all)
> > +#
> > +modules_init()
> > +{
> > + git ls-files --stage -- $@ | grep -e '^160000 ' |
>
> Did you mean "$@", i.e. inside double-quotes?
>
> Because this pattern would appear a lot in superproject support,
> it might be a good idea to give a new option, --subprojects, to
> git-ls-files to limit its output to 160000 entries, but that is
> a minor detail.
I think that makes sense. It would also help the next one:
>
> > + while read mode sha1 stage path
> > + do
>
> We would need to undo the shell-safety "quoted" output of paths
> here. I suspect it would be much easier to code this in Perl or
> Python, do the "grep -e" part above in the script, when we start
> caring about unwrapping c-quoting of path (or "ls-files -z").
I would prefer this to be in shell... so to be easier make it a builtin
later, when the script has evolved into a stable state. By introducing
--subprojects to git-ls-files, you can roll your own quoting just for
git-submodule, as needed.
> But that is a minor detail we could fix up later.
I agree. It makes for a nice incremental patch.
> > + test -d "$path/.git" && continue
> > +
> > + if test -d "$path"
> > + then
> > + rmdir "$path" 2>/dev/null ||
> > + die "Directory '$path' exist, but not as a submodule"
> > + fi
>
> Could the currently checked-out $path be a symlink to another
> directory, and what does the code do in such a case?
I am not quite sure if you should allow that... So, IMHO this is stuff to
discuss after the initial revision (and after we have a test case, so we
can play around with symlinks safely).
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Don't ignore write failure from git-diff, git-log, etc.
From: Jim Meyering @ 2007-05-26 11:45 UTC (permalink / raw)
To: git
Currently, when git-diff writes to a full device or gets an I/O error,
it fails to detect the write error:
$ git-diff |wc -c
3984
$ git-diff > /dev/full && echo ignored write failure
ignored write failure
git-log does the same thing:
$ git-log -n1 > /dev/full && echo ignored write failure
ignored write failure
Each git command should report such a failure.
Some already do, but with the patch below, they all do, and we
won't have to rely on code in each command's implementation to
perform the right incantation.
$ ./git-log -n1 > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
$ ./git-diff > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
You can demonstrate this with git's own --version output, too:
(but git --help detects the failure without this patch)
$ ./git --version > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
Note that the fcntl test (for whether the fileno may be closed) is
required in order to avoid EBADF upon closing an already-closed stdout,
as would happen for each git command that already closes stdout; I think
update-index was the one I noticed in the failure of t5400, before I
added that test.
Signed-off-by: Jim Meyering <jim@meyering.net>
---
git.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/git.c b/git.c
index 29b55a1..a7d6515 100644
--- a/git.c
+++ b/git.c
@@ -308,6 +308,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
for (i = 0; i < ARRAY_SIZE(commands); i++) {
struct cmd_struct *p = commands+i;
const char *prefix;
+ int status;
if (strcmp(p->cmd, cmd))
continue;
@@ -321,7 +322,15 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
die("%s must be run in a work tree", cmd);
trace_argv_printf(argv, argc, "trace: built-in: git");
- exit(p->fn(argc, argv, prefix));
+ status = p->fn(argc, argv, prefix);
+
+ /* Close stdout if necessary, and diagnose any failure. */
+ if (0 <= fcntl(fileno (stdout), F_GETFD)
+ && (ferror(stdout) || fclose(stdout)))
+ die("write failure on standard output: %s",
+ strerror(errno));
+
+ exit(status);
}
}
--
1.5.2.73.g18bece
^ permalink raw reply related
* Problems importing SVN repo via git-svnimport
From: Art Haas @ 2007-05-26 12:55 UTC (permalink / raw)
To: git
Hi.
I'm attempting to convert the Subversion repo of my project PythonCAD
(shameless plug http://www.pythoncad.org) into git, and have not had
much luck so far. Yesterday I installed an up-to-date set of SVN::Perl
modules and began trying to do the import.
On my local machine the repo is file:///mnt/src/svnrepo, and the
structure inside is 'pythoncad/trunk', 'pythoncad/branches', and
'pythoncad/tags'. For those playing at home, you can access the
public repo via http://subversion.pythoncad.org:9000/svn and you
should see the same layout. Way back when I started, though, the
initial directory was called 'pycad', and it lasted up through
revision 113, when in revision 114 it became 'pythoncad'. The next
eight or nine revisions involve me moving files around into the
new directory path. I don't remember exactly why I did the rename
right now (it's more than four years ago) but I think it was because I
found other 'pycad' projects/companies on the internet.
So, with 'git-svnimport' I've tried a number of different commands to
pull my data into git, and have only succeeded in getting a log history
into git but none of the actual file data makes it in. In my newly
built git repo I can do 'git log' and read all the checkin comments that
I made using 'svn commit'; git imports this data without problem. My
actual files, however, never appear. I poked around the git-svnimport
code a bit, added a few print statements here and there, and found that
the commit subroutine is failing. Specifically, during the loop where
the actions listed in the log output are scanned (around line 690),
the tests to determine the node_kind are always returning
$SVN::Node::none, not $SVN::Node::file or $SVN::Node::dir, so my
tree never gets populated with files and directories.
I tried importing only the first 114 revisions (the 'pycad' set) with
the following command:
$ git svnimport -C /tmp/pycad.git -l 114 -A authors -T pycad/trunk \
-b pycad/branches -t pycad/tags -v file:///mnt/src/svnrepo /pycad
I've also tried starting at rev 114 and going to the end (the
'pythoncad' set) but the end result is a git repo with a log file but no
file content.
I'm happy to dive into the perl code in svnimport and make changes to
help debug this problem, if there is actually a problem and not user
error on my part. Any help from 'svnimport' experts will be greatly
appreciated. The public repo listed above has the same contents as my
local repo, so feel free to poke around it to see how things are
structured.
Thanks in advance.
Art Haas
--
Man once surrendering his reason, has no remaining guard against absurdities
the most monstrous, and like a ship without rudder, is the sport of every wind.
-Thomas Jefferson to James Smith, 1822
^ permalink raw reply
* Re: Problems importing SVN repo via git-svnimport
From: Frank Lichtenheld @ 2007-05-26 13:06 UTC (permalink / raw)
To: Art Haas; +Cc: git
In-Reply-To: <20070526125553.GC10324@artsapartment.org>
On Sat, May 26, 2007 at 07:55:53AM -0500, Art Haas wrote:
> I'm happy to dive into the perl code in svnimport and make changes to
> help debug this problem, if there is actually a problem and not user
> error on my part. Any help from 'svnimport' experts will be greatly
> appreciated. The public repo listed above has the same contents as my
> local repo, so feel free to poke around it to see how things are
> structured.
You might want to try git-svn instead. It should support everything you
can do with git-svnimport and much more. Plus it is actively maintained
which I wouldn't say about git-svnimport.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
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