* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Björn Steinbrink @ 2009-12-10 12:47 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jari Aalto, Junio C Hamano
In-Reply-To: <3b0a7bfa75126e4c13ec15a4357645b2bfd14b5b.1260447713.git.git@drmicha.warpmail.net>
On 2009.12.10 13:23:43 +0100, Michael J Gruber wrote:
> Print the sha1 of the deleted tag (in addition to the tag name) so that
> one can easily recreate a mistakenly deleted tag:
>
> git tag -d tagname
> Deleted tag 'tagname' DEADBEEF
> git tag 'tagname' DEADBEEF # for lightweight tags
> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
Using "git tag 'tagname' DEADBEEF" should actually work in both cases.
As that does nothing but creating the ref in the refs/tags/ namespace.
Bjoern
^ permalink raw reply
* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Jeff King @ 2009-12-10 12:49 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jari Aalto, Junio C Hamano
In-Reply-To: <3b0a7bfa75126e4c13ec15a4357645b2bfd14b5b.1260447713.git.git@drmicha.warpmail.net>
On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:
> Print the sha1 of the deleted tag (in addition to the tag name) so that
> one can easily recreate a mistakenly deleted tag:
>
> git tag -d tagname
> Deleted tag 'tagname' DEADBEEF
> git tag 'tagname' DEADBEEF # for lightweight tags
> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
I think this is a good idea, and we already do the same for branch
deletion.
I'm not sure your example is right. If "tag -d" always prints out the
sha1 in the tag ref, can't you just use "git tag 'tagname' DEADBEEF" to
recreate both lightweight and annotated tags? That is, making a
lightweight tag of an annotated tag's sha1 should just recreate the
original annotated tag.
That being said, I am not a fan of the cut-and-paste format. This is not
something that happens so frequently that I think we need to go out of
our way to save some typing. And for a user seeing this message for the
first time:
1. It is not immediately obvious to a user seeing this message
for this first time exactly what the trailing sha1 means. We
already had this discussion with "git branch -d" and decided
that "(was DEADBEEF)" was more readable.
2. Even if they know what it means, it is not immediately obvious that
the error line is meant to be cut-and-pasted. If you are going to
give something to cut-and-paste, I think you are better off making
it obvious, like:
Deleted tag 'foo'; you can recreate it with
git tag 'foo' DEADBEEF
Of course that is painfully long for a message that is meant to be
a "just in case" notification of a successful command (I can see it
more for an actual error, where git is telling you "I couldn't do
what you wanted, but you might try running this command first").
-Peff
^ permalink raw reply
* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Jari Aalto @ 2009-12-10 13:16 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, git, Junio C Hamano
In-Reply-To: <20091210124929.GA444@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:
>
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>>
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
> That being said, I am not a fan of the cut-and-paste format. This is not
> something that happens so frequently
It dpends on user. For me it it does.
> 1. It is not immediately obvious to a user seeing this message
> for this first time exactly what the trailing sha1 means.
>
> 2. Even if they know what it means, it is not immediately obvious that
> the error line is meant to be cut-and-pasted.
"not meant" specifically, but it's very convenient to have it in format
that happens to be "cut-n-paste ready". The SHA itself is easily
understood in the context.
Thanks for all that have so quickly implemented this,
Jari
^ permalink raw reply
* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Michael J Gruber @ 2009-12-10 13:21 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: git, Jari Aalto, Junio C Hamano
In-Reply-To: <20091210124701.GA12521@atjola.homenet>
Björn Steinbrink venit, vidit, dixit 10.12.2009 13:47:
> On 2009.12.10 13:23:43 +0100, Michael J Gruber wrote:
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>>
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
>
> Using "git tag 'tagname' DEADBEEF" should actually work in both cases.
> As that does nothing but creating the ref in the refs/tags/ namespace.
>
> Bjoern
Cool, even better! So, an annotated tag is practically a lightweight tag
pointing to a tag object. Once you think of it it's natural!
Michael
^ permalink raw reply
* [RFC/PATCH v2] tag: display original sha1 of deleted/overwritten tag
From: Zoltán Füzesi @ 2009-12-10 13:26 UTC (permalink / raw)
To: git; +Cc: jari.aalto, Zoltán Füzesi
In-Reply-To: <1260448496-527-1-git-send-email-zfuzesi@eaglet.hu>
It makes possible to undo accidental tag deletion and overwriting.
Signed-off-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
---
This patch was created in response to this feature request:
http://article.gmane.org/gmane.comp.version-control.git/135016
I named it "RFC" because of the overwriting part.
v2: works when tag object is created.
builtin-tag.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index c479018..1265b84 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
{
if (delete_ref(ref, sha1, 0))
return 1;
- printf("Deleted tag '%s'\n", name);
+ printf("Deleted tag '%s' %s\n", name, sha1_to_hex(sha1));
return 0;
}
@@ -363,7 +363,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
- unsigned char object[20], prev[20];
+ unsigned char object[20], prev[20], tag_object[20];
char ref[PATH_MAX];
const char *object_ref, *tag;
struct ref_lock *lock;
@@ -472,13 +472,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, msg.given || msgfile,
- sign, prev, object);
+ sign, prev, tag_object);
lock = lock_any_ref_for_update(ref, prev, 0);
if (!lock)
die("%s: cannot lock the ref", ref);
if (write_ref_sha1(lock, object, NULL) < 0)
die("%s: cannot update the ref", ref);
+ else if (force && hashcmp(object, prev))
+ printf("Overwritten tag '%s' (%s)\n", ref, sha1_to_hex(prev));
strbuf_release(&buf);
return 0;
--
1.6.6.rc2
^ permalink raw reply related
* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Michael J Gruber @ 2009-12-10 13:27 UTC (permalink / raw)
To: Jeff King; +Cc: git, Jari Aalto, Junio C Hamano
In-Reply-To: <20091210124929.GA444@coredump.intra.peff.net>
Jeff King venit, vidit, dixit 10.12.2009 13:49:
> On Thu, Dec 10, 2009 at 01:23:43PM +0100, Michael J Gruber wrote:
>
>> Print the sha1 of the deleted tag (in addition to the tag name) so that
>> one can easily recreate a mistakenly deleted tag:
>>
>> git tag -d tagname
>> Deleted tag 'tagname' DEADBEEF
>> git tag 'tagname' DEADBEEF # for lightweight tags
>> git update-ref refs/tags/'tagname' DEADBEEF # for annotated tags
>
> I think this is a good idea, and we already do the same for branch
> deletion.
>
> I'm not sure your example is right. If "tag -d" always prints out the
> sha1 in the tag ref, can't you just use "git tag 'tagname' DEADBEEF" to
> recreate both lightweight and annotated tags? That is, making a
> lightweight tag of an annotated tag's sha1 should just recreate the
> original annotated tag.
While my example is right it is unnecessarily complex. I learned that
through Björns and your remark.
> That being said, I am not a fan of the cut-and-paste format. This is not
> something that happens so frequently that I think we need to go out of
> our way to save some typing. And for a user seeing this message for the
> first time:
>
> 1. It is not immediately obvious to a user seeing this message
> for this first time exactly what the trailing sha1 means. We
> already had this discussion with "git branch -d" and decided
> that "(was DEADBEEF)" was more readable.
So, should we simply go with that then?
Meanwhile, RFCs/PATCHes crossed paths. I take it that Zoltan suggests
giving the same output for force-overwritten existing tags. I beat him
by 11 minutes, though ;)
Michael
^ permalink raw reply
* Re: [PATCH] tag -d: print sha1 of deleted tag
From: Jeff King @ 2009-12-10 13:36 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jari Aalto, Junio C Hamano
In-Reply-To: <4B20F733.6010401@drmicha.warpmail.net>
On Thu, Dec 10, 2009 at 02:27:15PM +0100, Michael J Gruber wrote:
> > 1. It is not immediately obvious to a user seeing this message
> > for this first time exactly what the trailing sha1 means. We
> > already had this discussion with "git branch -d" and decided
> > that "(was DEADBEEF)" was more readable.
>
> So, should we simply go with that then?
I think so. Jari obviously disagrees, but I don't have much more to say
in favor of it except that I find the other ugly and unintuitive. So it
is up to you what you want to submit and Junio what he wants to apply.
:)
> Meanwhile, RFCs/PATCHes crossed paths. I take it that Zoltan suggests
> giving the same output for force-overwritten existing tags. I beat him
> by 11 minutes, though ;)
Yes, I think if you are going to protect "tag -d", you might as well
protect overwriting, as well. Which made me think at first that we need
something similar for "branch -f", but I don't think we do; the last
branch value will be left in the reflog (but with tags, there is no
reflog).
-Peff
^ permalink raw reply
* [RFC/PATCH v3] tag: display original sha1 of deleted/overwritten tag
From: Zoltán Füzesi @ 2009-12-10 13:58 UTC (permalink / raw)
To: git; +Cc: jari.aalto, Zoltán Füzesi
In-Reply-To: <1260451579-4400-1-git-send-email-zfuzesi@eaglet.hu>
It makes possible to undo accidental tag deletion and overwriting.
Signed-off-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
---
This patch was created in response to this feature request:
http://article.gmane.org/gmane.comp.version-control.git/135016
I named it "RFC" because of the overwriting part.
v3: works when tag object is created (I should slow down...)
builtin-tag.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index c479018..1404a0e 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
{
if (delete_ref(ref, sha1, 0))
return 1;
- printf("Deleted tag '%s'\n", name);
+ printf("Deleted tag '%s' %s\n", name, sha1_to_hex(sha1));
return 0;
}
@@ -363,7 +363,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
- unsigned char object[20], prev[20];
+ unsigned char object[20], prev[20], tag_object[20];
char ref[PATH_MAX];
const char *object_ref, *tag;
struct ref_lock *lock;
@@ -472,13 +472,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, msg.given || msgfile,
- sign, prev, object);
+ sign, prev, tag_object);
lock = lock_any_ref_for_update(ref, prev, 0);
if (!lock)
die("%s: cannot lock the ref", ref);
- if (write_ref_sha1(lock, object, NULL) < 0)
+ if (write_ref_sha1(lock, annotate ? tag_object : object, NULL) < 0)
die("%s: cannot update the ref", ref);
+ else if (force && hashcmp(object, prev))
+ printf("Overwritten tag '%s' (%s)\n", ref, sha1_to_hex(prev));
strbuf_release(&buf);
return 0;
--
1.6.6.rc2
^ permalink raw reply related
* [PATCH v2] tag -d: print sha1 of deleted tag
From: Michael J Gruber @ 2009-12-10 14:01 UTC (permalink / raw)
To: git
Cc: Jari Aalto, Björn Steinbrink, Jeff King,
Zoltán Füzesi, Junio C Hamano
In-Reply-To: <20091210133645.GA2149@coredump.intra.peff.net>
Print the sha1 of the deleted tag (in addition to the tag name) so that
one can easily recreate a mistakenly deleted tag:
git tag -d tagname
Deleted tag 'tagname' (was DEADBEEF)
git tag 'tagname' DEADBEEF
We output the previous ref also in the case of forcefully overwriting
tags.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Suggested-by: Jari Aalto <jari.aalto@cante.net>
Helped-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
---
v2 changes the wording to match with branch -d and uses the same
for forcefully overwriting tags.
Zoltán, I don't think we should make this into a race. Posting in
the relevant thread (and actually following it) would help this.
Also, I think we should really compare the sha1 the tag points to,
i.e. like below and like in your v1 (not v2). Different tag object
is different tag (message may differ, e.g.).
builtin-tag.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index c479018..4ef1c4f 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
{
if (delete_ref(ref, sha1, 0))
return 1;
- printf("Deleted tag '%s'\n", name);
+ printf("Deleted tag '%s' (was %s)\n", name, find_unique_abbrev(sha1, DEFAULT_ABBREV));
return 0;
}
@@ -479,6 +479,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die("%s: cannot lock the ref", ref);
if (write_ref_sha1(lock, object, NULL) < 0)
die("%s: cannot update the ref", ref);
+ if (force && hashcmp(prev, object))
+ printf("Updated tag '%s' (was %s)\n", tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
strbuf_release(&buf);
return 0;
--
1.6.6.rc1.292.gd8fe
^ permalink raw reply related
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-10 14:03 UTC (permalink / raw)
To: Johan Herland; +Cc: git, gitster
In-Reply-To: <200912101039.44526.johan@herland.net>
Johan Herland <johan@herland.net> wrote:
> Do you have more comments/suggestions on this patch? Or is it ok to include
> in fast-import as-is?
Oops, sorry.
No, no additional comments. I am happy with this patch.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
--
Shawn.
^ permalink raw reply
* Re: [RFC/PATCH v3] tag: display original sha1 of deleted/overwritten tag
From: Zoltán Füzesi @ 2009-12-10 14:09 UTC (permalink / raw)
To: git
In-Reply-To: <1260453537-6156-1-git-send-email-zfuzesi@eaglet.hu>
Forget v2 and v3, v1 is the good one. Sorry.
2009/12/10 Zoltán Füzesi <zfuzesi@eaglet.hu>:
> It makes possible to undo accidental tag deletion and overwriting.
>
> Signed-off-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
> ---
> This patch was created in response to this feature request:
> http://article.gmane.org/gmane.comp.version-control.git/135016
> I named it "RFC" because of the overwriting part.
>
> v3: works when tag object is created (I should slow down...)
>
> builtin-tag.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/builtin-tag.c b/builtin-tag.c
> index c479018..1404a0e 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -140,7 +140,7 @@ static int delete_tag(const char *name, const char *ref,
> {
> if (delete_ref(ref, sha1, 0))
> return 1;
> - printf("Deleted tag '%s'\n", name);
> + printf("Deleted tag '%s' %s\n", name, sha1_to_hex(sha1));
> return 0;
> }
>
> @@ -363,7 +363,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
> int cmd_tag(int argc, const char **argv, const char *prefix)
> {
> struct strbuf buf = STRBUF_INIT;
> - unsigned char object[20], prev[20];
> + unsigned char object[20], prev[20], tag_object[20];
> char ref[PATH_MAX];
> const char *object_ref, *tag;
> struct ref_lock *lock;
> @@ -472,13 +472,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>
> if (annotate)
> create_tag(object, tag, &buf, msg.given || msgfile,
> - sign, prev, object);
> + sign, prev, tag_object);
>
> lock = lock_any_ref_for_update(ref, prev, 0);
> if (!lock)
> die("%s: cannot lock the ref", ref);
> - if (write_ref_sha1(lock, object, NULL) < 0)
> + if (write_ref_sha1(lock, annotate ? tag_object : object, NULL) < 0)
> die("%s: cannot update the ref", ref);
> + else if (force && hashcmp(object, prev))
> + printf("Overwritten tag '%s' (%s)\n", ref, sha1_to_hex(prev));
>
> strbuf_release(&buf);
> return 0;
> --
> 1.6.6.rc2
>
>
^ permalink raw reply
* Re: [PATCH v2] tag -d: print sha1 of deleted tag
From: Zoltán Füzesi @ 2009-12-10 14:16 UTC (permalink / raw)
To: Michael J Gruber
Cc: git, Jari Aalto, Björn Steinbrink, Jeff King, Junio C Hamano
In-Reply-To: <39e5cb38658919ed989f954a0917dfaa292f6350.1260453234.git.git@drmicha.warpmail.net>
2009/12/10 Michael J Gruber <git@drmicha.warpmail.net>:
> Zoltán, I don't think we should make this into a race. Posting in
> the relevant thread (and actually following it) would help this.
You are right. I've deleted the feature request mail, and then few
minutes laster decided to create the patch. So I've lost the message
ID (though I could retrieve it from the web...).
After posting the patch, noticed that you also posted one.
> Also, I think we should really compare the sha1 the tag points to,
> i.e. like below and like in your v1 (not v2). Different tag object
> is different tag (message may differ, e.g.).
I agree.
^ permalink raw reply
* Re: subtree merge tries to merge into wrong directory
From: Avery Pennarun @ 2009-12-10 14:38 UTC (permalink / raw)
To: Nils Adermann; +Cc: git
In-Reply-To: <4B206DCF.90202@naderman.de>
On Wed, Dec 9, 2009 at 10:41 PM, Nils Adermann <naderman@naderman.de> wrote:
> Following
> http://www.kernel.org/pub/software/scm/git/docs/howto/using-merge-subtree.html
> I have subtree merged a 3rd party library (ezc-reflection) into my
> repository (pflow). The prefix I used was lib/ezc/trunk/Reflection. Now
> there have been changes to ezc-reflection but merging them into my
> repository fails. The merge is attempted in example/ rather than
> lib/ezc/trunk/Reflection.
The auto-guessing feature of "merge -s subtree" doesn't always work.
The 'pu' branch of git has some patches which allow you to use a
command line like
git merge -Xsubtree=lib/ezc/trunk/Reflection
If you're nervous about using all of 'pu', try building git's 'master'
after merging from commit 0354b06927a1220b696a5ee1004a8f061ba9b153.
Eventually my git-subtree tool will use this automatically, but it's
obviously not much use until the patches hit a released version of
git.
Hope this helps.
Avery
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-10 14:40 UTC (permalink / raw)
To: gitster; +Cc: git, Shawn O. Pearce
In-Reply-To: <20091210140317.GA30665@spearce.org>
On Thursday 10 December 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > Do you have more comments/suggestions on this patch? Or is it ok to
> > include in fast-import as-is?
>
> Oops, sorry.
>
> No, no additional comments. I am happy with this patch.
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Thanks.
Junio: With the above Ack, I believe patches #1 - #4 (and possibly #5)
from this series are ready for 'next'.
You may want to hold off on the remainder of the series until I get
around to writing some functionality that actually _uses_ the new API.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [REROLL PATCH v2 6/8] Support remote helpers implementing smart transports
From: Shawn O. Pearce @ 2009-12-10 15:58 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1260372394-16427-7-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
This is better.
> + if (strcmp(name, exec)) {
> + r = set_helper_option(transport, "servpath", exec);
> + if (r > 0)
> + fprintf(stderr, "Warning: Setting remote service path "
> + "not supported by protocol.\n");
> + else if (r < 0)
> + fprintf(stderr, "Warning: Invalid remote service "
> + "path.\n");
Style-nit: We prefer "warning: " with lowercase.
--
Shawn.
^ permalink raw reply
* Re: [REROLL PATCH v2 6/8] Support remote helpers implementing smart transports
From: Junio C Hamano @ 2009-12-10 17:10 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Ilari Liusvaara, git
In-Reply-To: <20091210155811.GC30665@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
>> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
>
> This is better.
Is this sarcasm?
As far as I can tell, there is no "Junio received and added some that is
copyrightable, and Ilari later received it and worked on it" anywhere in
this series to require recording of patch ping-pong, except perhaps 3/8
where I gave remove_ext_force() with a shorter implementation. The only
edit I did myself was to fix style.
I did suggest renaming a few functions but that (1) eventually was done by
Ilari, and (2) isn't large enough change to be copyrightable, I think.
>> + if (strcmp(name, exec)) {
>> + r = set_helper_option(transport, "servpath", exec);
>> + if (r > 0)
>> + fprintf(stderr, "Warning: Setting remote service path "
>> + "not supported by protocol.\n");
>> + else if (r < 0)
>> + fprintf(stderr, "Warning: Invalid remote service "
>> + "path.\n");
>
> Style-nit: We prefer "warning: " with lowercase.
Also this could be a semantic nit; we prefer to call warning(), which is
meant to be overridable by the main program (it probably does not matter,
though).
^ permalink raw reply
* Re: [PATCH RFC] rebase: add --revisions flag
From: Junio C Hamano @ 2009-12-10 17:20 UTC (permalink / raw)
To: Björn Steinbrink
Cc: Michael S. Tsirkin, Andreas Schwab, Peter Krefting,
Git Mailing List
In-Reply-To: <20091210074358.GA7723@atjola.homenet>
Björn Steinbrink <B.Steinbrink@gmx.de> writes:
>> But at the conceptual level, "merge --squash" is a short-hand for this
>> command sequence:
>>
>> git rebase -i HEAD that-branch
>> ... make everything except the first one into "squash"
>> git checkout - ;# come back to the original branch
>> git merge that-branch ;# fast forward to it
>>
>> So after all, it is "merge it after squashing them".
>
> To me, that approach looks backwards,...
Yes, of course, but what you are missing (and I am at blame for forgetting
to mention the history behind this in the message you are responding to)
is that "merge --squash" to support a particular need/use case was done
way before "rebase -i" came into existence.
Here is how "merge --squash" is explained in the log message:
git-merge --squash
Some people tend to do many little commits on a topic branch,
recording all the trials and errors, and when the topic is
reasonably cooked well, would want to record the net effect of
the series as one commit on top of the mainline, removing the
cruft from the history. The topic is then abandoned or forked
off again from that point at the mainline.
A nicer workflow may be to use "rebase -i" to clean up the history before
even contemplating to integrate the topic to the mainline, instead of the
above "abandoning or forking off again", if you know today's git.
But interactive was not available back then. It was introduced at 1b1dce4
(Teach rebase an interactive mode, 2007-06-25), which is 1 year after
7d0c688 (git-merge --squash, 2006-06-23).
^ permalink raw reply
* [PATCH v3 1/2] Makefile: Add a target to generate C preprocessor output files
From: Ramsay Jones @ 2009-12-10 18:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
No changes from version 1 (apart from re-basing onto a more recent
commit).
ATB,
Ramsay Jones
Makefile | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 4a1e5bc..fdd9ce8 100644
--- a/Makefile
+++ b/Makefile
@@ -1584,6 +1584,8 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+%.i: %.c .ALWAYS
+ $(QUIET_CC)$(CC) -E $(ALL_CFLAGS) $< >$*.i
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS
@@ -1924,7 +1926,7 @@ endif
.PHONY: all install clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .ALWAYS
### Check documentation
#
--
1.6.5
^ permalink raw reply related
* Re: [RFC PATCH v2 2/2] MSVC: Fix an "incompatible pointer types" compiler warning
From: Ramsay Jones @ 2009-12-10 18:38 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, Johannes Schindelin, GIT Mailing-list
In-Reply-To: <200912082131.44498.j6t@kdbg.org>
Johannes Sixt wrote:
> To cut this short: According to your explanations, using -D_USE_32BIT_TIME_T
> with MSVC is bad. Please reroll without references to _USE_32BIT_TIME_T.
OK, will do...
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH v3 2/2] MSVC: Fix an "incompatible pointer types" compiler warning
From: Ramsay Jones @ 2009-12-10 19:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
In particular, the following warning is issued while compiling
compat/msvc.c:
...mingw.c(223) : warning C4133: 'function' : incompatible \
types - from '_stati64 *' to '_stat64 *'
which relates to a call of _fstati64() in the mingw_fstat()
function definition.
This is caused by various layers of macro magic and attempts to
avoid macro redefinition compiler warnings. For example, the call
to _fstati64() mentioned above is actually a call to _fstat64(),
and expects a pointer to a struct _stat64 rather than the struct
_stati64 which is passed to mingw_fstat().
The definition of struct _stati64 given in compat/msvc.h had the
same "shape" as the definition of struct _stat64, so the call to
_fstat64() does not actually cause any runtime errors, but the
structure types are indeed incompatible.
In order to avoid the compiler warning, we add declarations for the
mingw_lstat() and mingw_fstat() functions and supporting macros to
msvc.h, suppressing the corresponding declarations in mingw.h, so
that we can use the appropriate structure type (and function) names
from the msvc headers.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Changes from v2:
- removed the #if conditionals and related code depending on
the _USE_32BIT_TIME_T macro being defined.
- re-worded the commit message to reflect the above.
We assume that git will never be compiled with -D_USE_32BIT_TIME_T,
otherwise this patch will introduce a breakage that the current code
does not suffer.
[Hmm, should the above paragraph (or one like it) be added to the
commit message?]
Note that, if we wish to support 64-bit Windows, we would not be able
to use this macro anyway (it elicits an #error message on WIN64).
(ie. a time_t can only be 64-bits on WIN64)
ATB,
Ramsay Jones
compat/mingw.h | 4 +++-
compat/msvc.h | 40 ++++++++++++++++------------------------
2 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index b3d299f..55fd7fa 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -209,13 +209,15 @@ int mingw_getpagesize(void);
* mingw_fstat() instead of fstat() on Windows.
*/
#define off_t off64_t
-#define stat _stati64
#define lseek _lseeki64
+#ifndef ALREADY_DECLARED_STAT_FUNCS
+#define stat _stati64
int mingw_lstat(const char *file_name, struct stat *buf);
int mingw_fstat(int fd, struct stat *buf);
#define fstat mingw_fstat
#define lstat mingw_lstat
#define _stati64(x,y) mingw_lstat(x,y)
+#endif
int mingw_utime(const char *file_name, const struct utimbuf *times);
#define utime mingw_utime
diff --git a/compat/msvc.h b/compat/msvc.h
index 9c753a5..023aba0 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -21,30 +21,22 @@ static __inline int strcasecmp (const char *s1, const char *s2)
}
#undef ERROR
-#undef stat
-#undef _stati64
-#include "compat/mingw.h"
-#undef stat
-#define stat _stati64
+
+/* Use mingw_lstat() instead of lstat()/stat() and mingw_fstat() instead
+ * of fstat(). We add the declaration of these functions here, suppressing
+ * the corresponding declarations in mingw.h, so that we can use the
+ * appropriate structure type (and function) names from the msvc headers.
+ */
+#define stat _stat64
+int mingw_lstat(const char *file_name, struct stat *buf);
+int mingw_fstat(int fd, struct stat *buf);
+#define fstat mingw_fstat
+#define lstat mingw_lstat
#define _stat64(x,y) mingw_lstat(x,y)
+#define ALREADY_DECLARED_STAT_FUNCS
+
+#include "compat/mingw.h"
+
+#undef ALREADY_DECLARED_STAT_FUNCS
-/*
- Even though _stati64 is normally just defined at _stat64
- on Windows, we specify it here as a proper struct to avoid
- compiler warnings about macro redefinition due to magic in
- mingw.h. Struct taken from ReactOS (GNU GPL license).
-*/
-struct _stati64 {
- _dev_t st_dev;
- _ino_t st_ino;
- unsigned short st_mode;
- short st_nlink;
- short st_uid;
- short st_gid;
- _dev_t st_rdev;
- __int64 st_size;
- time_t st_atime;
- time_t st_mtime;
- time_t st_ctime;
-};
#endif
--
1.6.5
^ permalink raw reply related
* [BUG] git archive formats and dashdash
From: Ilari Liusvaara @ 2009-12-10 21:26 UTC (permalink / raw)
To: git
--format option of git archive stops working if -- is used:
Shell session:
----------------
$ git archive -o test3.zip --format=zip master -- exec_cmd.c
$ file test3.zip
test3.zip: POSIX tar archive
# Eh...
$ git archive -o test4.zip --format=zip master exec_cmd.c
$ file test4.zip
test4.zip: Zip archive data, at least v1.0 to extract
# That worked.
----------------
The bug seems to be in that archive appends the --format
option to its arguments, not taking into account that there
may be -- before that point, disabling option interpretation.
Git version 1.6.6-rc2
-Ilari
^ permalink raw reply
* Re: [BUG] git archive formats and dashdash
From: Junio C Hamano @ 2009-12-10 22:05 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091210212636.GA27722@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> --format option of git archive stops working if -- is used:
Good catch. Is this a regression between 1.6.5 and the current code?
builtin-archive.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 12351e9..8ef5ab3 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -106,13 +106,17 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
if (format) {
sprintf(fmt_opt, "--format=%s", format);
/*
- * This is safe because either --format and/or --output must
- * have been given on the original command line if we get to
- * this point, and parse_options() must have eaten at least
- * one argument, i.e. we have enough room to append to argv[].
+ * We have enough room in argv[] to muck it in place,
+ * because either --format and/or --output must have
+ * been given on the original command line if we get
+ * to this point, and parse_options() must have eaten
+ * it, i.e. we can add back one element to the array.
+ * But argv[] may contain "--" so we should make this
+ * the first option.
*/
- argv[argc++] = fmt_opt;
- argv[argc] = NULL;
+ memmove(argv + 2, argv + 1, sizeof(*argv) * argc);
+ argv[1] = fmt_opt;
+ argv[++argc] = NULL;
}
if (remote)
^ permalink raw reply related
* [PATCH] builtin-archive: insert --format before double dash if necessary.
From: Miklos Vajna @ 2009-12-10 22:20 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091210212636.GA27722@Knoppix>
Reported-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
What about this fix?
builtin-archive.c | 23 ++++++++++++++++++++++-
1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 12351e9..ffbc9b0 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -104,14 +104,35 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
}
if (format) {
+ int i, found = 0;
+ const char *ptr = NULL;
+
sprintf(fmt_opt, "--format=%s", format);
/*
* This is safe because either --format and/or --output must
* have been given on the original command line if we get to
* this point, and parse_options() must have eaten at least
* one argument, i.e. we have enough room to append to argv[].
+ *
+ * First check if we have to insert the argument before
+ * two dashes.
*/
- argv[argc++] = fmt_opt;
+ for (i = 0; i < argc; i++) {
+ if (found) {
+ const char *tmp = argv[i];
+ argv[i] = ptr;
+ ptr = tmp;
+ } else if (!strcmp(argv[i], "--")) {
+ found = 1;
+ ptr = argv[i];
+ argv[i] = fmt_opt;
+ argc++;
+ }
+ }
+
+ /* No double dash? Then just append it to the end of the list. */
+ if (!found)
+ argv[argc++] = fmt_opt;
argv[argc] = NULL;
}
--
1.6.5.2
^ permalink raw reply related
* Re: [BUG] git archive formats and dashdash
From: Miklos Vajna @ 2009-12-10 22:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7v1vj2a3ik.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
On Thu, Dec 10, 2009 at 02:05:39PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> Good catch. Is this a regression between 1.6.5 and the current code?
Ah, you version is much shorter. :)
I think it was introduced by 0f4b377c (git-archive: infer output format
from filename when unspecified, 2009-09-14), so it was introduced
before 1.6.5.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [BUG] git archive formats and dashdash
From: Ilari Liusvaara @ 2009-12-10 22:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1vj2a3ik.fsf@alter.siamese.dyndns.org>
On Thu, Dec 10, 2009 at 02:05:39PM -0800, Junio C Hamano wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
>
> > --format option of git archive stops working if -- is used:
>
> Good catch. Is this a regression between 1.6.5 and the current code?
Doesn't appear to be so:
Based on quick look at source, it seems that this is indeed regression,
and the commit which introduced it is:
commit 0f4b377c20fb7d93f8bfeec39efb2b9392d6aebc
Author: Dmitry Potapov <dpotapov@gmail.com>
Date: Mon Sep 14 00:17:01 2009 +0400
Describe: v1.6.5-rc1-7-g0f4b377 / v1.6.5-rc2~34
-Ilari
^ 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