* Re: [PATCH 2/3] send-pack: check ref->status before updating tracking refs
From: Jeff King @ 2007-11-17 13:53 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117134546.GC2716@steel.home>
On Sat, Nov 17, 2007 at 02:45:46PM +0100, Alex Riesen wrote:
> > +test_expect_success 'deleted branches have their tracking branches removed' '
> > + git push origin :b1 &&
> > + test "$(git rev-parse origin/b1)" = "origin/b1"
> > +'
> > +
> > test_done
>
> Oh, missed that.
>
> Completely-Acked-By: Alex "Sleepy" Riesen <raa.lkml@gmail.com>
Yes, I didn't put it in the first patch, because it was broken there. ;)
-Peff
^ permalink raw reply
* Re: [PATCH 2/3] send-pack: check ref->status before updating tracking refs
From: Alex Riesen @ 2007-11-17 13:45 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117125515.GB23186@sigill.intra.peff.net>
Jeff King, Sat, Nov 17, 2007 13:55:15 +0100:
> diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
> index 799e47e..1493a92 100755
> --- a/t/t5404-tracking-branches.sh
> +++ b/t/t5404-tracking-branches.sh
> @@ -45,4 +45,9 @@ test_expect_success 'check tracking branches not updated for failed refs' '
> test "$(git rev-parse origin/b2)" = "$b2"
> '
>
> +test_expect_success 'deleted branches have their tracking branches removed' '
> + git push origin :b1 &&
> + test "$(git rev-parse origin/b1)" = "origin/b1"
> +'
> +
> test_done
Oh, missed that.
Completely-Acked-By: Alex "Sleepy" Riesen <raa.lkml@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/3] send-pack: track errors for each ref
From: Alex Riesen @ 2007-11-17 13:34 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117125426.GA23186@sigill.intra.peff.net>
Jeff King, Sat, Nov 17, 2007 13:54:27 +0100:
> Instead of keeping the 'ret' variable, we instead have a
> status flag for each ref that tracks what happened to it.
> We then print the ref status after all of the refs have
> been examined.
>
> This paves the way for three improvements:
> - updating tracking refs only for non-error refs
> - incorporating remote rejection into the printed status
> - printing errors in a different order than we processed
> (e.g., consolidating non-ff errors near the end with
> a special message)
>
> Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Alex Riesen <raa.lkml@gmail.com>
Still would like to see the deletion of tracing branches checked.
Like this perhaps?
diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index 799e47e..4fe4a07 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -10,6 +10,7 @@ test_expect_success 'setup' '
git commit -m 1 &&
git branch b1 &&
git branch b2 &&
+ git branch b3 &&
git clone . aa &&
git checkout b1 &&
echo b1 >>file &&
@@ -23,6 +24,7 @@ test_expect_success 'prepare pushable branches' '
cd aa &&
b1=$(git rev-parse origin/b1) &&
b2=$(git rev-parse origin/b2) &&
+ b3=$(git rev-parse origin/b3) &&
git checkout -b b1 origin/b1 &&
echo aa-b1 >>file &&
git commit -a -m aa-b1 &&
@@ -45,4 +47,12 @@ test_expect_success 'check tracking branches not updated for failed refs' '
test "$(git rev-parse origin/b2)" = "$b2"
'
+test_expect_success 'delete remote branch' '
+ git push origin :refs/heads/b3 &&
+ {
+ git rev-parse --verify origin/b3
+ test $? != 0
+ }
+'
+
test_done
--
1.5.3.5.747.gf06543
^ permalink raw reply related
* Re: [PATCH] Fix t9101 test failure caused by Subversion "auto-props"
From: Wincent Colaiuta @ 2007-11-17 13:28 UTC (permalink / raw)
To: Junio C Hamano
Cc: Björn Steinbrink, Benoit Sigoure, Git Mailing List,
Väinö Järvelä
In-Reply-To: <7vy7cxwwoo.fsf@gitster.siamese.dyndns.org>
El 17/11/2007, a las 1:19, Junio C Hamano escribió:
> Wincent Colaiuta <win@wincent.com> writes:
>
>> If a user has an "auto-prop" in his/her ~/.subversion/config file for
>> automatically setting the svn:keyword Id property on all ".c" files
>> (a reasonably common configuration in the Subversion world) then one
>> of the "svn propset" operations in the very first test would become a
>> no-op, which in turn would make the next commit a no-op.
>
> Thanks for diagnosing and fixing.
>
> I presume this fix also applies to both 'maint' and 'master',
> right?
I prepared it against master, but I believe it will apply cleanly to
maint, where it's also needed.
Cheers,
Wincent
^ permalink raw reply
* [PATCH 3/3] send-pack: assign remote errors to each ref
From: Jeff King @ 2007-11-17 12:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alex Riesen, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117125323.GA23125@sigill.intra.peff.net>
This lets us show remote errors (e.g., a denied hook) along
with the usual push output.
There is a slightly clever optimization in receive_status
that bears explanation. We need to correlate the returned
status and our ref objects, which naively could be an O(m*n)
operation. However, since the current implementation of
receive-pack returns the errors to us in the same order that
we sent them, we optimistically look for the next ref to be
looked up to come after the last one we have found. So it
should be an O(m+n) merge if the receive-pack behavior
holds, but we fall back to a correct but slower behavior if
it should change.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-send-pack.c | 51 +++++++++++++++++++++++++++++++++++++++-----
cache.h | 2 +
t/t5406-remote-rejects.sh | 24 +++++++++++++++++++++
3 files changed, 71 insertions(+), 6 deletions(-)
create mode 100755 t/t5406-remote-rejects.sh
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index c7d07aa..bcf7143 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -146,19 +146,43 @@ static void get_local_heads(void)
for_each_ref(one_local_ref, NULL);
}
-static int receive_status(int in)
+static struct ref *set_ref_error(struct ref *refs, const char *line)
{
+ struct ref *ref;
+
+ for (ref = refs; ref; ref = ref->next) {
+ const char *msg;
+ if (prefixcmp(line, ref->name))
+ continue;
+ msg = line + strlen(ref->name);
+ if (*msg++ != ' ')
+ continue;
+ ref->status = REF_STATUS_REMOTE_REJECT;
+ ref->error = xstrdup(msg);
+ ref->error[strlen(ref->error)-1] = '\0';
+ return ref;
+ }
+ return NULL;
+}
+
+/* a return value of -1 indicates that an error occurred,
+ * but we were able to set individual ref errors. A return
+ * value of -2 means we couldn't even get that far. */
+static int receive_status(int in, struct ref *refs)
+{
+ struct ref *hint;
char line[1000];
int ret = 0;
int len = packet_read_line(in, line, sizeof(line));
if (len < 10 || memcmp(line, "unpack ", 7)) {
fprintf(stderr, "did not receive status back\n");
- return -1;
+ return -2;
}
if (memcmp(line, "unpack ok\n", 10)) {
fputs(line, stderr);
ret = -1;
}
+ hint = NULL;
while (1) {
len = packet_read_line(in, line, sizeof(line));
if (!len)
@@ -171,7 +195,10 @@ static int receive_status(int in)
}
if (!memcmp(line, "ok", 2))
continue;
- fputs(line, stderr);
+ if (hint)
+ hint = set_ref_error(hint, line + 3);
+ if (!hint)
+ hint = set_ref_error(refs, line + 3);
ret = -1;
}
return ret;
@@ -297,6 +324,12 @@ static void print_push_status(const char *dest, struct ref *refs)
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast forward");
break;
+ case REF_STATUS_REMOTE_REJECT:
+ if (ref->deletion)
+ print_ref_status('!', "[remote rejected]", ref, NULL, ref->error);
+ else
+ print_ref_status('!', "[remote rejected]", ref, ref->peer_ref, ref->error);
+ break;
case REF_STATUS_OK:
print_ok_ref_status(ref);
break;
@@ -312,6 +345,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
int allow_deleting_refs = 0;
int expect_status_report = 0;
int flags = MATCH_REFS_NONE;
+ int ret;
if (args.send_all)
flags |= MATCH_REFS_ALL;
@@ -429,12 +463,15 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
}
close(out);
- print_push_status(dest, remote_refs);
-
if (expect_status_report) {
- if (receive_status(in))
+ ret = receive_status(in, remote_refs);
+ if (ret == -2)
return -1;
}
+ else
+ ret = 0;
+
+ print_push_status(dest, remote_refs);
if (!args.dry_run && remote) {
for (ref = remote_refs; ref; ref = ref->next)
@@ -443,6 +480,8 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
if (!new_refs)
fprintf(stderr, "Everything up-to-date\n");
+ if (ret < 0)
+ return ret;
for (ref = remote_refs; ref; ref = ref->next) {
switch (ref->status) {
case REF_STATUS_NONE:
diff --git a/cache.h b/cache.h
index 2768342..ba9178f 100644
--- a/cache.h
+++ b/cache.h
@@ -510,7 +510,9 @@ struct ref {
REF_STATUS_REJECT_NONFASTFORWARD,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
+ REF_STATUS_REMOTE_REJECT,
} status;
+ char *error;
struct ref *peer_ref; /* when renaming */
char name[FLEX_ARRAY]; /* more */
};
diff --git a/t/t5406-remote-rejects.sh b/t/t5406-remote-rejects.sh
new file mode 100755
index 0000000..46b2cb4
--- /dev/null
+++ b/t/t5406-remote-rejects.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='remote push rejects are reported by client'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ mkdir .git/hooks &&
+ (echo "#!/bin/sh" ; echo "exit 1") >.git/hooks/update &&
+ chmod +x .git/hooks/update &&
+ echo 1 >file &&
+ git add file &&
+ git commit -m 1 &&
+ git clone . child &&
+ cd child &&
+ echo 2 >file &&
+ git commit -a -m 2
+'
+
+test_expect_success 'push reports error' '! git push 2>stderr'
+
+test_expect_success 'individual ref reports error' 'grep rejected stderr'
+
+test_done
--
1.5.3.5.1795.gf2a4e-dirty
^ permalink raw reply related
* [PATCH 2/3] send-pack: check ref->status before updating tracking refs
From: Jeff King @ 2007-11-17 12:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alex Riesen, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117125323.GA23125@sigill.intra.peff.net>
Previously, we manually checked the 'NONE' and 'UPTODATE'
conditions. Now that we have ref->status, we can easily
say "only update if we pushed successfully".
This adds a test for and fixes a regression introduced in
ed31df31 where deleted refs did not have their tracking
branches removed. This was due to a bogus per-ref error test
that is superseded by the more accurate ref->status flag.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-send-pack.c | 18 +++++-------------
t/t5404-tracking-branches.sh | 5 +++++
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 90ca2d3..c7d07aa 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -180,24 +180,17 @@ static int receive_status(int in)
static void update_tracking_ref(struct remote *remote, struct ref *ref)
{
struct refspec rs;
- int will_delete_ref;
- rs.src = ref->name;
- rs.dst = NULL;
-
- if (!ref->peer_ref)
+ if (ref->status != REF_STATUS_OK)
return;
- will_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);
-
- if (!will_delete_ref &&
- !hashcmp(ref->old_sha1, ref->peer_ref->new_sha1))
- return;
+ rs.src = ref->name;
+ rs.dst = NULL;
if (!remote_find_tracking(remote, &rs)) {
if (args.verbose)
fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
- if (is_null_sha1(ref->peer_ref->new_sha1)) {
+ if (ref->deletion) {
if (delete_ref(rs.dst, NULL))
error("Failed to delete");
} else
@@ -445,8 +438,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
if (!args.dry_run && remote) {
for (ref = remote_refs; ref; ref = ref->next)
- if (!is_null_sha1(ref->new_sha1))
- update_tracking_ref(remote, ref);
+ update_tracking_ref(remote, ref);
}
if (!new_refs)
diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index 799e47e..1493a92 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -45,4 +45,9 @@ test_expect_success 'check tracking branches not updated for failed refs' '
test "$(git rev-parse origin/b2)" = "$b2"
'
+test_expect_success 'deleted branches have their tracking branches removed' '
+ git push origin :b1 &&
+ test "$(git rev-parse origin/b1)" = "origin/b1"
+'
+
test_done
--
1.5.3.5.1795.gf2a4e-dirty
^ permalink raw reply related
* [PATCH 1/3] send-pack: track errors for each ref
From: Jeff King @ 2007-11-17 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alex Riesen, Pierre Habouzit, Daniel Barkalow
In-Reply-To: <20071117125323.GA23125@sigill.intra.peff.net>
Instead of keeping the 'ret' variable, we instead have a
status flag for each ref that tracks what happened to it.
We then print the ref status after all of the refs have
been examined.
This paves the way for three improvements:
- updating tracking refs only for non-error refs
- incorporating remote rejection into the printed status
- printing errors in a different order than we processed
(e.g., consolidating non-ff errors near the end with
a special message)
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-send-pack.c | 225 +++++++++++++++++++++++++-----------------
cache.h | 13 ++-
t/t5404-tracking-branches.sh | 14 ++-
3 files changed, 157 insertions(+), 95 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 418925e..90ca2d3 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -207,8 +207,9 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
}
}
-static const char *prettify_ref(const char *name)
+static const char *prettify_ref(const struct ref *ref)
{
+ const char *name = ref->name;
return name + (
!prefixcmp(name, "refs/heads/") ? 11 :
!prefixcmp(name, "refs/tags/") ? 10 :
@@ -218,15 +219,105 @@ static const char *prettify_ref(const char *name)
#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
+static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg)
+{
+ fprintf(stderr, " %c %-*s ", flag, SUMMARY_WIDTH, summary);
+ if (from)
+ fprintf(stderr, "%s -> %s", prettify_ref(from), prettify_ref(to));
+ else
+ fputs(prettify_ref(to), stderr);
+ if (msg) {
+ fputs(" (", stderr);
+ fputs(msg, stderr);
+ fputc(')', stderr);
+ }
+ fputc('\n', stderr);
+}
+
+static const char *status_abbrev(unsigned char sha1[20])
+{
+ const char *abbrev;
+ abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
+ return abbrev ? abbrev : sha1_to_hex(sha1);
+}
+
+static void print_ok_ref_status(struct ref *ref)
+{
+ if (ref->deletion)
+ print_ref_status('-', "[deleted]", ref, NULL, NULL);
+ else if (is_null_sha1(ref->old_sha1))
+ print_ref_status('*',
+ (!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
+ "[new branch]"),
+ ref, ref->peer_ref, NULL);
+ else {
+ char quickref[83];
+ char type;
+ const char *msg;
+
+ strcpy(quickref, status_abbrev(ref->old_sha1));
+ if (ref->nonfastforward) {
+ strcat(quickref, "...");
+ type = '+';
+ msg = " (forced update)";
+ }
+ else {
+ strcat(quickref, "..");
+ type = ' ';
+ msg = NULL;
+ }
+ strcat(quickref, status_abbrev(ref->new_sha1));
+
+ print_ref_status(type, quickref, ref, ref->peer_ref, msg);
+ }
+}
+
+static void print_push_status(const char *dest, struct ref *refs)
+{
+ struct ref *ref;
+ int shown_dest = 0;
+
+ for (ref = refs; ref; ref = ref->next) {
+ if (!ref->status)
+ continue;
+ if (ref->status == REF_STATUS_UPTODATE && !args.verbose)
+ continue;
+
+ if (!shown_dest) {
+ fprintf(stderr, "To %s\n", dest);
+ shown_dest = 1;
+ }
+
+ switch(ref->status) {
+ case REF_STATUS_NONE:
+ print_ref_status('X', "[no match]", ref, NULL, NULL);
+ break;
+ case REF_STATUS_REJECT_NODELETE:
+ print_ref_status('!', "[rejected]", ref, NULL,
+ "remote does not support deleting refs");
+ break;
+ case REF_STATUS_UPTODATE:
+ print_ref_status('=', "[up to date]", ref,
+ ref->peer_ref, NULL);
+ break;
+ case REF_STATUS_REJECT_NONFASTFORWARD:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "non-fast forward");
+ break;
+ case REF_STATUS_OK:
+ print_ok_ref_status(ref);
+ break;
+ }
+ }
+}
+
static int do_send_pack(int in, int out, struct remote *remote, const char *dest, int nr_refspec, const char **refspec)
{
struct ref *ref;
int new_refs;
- int ret = 0;
int ask_for_status_report = 0;
int allow_deleting_refs = 0;
int expect_status_report = 0;
- int shown_dest = 0;
int flags = MATCH_REFS_NONE;
if (args.send_all)
@@ -262,10 +353,6 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
*/
new_refs = 0;
for (ref = remote_refs; ref; ref = ref->next) {
- char old_hex[60], *new_hex;
- int will_delete_ref;
- const char *pretty_ref;
- const char *pretty_peer = NULL; /* only used when not deleting */
const unsigned char *new_sha1;
if (!ref->peer_ref) {
@@ -276,29 +363,15 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
else
new_sha1 = ref->peer_ref->new_sha1;
- if (!shown_dest) {
- fprintf(stderr, "To %s\n", dest);
- shown_dest = 1;
- }
-
- will_delete_ref = is_null_sha1(new_sha1);
- pretty_ref = prettify_ref(ref->name);
- if (!will_delete_ref)
- pretty_peer = prettify_ref(ref->peer_ref->name);
-
- if (will_delete_ref && !allow_deleting_refs) {
- fprintf(stderr, " ! %-*s %s (remote does not support deleting refs)\n",
- SUMMARY_WIDTH, "[rejected]", pretty_ref);
- ret = -2;
+ ref->deletion = is_null_sha1(new_sha1);
+ if (ref->deletion && !allow_deleting_refs) {
+ ref->status = REF_STATUS_REJECT_NODELETE;
continue;
}
- if (!will_delete_ref &&
+ if (!ref->deletion &&
!hashcmp(ref->old_sha1, new_sha1)) {
- if (args.verbose)
- fprintf(stderr, " = %-*s %s -> %s\n",
- SUMMARY_WIDTH, "[up to date]",
- pretty_peer, pretty_ref);
+ ref->status = REF_STATUS_UPTODATE;
continue;
}
@@ -321,33 +394,26 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
* always allowed.
*/
- if (!args.force_update &&
- !will_delete_ref &&
+ ref->nonfastforward =
+ !ref->deletion &&
!is_null_sha1(ref->old_sha1) &&
- !ref->force) {
- if (!has_sha1_file(ref->old_sha1) ||
- !ref_newer(new_sha1, ref->old_sha1)) {
- /* We do not have the remote ref, or
- * we know that the remote ref is not
- * an ancestor of what we are trying to
- * push. Either way this can be losing
- * commits at the remote end and likely
- * we were not up to date to begin with.
- */
- fprintf(stderr, " ! %-*s %s -> %s (non-fast forward)\n",
- SUMMARY_WIDTH, "[rejected]",
- pretty_peer, pretty_ref);
- ret = -2;
- continue;
- }
+ (!has_sha1_file(ref->old_sha1)
+ || !ref_newer(new_sha1, ref->old_sha1));
+
+ if (ref->nonfastforward && !ref->force && !args.force_update) {
+ ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ continue;
}
+
hashcpy(ref->new_sha1, new_sha1);
- if (!will_delete_ref)
+ if (!ref->deletion)
new_refs++;
- strcpy(old_hex, sha1_to_hex(ref->old_sha1));
- new_hex = sha1_to_hex(ref->new_sha1);
+ ref->status = REF_STATUS_OK;
if (!args.dry_run) {
+ char *old_hex = sha1_to_hex(ref->old_sha1);
+ char *new_hex = sha1_to_hex(ref->new_sha1);
+
if (ask_for_status_report) {
packet_write(out, "%s %s %s%c%s",
old_hex, new_hex, ref->name, 0,
@@ -359,64 +425,43 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
packet_write(out, "%s %s %s",
old_hex, new_hex, ref->name);
}
- if (will_delete_ref)
- fprintf(stderr, " - %-*s %s\n",
- SUMMARY_WIDTH, "[deleting]",
- pretty_ref);
- else if (is_null_sha1(ref->old_sha1)) {
- const char *msg;
-
- if (!prefixcmp(ref->name, "refs/tags/"))
- msg = "[new tag]";
- else
- msg = "[new branch]";
- fprintf(stderr, " * %-*s %s -> %s\n",
- SUMMARY_WIDTH, msg,
- pretty_peer, pretty_ref);
- }
- else {
- char quickref[83];
- char type = ' ';
- const char *msg = "";
- const char *old_abb;
- old_abb = find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV);
- strcpy(quickref, old_abb ? old_abb : old_hex);
- if (ref_newer(ref->peer_ref->new_sha1, ref->old_sha1))
- strcat(quickref, "..");
- else {
- strcat(quickref, "...");
- type = '+';
- msg = " (forced update)";
- }
- strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
-
- fprintf(stderr, " %c %-*s %s -> %s%s\n",
- type,
- SUMMARY_WIDTH, quickref,
- pretty_peer, pretty_ref,
- msg);
- }
}
packet_flush(out);
- if (new_refs && !args.dry_run)
- ret = pack_objects(out, remote_refs);
+ if (new_refs && !args.dry_run) {
+ if (pack_objects(out, remote_refs) < 0) {
+ close(out);
+ return -1;
+ }
+ }
close(out);
+ print_push_status(dest, remote_refs);
+
if (expect_status_report) {
if (receive_status(in))
- ret = -4;
+ return -1;
}
- if (!args.dry_run && remote && ret == 0) {
+ if (!args.dry_run && remote) {
for (ref = remote_refs; ref; ref = ref->next)
if (!is_null_sha1(ref->new_sha1))
update_tracking_ref(remote, ref);
}
- if (!new_refs && ret == 0)
+ if (!new_refs)
fprintf(stderr, "Everything up-to-date\n");
- return ret;
+ for (ref = remote_refs; ref; ref = ref->next) {
+ switch (ref->status) {
+ case REF_STATUS_NONE:
+ case REF_STATUS_UPTODATE:
+ case REF_STATUS_OK:
+ break;
+ default:
+ return -1;
+ }
+ }
+ return 0;
}
static void verify_remote_names(int nr_heads, const char **heads)
diff --git a/cache.h b/cache.h
index e9b3ce3..2768342 100644
--- a/cache.h
+++ b/cache.h
@@ -500,8 +500,17 @@ struct ref {
struct ref *next;
unsigned char old_sha1[20];
unsigned char new_sha1[20];
- unsigned char force;
- unsigned char merge;
+ unsigned char force : 1;
+ unsigned char merge : 1;
+ unsigned char nonfastforward : 1;
+ unsigned char deletion : 1;
+ enum {
+ REF_STATUS_NONE = 0,
+ REF_STATUS_OK,
+ REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_NODELETE,
+ REF_STATUS_UPTODATE,
+ } status;
struct ref *peer_ref; /* when renaming */
char name[FLEX_ARRAY]; /* more */
};
diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index 20718d4..799e47e 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
git commit -a -m b2
'
-test_expect_success 'check tracking branches updated correctly after push' '
+test_expect_success 'prepare pushable branches' '
cd aa &&
b1=$(git rev-parse origin/b1) &&
b2=$(git rev-parse origin/b2) &&
@@ -31,8 +31,16 @@ test_expect_success 'check tracking branches updated correctly after push' '
git commit -a -m aa-b2 &&
git checkout master &&
echo aa-master >>file &&
- git commit -a -m aa-master &&
- git push &&
+ git commit -a -m aa-master
+'
+
+test_expect_success 'mixed-success push returns error' '! git push'
+
+test_expect_success 'check tracking branches updated correctly after push' '
+ test "$(git rev-parse origin/master)" = "$(git rev-parse master)"
+'
+
+test_expect_success 'check tracking branches not updated for failed refs' '
test "$(git rev-parse origin/b1)" = "$b1" &&
test "$(git rev-parse origin/b2)" = "$b2"
'
--
1.5.3.5.1795.gf2a4e-dirty
^ permalink raw reply related
* [PATCH v3 0/3] tracking per-ref errors on push
From: Jeff King @ 2007-11-17 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alex Riesen, Pierre Habouzit, Daniel Barkalow
Here are cleaned-up versions of the previous patches I sent. The
improvements are:
1/3 send-pack: track errors for each ref
- there was a t5404 regression because I removed the is_null_sha1()
check before updating a tracking ref (but the replacement fix
doesn't come until 2/3). Even though this check is not correct,
it's better to fix it all at once correctly in 2/3.
- clarified the desired git-push exit code in t5404
- I renamed the struct elements to (hopefully) be a bit more obvious
- readability cleanups (fixed some very long lines, hoisted some
code into its own functions)
2/3 send-pack: check ref->status before updating tracking refs
- moved in fix from 1/3 mentioned above
- add test for deleting tracking branches, which was broken in next
but fixed by this patch
3/3 send-pack: assign remote errors to each ref
- squashed optimization patch
- remove bogus parsing drawback in commit message
- add test
I'm hoping to get feedback from the cc'd people:
- Alex: please OK the modifications to t5404
- Pierre: this should fix the tracking ref update issues you reported
- Daniel: a general OK, since I am mangling your code :)
-Peff
^ permalink raw reply
* Re: New repo quickly corrupted
From: Christian Couder @ 2007-11-17 12:53 UTC (permalink / raw)
To: Andreas Ericsson
Cc: Linus Torvalds, Nicolas Pitre, Jason Sewall, git, Junio C Hamano
In-Reply-To: <473D4827.1060109@op5.se>
Le vendredi 16 novembre 2007, Andreas Ericsson a écrit :
> Christian Couder wrote:
> > Le jeudi 15 novembre 2007, Linus Torvalds a écrit :
> >> On Thu, 15 Nov 2007, Nicolas Pitre wrote:
> >>> Does "dos2unix" override file access bits? Because the object store
> >>> is always made read-only.
> >>
> >> Almost all programs like that will entirely ignor the fact that
> >> something is read-only.
> >
> > What if the .git/objects/ sudirectories were also read-only ?
>
> Then git wouldn't be able to write to it without chmod()'ing it each
> time.
Yes, but some (not manly enough) people might want the extra safety even if
it means a performance penalty.
Christian.
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Jeff King @ 2007-11-17 12:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfxz89x9q.fsf@gitster.siamese.dyndns.org>
On Wed, Nov 14, 2007 at 04:18:25PM -0800, Junio C Hamano wrote:
> * jk/send-pack (Tue Nov 13 06:37:10 2007 -0500) 24 commits
> [...]
> This three-patch series is built on top of four other topics and
> is meant to fix issues in built-in send-pack. I dropped
> individial topics from Alex, Daniel, Andy and another from Jeff
> that this series depends on. IOW, they all will graduate to
> "master" at the same time when this series proves to be stable.
Thank you, it was getting confusing with so many people working in the
same area. :)
> Will wait for a few days to hear opinions from the list, and
> then merge to "next" and start cooking.
I am about to send out an improved patch set that incorporates some of
the test fixes from Alex, some new tests from me, and a few code
cleanups.
-Peff
^ permalink raw reply
* Re: [PATCH] Rewrite some function exit paths to avoid "unreachable code" traps
From: Alex Riesen @ 2007-11-17 12:23 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Guido Ostkamp, Junio C Hamano, git
In-Reply-To: <200711171139.32631.robin.rosenberg.lists@dewire.com>
Robin Rosenberg, Sat, Nov 17, 2007 11:39:32 +0100:
> lördag 17 november 2007 skrev Alex Riesen:
> > Noticed by Guido Ostkamp for Sun's Workshop cc.
> >
> > Originally-by: Guido Ostkamp <git@ostkamp.fastmail.fm>
> > Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> > ---
> > Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:
> > >
> > > What about the xdiff/xdiffi.c problem that should also be solved?
> > >
> >
>
> Please... This just looks bad. I'm sure we'll have fixup patches on the list
> to fix those gotos.
>
> Do we support any such stupid compiler that requires a dummy goto?
It is more for the compilers we don't know about yet.
Userspace programming, especially with intent to be portable, often
means supporting *bugs* of the platform where it happens.
^ permalink raw reply
* Re: [PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.
From: Paul Mackerras @ 2007-11-17 11:44 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <20071102222436.23191.91785.stgit@gandelf.nowhere.earth>
Yann Dirson writes:
> This allows to display a set of refs, when the refs in the set may
> themselves change between two refresh operations (eg. the set of
> patches in a patch stack), and is expected to be called from other
> porcelain suites.
>
> The command is expected to generate a list of commits, which will be
> appended to the commits litterally passed on the command-line. That
> command is handled similarly to the litteral refs, and has its own
> field in the saved view, and an edit field in the view editor.
The idea is fine, but I have some comments on the implementation:
> +--argscmd=<command>::
> + Command to be run each time gitk has to determine the list of
> + <revs> to show. The command is expected to print on its
> + standard output a list of additional revs to be shown. Use
, one per line
> + this instead of explicitely specifying <revs> if the set of
explicitly
> + commits to show may vary between refreshs.
refreshes
> + set args $viewargs($view)
> + if {$viewargscmd($view) ne "None"} {
> + if {[catch {
> + set fd [open [concat | $viewargscmd($view)] r]
> + } err]} {
> + puts stderr "Error executing --argscmd command: $err"
> + exit 1
> + }
> + set args [concat $args [read $fd 500000]]
I don't think you necessarily want to limit the number of characters
read to 500000, do you?
What this will do is interpret the output of the program according to
Tcl list syntax. I think it would be better to use [split $str "\n"]
to split the program's output at the newlines. Also, you could
combine the open, read and close into a single exec call. Thirdly,
use error_popup rather than just writing to stderr.
I wonder if you should be invoking a shell to interpret the command.
As you have it at the moment, the string that the user puts after
--argscmd= is treated as a Tcl list, whereas the string they put into
the entry field when creating a new view is split with shellsplit,
which implements shell-style quoting. I think we should at least be
consistent here, and possibly just leave it as a string and pass it to
sh -c.
> +set revtreeargscmd None
Why the string "None" rather than the empty string? Is this a
python-ism that has crept in?
> foreach arg $argv {
> - switch -- $arg {
> - "" { }
> - "-d" { set datemode 1 }
> - "--merge" {
> + switch -regexp -- $arg {
Hmmm, it'd be simpler to use switch -glob here, I think.
> + "^$" { }
> + "^-d$" { set datemode 1 }
> + "^--merge$" {
> set mergeonly 1
> lappend revtreeargs $arg
> }
> - "--" {
> + "^--$" {
> set cmdline_files [lrange $argv [expr {$i + 1}] end]
> break
> }
> + "^--argscmd=" {
> + regexp {^--argscmd=(.*)} $arg match revtreeargscmd
set revtreeargscmd [string range $arg 10 end]
Paul.
^ permalink raw reply
* Re: [PATCH] Rewrite some function exit paths to avoid "unreachable code" traps
From: Robin Rosenberg @ 2007-11-17 10:39 UTC (permalink / raw)
To: Alex Riesen; +Cc: Guido Ostkamp, Junio C Hamano, git
In-Reply-To: <20071117094617.GD4086@steel.home>
lördag 17 november 2007 skrev Alex Riesen:
> Noticed by Guido Ostkamp for Sun's Workshop cc.
>
> Originally-by: Guido Ostkamp <git@ostkamp.fastmail.fm>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
> Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:
> >
> > What about the xdiff/xdiffi.c problem that should also be solved?
> >
>
Please... This just looks bad. I'm sure we'll have fixup patches on the list
to fix those gotos.
Do we support any such stupid compiler that requires a dummy goto?
If so we could just add a macro to compat-util.h
#if stupid_compiler
#define DUMMY_RETURN(x) return x;
#else
#define DUMMY_RETURN(x)
#endif
and then use it like this:
diff --git a/builtin-apply.c b/builtin-apply.c
index 8edcc08..91f8752 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -683,7 +683,6 @@ static char *git_header_name(char *line, int llen)
}
}
}
- return NULL;
+ DUMMY_RETURN(NULL)
}
My vote is for Guidos patch and fallback to the suggestion above if we support
really stupid compilers.
-- robin
^ permalink raw reply related
* Re: git-cvsimport bug with dates
From: Robin Rosenberg @ 2007-11-17 10:12 UTC (permalink / raw)
To: Elijah Newren; +Cc: Junio C Hamano, git
In-Reply-To: <51419b2c0711160612r1a80bd5o686040f945e8d9c3@mail.gmail.com>
fredag 16 november 2007 skrev Elijah Newren:
> On Nov 15, 2007 11:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > When you use "branch@{date}" notation, you are not asking a
> > question on the project history, but a question on the local
> > view from _your_ repository on that project.
>
> Interesting; that makes sense from a merge or pull viewpoint, but
> wouldn't it make more sense to have cvsimport ensure the commits are
> treated as though they actually existed in master as of the date
> specified in CVS?
Reflog do not work that way. They don't say when a commit entered a repo,
only when a ref changed. For a CVS import things could work as you suggest
but I think the confusion among newcomers would be massive if people start
using reflogs 'as if' it said anyting about when a commit entered. It can be used
as a hint.
Reflogs get pruned by git-gc and are not cloned (cannot be since they store
local-only information.
Use the --since and --until switches instead.
-- robin
^ permalink raw reply
* [PATCH] Rewrite some function exit paths to avoid "unreachable code" traps
From: Alex Riesen @ 2007-11-17 9:46 UTC (permalink / raw)
To: Guido Ostkamp; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711162346270.7281@bianca.dialin.t-online.de>
Noticed by Guido Ostkamp for Sun's Workshop cc.
Originally-by: Guido Ostkamp <git@ostkamp.fastmail.fm>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:
>
> What about the xdiff/xdiffi.c problem that should also be solved?
>
Here you go.
builtin-apply.c | 5 +++--
utf8.c | 2 +-
xdiff/xdiffi.c | 14 +++++++-------
xdiff/xutils.c | 5 ++---
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 8edcc08..6267396 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -668,13 +668,13 @@ static char *git_header_name(char *line, int llen)
default:
continue;
case '\n':
- return NULL;
+ goto eol;
case '\t': case ' ':
second = name+len;
for (;;) {
char c = *second++;
if (c == '\n')
- return NULL;
+ goto eol;
if (c == '/')
break;
}
@@ -683,6 +683,7 @@ static char *git_header_name(char *line, int llen)
}
}
}
+eol:
return NULL;
}
diff --git a/utf8.c b/utf8.c
index 8095a71..50c46af 100644
--- a/utf8.c
+++ b/utf8.c
@@ -262,7 +262,7 @@ int print_wrapped_text(const char *text, int indent, int indent2, int width)
print_spaces(indent);
fwrite(start, text - start, 1, stdout);
if (!c)
- return w;
+ break;
else if (c == '\t')
w |= 0x07;
space = text;
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 5cb7171..365d768 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -110,7 +110,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,
spl->i1 = i1;
spl->i2 = i2;
spl->min_lo = spl->min_hi = 1;
- return ec;
+ goto end;
}
}
@@ -145,7 +145,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,
spl->i1 = i1;
spl->i2 = i2;
spl->min_lo = spl->min_hi = 1;
- return ec;
+ goto end;
}
}
@@ -184,7 +184,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,
if (best > 0) {
spl->min_lo = 1;
spl->min_hi = 0;
- return ec;
+ goto end;
}
for (best = 0, d = bmax; d >= bmin; d -= 2) {
@@ -208,7 +208,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,
if (best > 0) {
spl->min_lo = 0;
spl->min_hi = 1;
- return ec;
+ goto end;
}
}
@@ -254,11 +254,11 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,
spl->min_lo = 0;
spl->min_hi = 1;
}
- return ec;
+ goto end;
}
}
-
- return -1;
+end:
+ return ec;
}
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 2ade97b..533ff76 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -230,10 +230,9 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
i2++;
}
return i1 >= s1 && i2 >= s2;
- } else
- return s1 == s2 && !memcmp(l1, l2, s1);
+ }
- return 0;
+ return s1 == s2 && !memcmp(l1, l2, s1);
}
static unsigned long xdl_hash_record_with_whitespace(char const **data,
--
1.5.3.5.750.g9f37
^ permalink raw reply related
* Re: Fwd: [postmaster@vger.kernel.org: Delivery reports about your email [FAILED(1)]]
From: Jeff King @ 2007-11-17 9:06 UTC (permalink / raw)
To: Matti Aarnio; +Cc: git
In-Reply-To: <20071116183530.GI6372@mea-ext.zmailer.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 6453 bytes --]
On Fri, Nov 16, 2007 at 08:35:30PM +0200, Matti Aarnio wrote:
> Here is a sample message that NEEDS proper charset mime tags.
Thank you for posting a complete example.
However, I'm not sure that git is to blame here. The problem text seems
to be "Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>". However, that text
seems to be included in a regular mail sent by gregkh. I see no evidence
of git-send-email being used (neither an X-Mailer, nor any message-id
which would have been generated by it).
It looks like the culprit is whatever he is using to generate the
stable-commit response. I'll note a few things below (sorry, the quoting
is long, but I don't want to omit any details):
> Following is copy of the message headers. Original message content may
> be in subsequent parts of this MESSAGE/DELIVERY-STATUS structure.
>
> Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
> id S1751399AbXKPSJk; Fri, 16 Nov 2007 13:09:40 -0500
> Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756649AbXKPSJk
> (ORCPT <rfc822;stable-commits-outgoing>);
> Fri, 16 Nov 2007 13:09:40 -0500
> Received: from ns2.suse.de ([195.135.220.15]:33829 "EHLO mx2.suse.de"
> rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
> id S1751399AbXKPSJj (ORCPT <rfc822;stable-commits@vger.kernel.org>);
> Fri, 16 Nov 2007 13:09:39 -0500
> Received: from Relay2.suse.de (mail2.suse.de [195.135.221.8])
> (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
> (No client certificate requested)
> by mx2.suse.de (Postfix) with ESMTP id 142E02BDB9;
> Fri, 16 Nov 2007 19:09:38 +0100 (CET)
> Subject: patch tcp-make-sure-write_queue_from-does-not-begin-with-null-ptr.patch queued to -stable tree
> To: ilpo.jarvinen@helsinki.fi, davem@davemloft.net
> Cc: <stable@kernel.org>, <stable-commits@vger.kernel.org>
> From: <gregkh@suse.de>
> Date: Fri, 16 Nov 2007 10:08:58 -0800
> Message-Id: <20071116180937.250A0144AB0C@imap.suse.de>
> Sender: stable-commits-owner@vger.kernel.org
> Precedence: bulk
> Reply-To: linux-kernel@vger.kernel.org
> X-Mailing-List: stable-commits@vger.kernel.org
This is presumably the complete header for the rejected message. I agree
this ought to have a content-type header, but it clearly wasn't sent by
git-send-email.
Presumably there is some post-receive hook that is doing this, but it's
hard to say more without seeing the hook.
> Reporting-MTA: dns; vger.kernel.org
> Arrival-Date: Fri, 16 Nov 2007 13:09:40 -0500
> Local-Spool-ID: S1751399AbXKPSJk
>
> Original-Recipient: rfc822;jfunk@funktronics.ca
> Final-Recipient: RFC822;jfunk@funktronics.ca
> Action: failed
> Status: 5.1.1 (bad destination mailbox)
> Remote-MTA: dns; elseed.funktronics.ca (65.61.206.36|25|209.132.176.167|48741)
> Last-Attempt-Date: Fri, 16 Nov 2007 13:10:02 -0500
> Diagnostic-Code: smtp; 550 (Error: improper use of 8-bit data in message body)
> Date: Fri, 16 Nov 2007 10:08:58 -0800
> From: gregkh@suse.de
> To: ilpo.jarvinen@helsinki.fi, davem@davemloft.net
> Cc: stable@kernel.org, stable-commits@vger.kernel.org
> Reply-To: linux-kernel@vger.kernel.org
> Subject: patch
> tcp-make-sure-write_queue_from-does-not-begin-with-null-ptr.patch
> queued to -stable tree
>
>
> This is a note to let you know that we have just queued up the patch titled
>
> Subject: TCP: Make sure write_queue_from does not begin with NULL ptr (CVE-2007-5501)
>
> to the 2.6.23-stable tree. Its filename is
>
> tcp-make-sure-write_queue_from-does-not-begin-with-null-ptr.patch
>
> A git repo of this tree can be found at
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
>
> >From 96a2d41a3e495734b63bff4e5dd0112741b93b38 Mon Sep 17 00:00:00 2001
> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> Date: Wed, 14 Nov 2007 15:47:18 -0800
> Subject: TCP: Make sure write_queue_from does not begin with NULL ptr (CVE-2007-5501)
>
> From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
And this is clearly generated by git-format-patch. The signed-off-by
requires a charset specifier. This was fixed by Junio in 4593fb84 about
2 weeks ago, but hasn't made it into a released version yet.
The extra 'From' line in the body of the email is not something
generated by git-format-patch. Usually such lines are placed by
git-send-email, and would require encoding; we just queued a fix for
that yesterday. However, I don't see any other evidence of
git-send-email being used here, so it looks more like whatever script
generated the outer mail just called git-format-patch.
> patch 96a2d41a3e495734b63bff4e5dd0112741b93b38 in mainline.
>
> NULL ptr can be returned from tcp_write_queue_head to cached_skb
> and then assigned to skb if packets_out was zero. Without this,
> system is vulnerable to a carefully crafted ACKs which obviously
> is remotely triggerable.
>
> Besides, there's very little that needs to be done in sacktag
> if there weren't any packets outstanding, just skipping the rest
> doesn't hurt.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> ---
> net/ipv4/tcp_input.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -1012,6 +1012,9 @@ tcp_sacktag_write_queue(struct sock *sk,
> if (before(TCP_SKB_CB(ack_skb)->ack_seq, prior_snd_una - tp->max_window))
> return 0;
>
> + if (!tp->packets_out)
> + goto out;
> +
> /* SACK fastpath:
> * if the only SACK change is the increase of the end_seq of
> * the first block then only apply that SACK block
> @@ -1280,6 +1283,8 @@ tcp_sacktag_write_queue(struct sock *sk,
> (!tp->frto_highmark || after(tp->snd_una, tp->frto_highmark)))
> tcp_update_reordering(sk, ((tp->fackets_out + 1) - reord), 0);
>
> +out:
> +
> #if FASTRETRANS_DEBUG > 0
> BUG_TRAP((int)tp->sacked_out >= 0);
> BUG_TRAP((int)tp->lost_out >= 0);
>
>
> Patches currently in stable-queue which might be from ilpo.jarvinen@helsinki.fi are
>
> queue-2.6.23/tcp-make-sure-write_queue_from-does-not-begin-with-null-ptr.patch
> -
> To unsubscribe from this list: send the line "unsubscribe stable-commits" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Fix and improve t7004
From: Junio C Hamano @ 2007-11-17 8:55 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <1195250528-9727-1-git-send-email-mh@glandium.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] builtin-commit: fix "git add x y && git commit y" committing x, too
From: Junio C Hamano @ 2007-11-17 8:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Kristian Høgsberg, git
In-Reply-To: <Pine.LNX.4.64.0711160036450.30886@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> It's not only about discarding the cache. It's also about avoiding do
> regenerate the index completely; this would waste time, especially for big
> trees.
I was looking at this code earlier tonight but I am too tired so
here are a few comments before I stop.
> But the code you are referencing is only updating the index. The code I
> added is to build the temporary index in a correct manner.
Yes, except that it is only in the partial commit codepath and
there is not much point optimizing it, as there are more to it.
If a path that was not in the HEAD was added to the index
earlier, and the path was named on the command line, the
add_files_to_index() function you are borrowing from the
implementation of "add -u" would not notice it. Look at the
script version of git-commit.sh and look for places near
"ls-files --error-unmatch --with-tree=HEAD".
I _think_ we need to do the equivalent of this, keep the
affected paths in a path-list and use add_file_to_cache()
instead. We need to feed the same set of paths to update the
index twice (once for the fake one for partial commit, and
another for the real index to be used after the commit is made),
and (1) using add_files_to_index() is more expensive than
walking a path-list, and (2) add_files_to_index() is a wrong
thing to use anyway (by definition you cannot notice addition
when you are comparing the index and the work tree, so I think
your patch to update_callback() is a no-op).
I noticed that the implementation left next-index crufts almost
every time it was run, and started to clean it up. Here is
still a WIP and it does not optimize the read_tree(HEAD) part,
but you should be able to replace that part with your one-way
merge easily. As I haven't done that ls-files --error-unmatch
equivalent, this does not pass tests that involve partial
commits with added or removed paths.
---
builtin-commit.c | 174 +++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 139 insertions(+), 35 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 3e7d281..187d613 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -7,6 +7,7 @@
#include "cache.h"
#include "cache-tree.h"
+#include "dir.h"
#include "builtin.h"
#include "diff.h"
#include "diffcore.h"
@@ -28,7 +29,13 @@ static const char * const builtin_commit_usage[] = {
static unsigned char head_sha1[20], merge_head_sha1[20];
static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
-static struct lock_file lock_file;
+static struct lock_file index_lock; /* real index */
+static struct lock_file false_lock; /* used only for partial commits */
+static enum {
+ COMMIT_AS_IS = 1,
+ COMMIT_NORMAL,
+ COMMIT_PARTIAL,
+} commit_style;
static char *logfile, *force_author, *template_file;
static char *edit_message, *use_message;
@@ -78,41 +85,122 @@ static struct option builtin_commit_options[] = {
OPT_END()
};
+static void rollback_index_files(void)
+{
+ switch (commit_style) {
+ case COMMIT_AS_IS:
+ break; /* nothing to do */
+ case COMMIT_NORMAL:
+ rollback_lock_file(&index_lock);
+ break;
+ case COMMIT_PARTIAL:
+ rollback_lock_file(&index_lock);
+ rollback_lock_file(&false_lock);
+ break;
+ }
+}
+
+static void commit_index_files(void)
+{
+ switch (commit_style) {
+ case COMMIT_AS_IS:
+ break; /* nothing to do */
+ case COMMIT_NORMAL:
+ commit_lock_file(&index_lock);
+ break;
+ case COMMIT_PARTIAL:
+ commit_lock_file(&index_lock);
+ rollback_lock_file(&false_lock);
+ break;
+ }
+}
+
static char *prepare_index(const char **files, const char *prefix)
{
int fd;
struct tree *tree;
- struct lock_file *next_index_lock;
if (interactive) {
interactive_add();
return get_index_file();
}
- fd = hold_locked_index(&lock_file, 1);
if (read_cache() < 0)
die("index file corrupt");
+ /*
+ * Non partial, non as-is commit.
+ *
+ * (1) get the real index;
+ * (2) update the_index as necessary;
+ * (3) write the_index out to the real index (still locked);
+ * (4) return the name of the locked index file.
+ *
+ * The caller should run hooks on the locked real index, and
+ * (A) if all goes well, commit the real index;
+ * (B) on failure, rollback the real index.
+ */
if (all || also) {
+ fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(verbose, also ? prefix : NULL, files);
refresh_cache(REFRESH_QUIET);
if (write_cache(fd, active_cache, active_nr) || close(fd))
die("unable to write new_index file");
- return lock_file.filename;
+ commit_style = COMMIT_NORMAL;
+ return index_lock.filename;
}
+ /*
+ * As-is commit.
+ *
+ * (1) return the name of the real index file.
+ *
+ * The caller should run hooks on the real index, and run
+ * hooks on the real index, and create commit from the_index.
+ * No lockfile is needed.
+ */
if (*files == NULL) {
- /* Commit index as-is. */
- rollback_lock_file(&lock_file);
+ fd = hold_locked_index(&index_lock, 1);
+ refresh_cache(REFRESH_QUIET);
+ if (write_cache(fd, active_cache, active_nr) ||
+ close(fd) || commit_locked_index(&index_lock))
+ die("unable to write new_index file");
+ commit_style = COMMIT_AS_IS;
return get_index_file();
}
- /* update the user index file */
+ /*
+ * A partial commit.
+ *
+ * (0) find the set of affected paths [NEEDSWORK: NOT DONE YET]
+ * (1) get lock on the real index file;
+ * (2) update the_index with the given paths;
+ * (3) write the_index out to the real index (still locked);
+ * (4) get lock on the false index file;
+ * (5) reset the_index from HEAD, but keep the addition;
+ * (6) update the_index the same way as (2);
+ * (7) write the_index out to the false index file;
+ * (8) return the name of the false index file (still locked);
+ *
+ * The caller should run hooks on the locked false index, and
+ * (A) if all goes well, commit the real index;
+ * (B) on failure, rollback the real index;
+ * In either case, rollback the false index.
+ */
+ commit_style = COMMIT_PARTIAL;
+
+ if (file_exists(git_path("MERGE_HEAD")))
+ die("cannot do a partial commit during a merge.");
+
+ fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(verbose, prefix, files);
refresh_cache(REFRESH_QUIET);
if (write_cache(fd, active_cache, active_nr) || close(fd))
die("unable to write new_index file");
+ fd = hold_lock_file_for_update(&false_lock,
+ git_path("next-index-%d", getpid()), 1);
+ discard_cache();
if (!initial_commit) {
tree = parse_tree_indirect(head_sha1);
if (!tree)
@@ -120,17 +208,12 @@ static char *prepare_index(const char **files, const char *prefix)
if (read_tree(tree, 0, NULL))
die("failed to read HEAD tree object");
}
-
- /* Use a lock file to garbage collect the temporary index file. */
- next_index_lock = xmalloc(sizeof(*next_index_lock));
- fd = hold_lock_file_for_update(next_index_lock,
- git_path("next-index-%d", getpid()), 1);
add_files_to_cache(verbose, prefix, files);
refresh_cache(REFRESH_QUIET);
- if (write_cache(fd, active_cache, active_nr) || close(fd))
- die("unable to write new_index file");
- return next_index_lock->filename;
+ if (write_cache(fd, active_cache, active_nr) || close(fd))
+ die("unable to write temporary index file");
+ return false_lock.filename;
}
static int run_status(FILE *fp, const char *index_file, const char *prefix)
@@ -437,7 +520,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
commitable = run_status(stdout, index_file, prefix);
- rollback_lock_file(&lock_file);
+ rollback_index_files();
return commitable ? 0 : 1;
}
@@ -527,23 +610,36 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
index_file = prepare_index(argv, prefix);
- if (!no_verify && run_hook(index_file, "pre-commit", NULL))
- exit(1);
+ if (!no_verify && run_hook(index_file, "pre-commit", NULL)) {
+ rollback_index_files();
+ return 1;
+ }
if (!prepare_log_message(index_file, prefix) && !in_merge) {
run_status(stdout, index_file, prefix);
+ rollback_index_files();
unlink(commit_editmsg);
return 1;
}
- strbuf_init(&sb, 0);
-
- /* Start building up the commit header */
+ /*
+ * Re-read the index as pre-commit hook could have updated it,
+ * and write it out as a tree.
+ */
+ discard_cache();
read_cache_from(index_file);
- active_cache_tree = cache_tree();
+ if (!active_cache_tree)
+ active_cache_tree = cache_tree();
if (cache_tree_update(active_cache_tree,
- active_cache, active_nr, 0, 0) < 0)
+ active_cache, active_nr, 0, 0) < 0) {
+ rollback_index_files();
die("Error building trees");
+ }
+
+ /*
+ * The commit object
+ */
+ strbuf_init(&sb, 0);
strbuf_addf(&sb, "tree %s\n",
sha1_to_hex(active_cache_tree->sha1));
@@ -592,20 +688,27 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
header_len = sb.len;
if (!no_edit)
launch_editor(git_path(commit_editmsg), &sb);
- else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0)
+ else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
+ rollback_index_files();
die("could not read commit message\n");
- if (run_hook(index_file, "commit-msg", commit_editmsg))
+ }
+ if (run_hook(index_file, "commit-msg", commit_editmsg)) {
+ rollback_index_files();
exit(1);
+ }
stripspace(&sb, 1);
- if (sb.len < header_len ||
- message_is_empty(&sb, header_len))
+ if (sb.len < header_len || message_is_empty(&sb, header_len)) {
+ rollback_index_files();
die("* no commit message? aborting commit.");
+ }
strbuf_addch(&sb, '\0');
if (is_encoding_utf8(git_commit_encoding) && !is_utf8(sb.buf))
fprintf(stderr, commit_utf8_warn);
- if (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1))
+ if (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1)) {
+ rollback_index_files();
die("failed to write commit object");
+ }
ref_lock = lock_any_ref_for_update("HEAD",
initial_commit ? NULL : head_sha1,
@@ -620,21 +723,22 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
- if (!ref_lock)
+ if (!ref_lock) {
+ rollback_index_files();
die("cannot lock HEAD ref");
- if (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0)
+ }
+ if (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0) {
+ rollback_index_files();
die("cannot update HEAD ref");
+ }
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
- if (lock_file.filename[0] && commit_locked_index(&lock_file))
- die("failed to write new index");
+ commit_index_files();
rerere();
-
- run_hook(index_file, "post-commit", NULL);
-
+ run_hook(get_index_file(), "post-commit", NULL);
if (!quiet)
print_summary(prefix, commit_sha1);
^ permalink raw reply related
* [PATCH] Fix and improve t7004
From: Mike Hommey @ 2007-11-16 22:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7vbq9tyii3.fsf@gitster.siamese.dyndns.org>
Brown paper bag fix to avoid using non portable sed syntax. The
test by itself didn't catch what it was supposed to, anyways.
The new test first checks whether the user exited the editor
without editing the file, then whether what the user was
presented in the editor was any useful to her, which we define
as the following:
* It begins with a single blank line, where the invoked editor
would typically place the editing curser at so that the user
can immediately start typing;
* It has some instruction but that comes after that initial
blank line, all lines prefixed with "#".
* And it has nothing else, as the expected behaviour is "Hey
you did not leave any message".
Signed-off-by: Mike Hommey <mh@glandium.org>
---
> Perhaps...
>
> # check the first line --- should be empty
> first=$(sed -e 1q <actual) &&
> test -z "$first" &&
> # remove commented lines from the remainder -- should be empty
> rest=$(sed -e 1d -e '/^#/d' <actual) &&
> test -z "$rest"
I like that better. So here is a new version using this. In the end, the
patch and most of the comment are yours, so maybe you should claim it is
your patch.
t/t7004-tag.sh | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..162bdf7 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1004,10 +1004,20 @@ test_expect_failure \
'verify signed tag fails when public key is not present' \
'git-tag -v signed-tag'
+test_expect_failure \
+ 'git-tag -a fails if tag annotation is empty' '
+ GIT_EDITOR=cat git tag -a initial-comment > /dev/null 2>&1
+'
+
test_expect_success \
'message in editor has initial comment' '
GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
- test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+ # check the first line --- should be empty
+ first=$(sed -e 1q <actual) &&
+ test -z "$first" &&
+ # remove commented lines from the remainder -- should be empty
+ rest=$(sed -e 1d -e '/^#/d' <actual) &&
+ test -z "$rest"
'
get_tag_header reuse $commit commit $time >expect
--
1.5.3.5
^ permalink raw reply related
* Re: [PATCH] Git.pm: Don't return 'undef' in vector context.
From: Dan Zwell @ 2007-11-17 3:20 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Dan Zwell, Petr Baudis, Git Mailing List
In-Reply-To: <fhjl70$db4$1@ger.gmane.org>
Jakub Narebski wrote:
>
> By the way, what do you think about changing Git.pm config handling
> to the 'eager' one used currently by gitweb, namely reading all the
> config to hash, and later getting config values from hash instead of
> calling git-config? Or at least make it an option?
>
That seems appropriate, though it may be a slight trade-off between
complexity and efficiency. I don't think it's strictly necessary, at
least for git-add--interactive. My experience is that the nine calls to
config() (that I am adding) do not slow down the program from a user
perspective (though I haven't tested on a slower computer).
The big reason to do it would be if you wanted to convert gitweb to use
the standard config() call from Git.pm. Because right now, config()
isn't efficient, but it probably doesn't need to be.
Dan
^ permalink raw reply
* Re: [BUG] encoding problem with format-patch + send-email
From: Junio C Hamano @ 2007-11-17 0:48 UTC (permalink / raw)
To: Jeff King
Cc: Uwe Kleine-König, git, Brian Swetland,
Russell King - ARM Linux, Nicolas Pitre
In-Reply-To: <20071116104907.GA13087@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> git-send-email: add charset header if we add encoded 'From'
Thanks.
> If we already had a content-type header in the mail, then we
> may need to re-encode. The logic is there to detect
> this case, but it doesn't actually do the re-encoding.
Although the charset on rfc2047 encoded header fields can be
independent of the charset in the body, I think you need to do
crazy things to do so.
Will queue.
^ permalink raw reply
* Re: [PATCH] Git.pm: Don't return 'undef' in vector context.
From: Junio C Hamano @ 2007-11-17 0:39 UTC (permalink / raw)
To: Dan Zwell; +Cc: Git Mailing List
In-Reply-To: <473D4E18.3060006@zwell.net>
Dan Zwell <dzwell@gmail.com> writes:
> That's reasonable. I'll resend this as part of the
> git-add--interactive color patches. This can be cherry-picked out, but
> some of the other stuff I want to do depends on it (a helper function
> that I wrote, config_with_default($repo, $key, $default)).
Sure; as long as the "Don't return 'undef'" remains a separate
patch early in the series, that is easy to manage.
Thanks.
^ permalink raw reply
* [RFH] Solaris portability
From: Junio C Hamano @ 2007-11-17 0:33 UTC (permalink / raw)
To: Guido Ostkamp; +Cc: git, Shawn O Pearce, Jason Riedy, Dennis Stosberg
In-Reply-To: <Pine.LNX.4.64.0711161954510.7139@bianca.dialin.t-online.de>
Guido Ostkamp <git@ostkamp.fastmail.fm> writes:
> On Thu, 15 Nov 2007, Junio C Hamano wrote:
>> Are there problems with the implementation in compat/ directory, we
>> ship specifically to help platforms without mkdtemp()?
>
> I checked again and the answer is 'yes'. The reason is trivial - for
> Solaris 10 the workaround is not activated and my version of Solaris
> 10 (Sparc) has no mkdtemp() in libc.so.
Thanks.
This makes me wonder if treating it just like strcasestr() might
be simpler. Could folks with access to Solaris boxes of
different vintages please see if the attached patch makes sense?
Can we also unify UNSETENV, SETENV, C99_FORMAT and STRTOUMAX, by
the way?
diff --git a/Makefile b/Makefile
index e830bc7..cabde81 100644
--- a/Makefile
+++ b/Makefile
@@ -412,26 +412,25 @@ endif
ifeq ($(uname_S),SunOS)
NEEDS_SOCKET = YesPlease
NEEDS_NSL = YesPlease
SHELL_PATH = /bin/bash
NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_HSTRERROR = YesPlease
+ NO_MKDTEMP = YesPlease
ifeq ($(uname_R),5.8)
NEEDS_LIBICONV = YesPlease
NO_UNSETENV = YesPlease
NO_SETENV = YesPlease
- NO_MKDTEMP = YesPlease
NO_C99_FORMAT = YesPlease
NO_STRTOUMAX = YesPlease
endif
ifeq ($(uname_R),5.9)
NO_UNSETENV = YesPlease
NO_SETENV = YesPlease
- NO_MKDTEMP = YesPlease
NO_C99_FORMAT = YesPlease
NO_STRTOUMAX = YesPlease
endif
INSTALL = ginstall
TAR = gtar
BASIC_CFLAGS += -D__EXTENSIONS__
endif
^ permalink raw reply related
* Re: [PATCH] Fix t9101 test failure caused by Subversion "auto-props"
From: Junio C Hamano @ 2007-11-17 0:19 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Björn Steinbrink, Benoit Sigoure, Git Mailing List,
Väinö Järvelä
In-Reply-To: <2F7DFDC9-D4E2-42D0-9E48-E51E7905FF42@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> If a user has an "auto-prop" in his/her ~/.subversion/config file for
> automatically setting the svn:keyword Id property on all ".c" files
> (a reasonably common configuration in the Subversion world) then one
> of the "svn propset" operations in the very first test would become a
> no-op, which in turn would make the next commit a no-op.
Thanks for diagnosing and fixing.
I presume this fix also applies to both 'maint' and 'master',
right?
^ permalink raw reply
* Re: Solaris 'make' & Git Makefile
From: Junio C Hamano @ 2007-11-16 23:49 UTC (permalink / raw)
To: Guido Ostkamp; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711162353170.7281@bianca.dialin.t-online.de>
Guido Ostkamp <git@ostkamp.fastmail.fm> writes:
> Question: Do you require the GNU make utility for Git?
>
> If yes, then I think this should be mentioned in README or INSTALL.
I think that would be a sensible thing to do. Please make it
happen.
^ 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