* Re: git-rm -n leaves .git/index.lock if not allowed to finish
From: Junio C Hamano @ 2008-12-19 1:31 UTC (permalink / raw)
To: Miklos Vajna; +Cc: jidanni, git
In-Reply-To: <7v4p11f18d.fsf@gitster.siamese.dyndns.org>
Subject: [PATCH] Make sure lockfiles are unlocked when dying on SIGPIPE
We cleaned up lockfiles upon receiving the usual suspects HUP, TERM, QUIT
but a wicked user could kill us of asphyxiation by piping our output to a
pipe that does not read. Protect ourselves by catching SIGPIPE and clean
up the lockfiles as well in such a case.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
lockfile.c | 1 +
t/t3600-rm.sh | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git c/lockfile.c w/lockfile.c
index 6d75608..8589155 100644
--- c/lockfile.c
+++ w/lockfile.c
@@ -140,6 +140,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
signal(SIGHUP, remove_lock_file_on_signal);
signal(SIGTERM, remove_lock_file_on_signal);
signal(SIGQUIT, remove_lock_file_on_signal);
+ signal(SIGPIPE, remove_lock_file_on_signal);
atexit(remove_lock_file);
}
lk->owner = getpid();
diff --git c/t/t3600-rm.sh w/t/t3600-rm.sh
index b7d46e5..95542e9 100755
--- c/t/t3600-rm.sh
+++ w/t/t3600-rm.sh
@@ -251,4 +251,21 @@ test_expect_success 'refresh index before checking if it is up-to-date' '
'
+test_expect_success 'choking "git rm" should not let it die with cruft' '
+ git reset -q --hard &&
+ H=0000000000000000000000000000000000000000 &&
+ i=0 &&
+ while test $i -lt 12000
+ do
+ echo "100644 $H 0 some-file-$i"
+ i=$(( $i + 1 ))
+ done | git update-index --index-info &&
+ git rm -n "some-file-*" | :;
+ test -f .git/index.lock
+ status=$?
+ rm -f .git/index.lock
+ git reset -q --hard
+ test "$status" != 0
+'
+
test_done
^ permalink raw reply related
* [PATCH] Remove the requirement opaquelocktoken uri scheme
From: Kirill A. Korinskiy @ 2008-12-19 1:51 UTC (permalink / raw)
To: git; +Cc: gitster, Kirill A. Korinskiy
Server can use any URI for token by rfc 4918 section 6.5 paragraph five
Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
http-push.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/http-push.c b/http-push.c
index 5cecef434a7740a3f853462978c3e071b4da7e74..7c6460919bf3eba10c46cede11ffdd9c53fd2dd2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -595,7 +595,7 @@ static int refresh_lock(struct remote_lock *lock)
lock->refreshing = 1;
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
sprintf(timeout_header, "Timeout: Second-%ld", lock->timeout);
dav_headers = curl_slist_append(dav_headers, if_header);
dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1120,10 +1120,8 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
lock->timeout =
strtol(ctx->cdata + 7, NULL, 10);
} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
- if (!prefixcmp(ctx->cdata, "opaquelocktoken:")) {
- lock->token = xmalloc(strlen(ctx->cdata) - 15);
- strcpy(lock->token, ctx->cdata + 16);
- }
+ lock->token = xmalloc(strlen(ctx->cdata) + 1);
+ strcpy(lock->token, ctx->cdata);
}
}
}
@@ -1308,7 +1306,7 @@ static int unlock_remote(struct remote_lock *lock)
int rc = 0;
lock_token_header = xmalloc(strlen(lock->token) + 31);
- sprintf(lock_token_header, "Lock-Token: <opaquelocktoken:%s>",
+ sprintf(lock_token_header, "Lock-Token: <%s>",
lock->token);
dav_headers = curl_slist_append(dav_headers, lock_token_header);
@@ -1722,7 +1720,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
struct curl_slist *dav_headers = NULL;
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
dav_headers = curl_slist_append(dav_headers, if_header);
strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
@@ -1941,7 +1939,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
add_remote_info_ref, &buffer.buf);
if (!aborted) {
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
dav_headers = curl_slist_append(dav_headers, if_header);
slot = get_active_slot();
--
1.5.6.5
^ permalink raw reply related
* Re: Git with Hudson
From: Tim Visher @ 2008-12-19 1:58 UTC (permalink / raw)
To: Stephen Haberman; +Cc: Indy Nagpal, git
In-Reply-To: <20081218160734.b1992eb8.stephen@exigencecorp.com>
On Thu, Dec 18, 2008 at 5:07 PM, Stephen Haberman
<stephen@exigencecorp.com> wrote:
>
> I've got permission to publish it if you're interested--just haven't
> yet.
I'm interested! Please publish away!
--
In Christ,
Timmy V.
http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail
^ permalink raw reply
* sneakernet everyday recommendations?
From: jidanni @ 2008-12-19 2:33 UTC (permalink / raw)
To: git; +Cc: etckeeper
Say, on a sneakernet what would you use for
A>B A<B A>B... updating? git-commit and git-bundle and git-merge?
You see I have the two computers, that I use on alternating days, a
sort of redundancy strategy.
Normally if I change my .bashrc or /etc/ppp/ip-up.d/bla, etc. I just
cpio -o it to a CF card, the common sneakernet junction between my two
systems that are never powered up at the same time. And then cpio -i
it to the other system the following day.
To use git instead, On each computer I shall have my shutdown script
ask me about a commit, and my boot script ask me about a merge.
To track metadata, I will employ etckeeper.
I don't want to track the entire machines, just 100 or so files, which
I have git-added.
(Hmmm, also copying /var/cache/debconf/config.dat and
/var/lib/aptitude/pkgstates to the CF card
should be enough to complete my minimal restoreability strategy
in contrast to the many more bulkier 'system snapshot' solutions out there).
^ permalink raw reply
* [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-19 2:39 UTC (permalink / raw)
To: git
Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
On Thursday 2008 December 18 18:21:25 Linus Torvalds wrote:
> I suspect we should warn about reverting merges.
Here is a patch (aginst c0ceb2c, which I believe is master currently) that
does just that.
After applying the patch I get the following test results:
fixed 1
success 4108
failed 0
broken 4
total 4113
builtin-revert.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c
index 4038b41..7f121a5 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -296,6 +296,21 @@ static int revert_or_cherry_pick(int argc, const char
**argv)
int cnt;
struct commit_list *p;
+ do {
+ switch (action) {
+ case REVERT:
+ warning("revert on a merge commit may not do what you expect.");
+ continue;
+ case CHERRY_PICK:
+ /* Cherry picking a merge doesn't merge the history, but
+ * I don't think many people expect that.
+ */
+ continue;
+ }
+ /* Unhandled enum member. */
+ die("Unknown action on a merge commit.");
+ } while (0);
+
if (!mainline)
die("Commit %s is a merge but no -m option was given.",
sha1_to_hex(commit->object.sha1));
--
1.5.6
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* Re: [PATCH] Simplified GIT usage guide
From: Johannes Schindelin @ 2008-12-19 2:38 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: David Howells, torvalds, git, linux-kernel
In-Reply-To: <20081219000218.GA23990@linux.vnet.ibm.com>
Hi,
On Thu, 18 Dec 2008, Paul E. McKenney wrote:
> On Fri, Dec 12, 2008 at 07:57:38PM +0100, Johannes Schindelin wrote:
>
> > On Fri, 12 Dec 2008, David Howells wrote:
> >
> > > Documentation/git-haters-guide.txt | 1283 ++++++++++++++++++++++++++++++++++++
> >
> > I am sure we want to have something like that in git.git.
>
> So am I. Except that I am not being sarcastic. ;-)
Good idea. Ask somebody to put up some advertisement _against_ her own
product into their shop window.
Very good idea.
Ciao,
Dscho
P.S.: Can I ask you to shoot yourself in the foot?
P.P.S.: If you are really a Git hater, why are you lurking here? Go away,
Subversion is not that bad.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Johannes Schindelin @ 2008-12-19 2:57 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: git
In-Reply-To: <200812182039.15169.bss@iguanasuicide.net>
Hi,
On Thu, 18 Dec 2008, Boyd Stephen Smith Jr. wrote:
> + do {
> + switch (action) {
> + case REVERT:
> + warning("revert on a merge commit may not do what you expect.");
> + continue;
> + case CHERRY_PICK:
> + /* Cherry picking a merge doesn't merge the history, but
> + * I don't think many people expect that.
> + */
> + continue;
> + }
> + /* Unhandled enum member. */
> + die("Unknown action on a merge commit.");
> + } while (0);
> +
Wow. That must be one of the, uhm, less beautiful ways to write
if (action == REVERT)
warning("revert on a merge commit may not do what you "
"expect.");
else if (action != CHERRY_PICK)
die("Unknown action on a merge commit.");
Besides, I am actually pretty much against this change. You already have
to ask very explicitely to revert a merge, by specifying a parent number.
If I ask for something explicitely, I do not want the tool to tell me that
it's dangerous. I know that already, thankyouverymuch.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-19 3:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <alpine.DEB.1.00.0812190353520.14632@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Wow. That must be one of the, uhm, less beautiful ways to write
>
> if (action == REVERT)
> warning("revert on a merge commit may not do what you "
> "expect.");
> else if (action != CHERRY_PICK)
> die("Unknown action on a merge commit.");
>
> Besides, I am actually pretty much against this change. You already have
> to ask very explicitely to revert a merge, by specifying a parent number.
> If I ask for something explicitely, I do not want the tool to tell me that
> it's dangerous. I know that already, thankyouverymuch.
Or you may not have known that it is dangerous, but the new warning does
not give you enough clue where to go next, so this warning does not give
real value. It is pretty much meaningless noise to users.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-19 3:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Alan, Linus Torvalds
In-Reply-To: <alpine.DEB.1.00.0812190353520.14632@racer>
[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]
Blah, my --in-reply-to didn't work so this didn't thread right.
On Thursday 2008 December 18 20:57:57 you wrote:
> On Thu, 18 Dec 2008, Boyd Stephen Smith Jr. wrote:
> > + do {
> > + switch (action) {
> > + case REVERT:
> > + warning("revert on a merge commit may not do what you expect.");
> > + continue;
> > + case CHERRY_PICK:
> > + /* Cherry picking a merge doesn't merge the history, but
> > + * I don't think many people expect that.
> > + */
> > + continue;
> > + }
> > + /* Unhandled enum member. */
> > + die("Unknown action on a merge commit.");
> > + } while (0);
> > +
>
> Wow. That must be one of the, uhm, less beautiful ways to write
>
> if (action == REVERT)
> warning("revert on a merge commit may not do what you "
> "expect.");
> else if (action != CHERRY_PICK)
> die("Unknown action on a merge commit.");
My way, a smart compiler will warn at compile time that there's a new enum
member that needs to be handled. Your way, no such compile-time warning will
be emitted. At runtime, they have the same behavior. Athestically, I agree
with you, but my way may have technical advantages.
I did check the CodingGuidelines and didn't see this construct mentioned.
> Besides, I am actually pretty much against this change.
I've never had a need to revert a merge commit, so it's not a big win either
way for me. I wrote the patch because alan@clueserver.org had the revert
behavior bite him and Linus suggested a warning might be apropos.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-19 3:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin, Linus Torvalds, Alan
In-Reply-To: <7vej04eui5.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 903 bytes --]
On Thursday 2008 December 18 21:03:46 Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > warning("revert on a merge commit may not do what you "
> > "expect.");
>
> [T]he new warning does
> not give you enough clue where to go next, so this warning does not give
> real value. It is pretty much meaningless noise to users.
At least, it might make someone read the manpage again. Still, I'm unhappy
with the message, but I didn't want to be too wordy. A URL or manpage
reference would be nice, but I didn't know of a good guide that explained the
dangers of reverting a merge commit as well as Linus's emails.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] git-send-email: handle email address with quoted comma
From: Wu Fengguang @ 2008-12-19 3:40 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Wu Fengguang
Correctly handle email addresses containing quoted commas, e.g.
"Zhu, Yi" <yi.zhu@intel.com>, "Li, Shaohua" <shaohua.li@intel.com>
Here the commas inside the double quotes are NOT email separators.
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
git-send-email.perl | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3112f76..d44e99c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -20,6 +20,7 @@ use strict;
use warnings;
use Term::ReadLine;
use Getopt::Long;
+use Text::ParseWords;
use Data::Dumper;
use Term::ANSIColor;
use File::Temp qw/ tempdir /;
@@ -359,6 +360,12 @@ foreach my $entry (@bcclist) {
die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
}
+sub split_addrs($) {
+ my ($addrs) = @_;
+
+ return "ewords('\s*,\s*', 1, $addrs);
+}
+
my %aliases;
my %parse_alias = (
# multiline formats can be supported in the future
@@ -367,7 +374,7 @@ my %parse_alias = (
my ($alias, $addr) = ($1, $2);
$addr =~ s/#.*$//; # mutt allows # comments
# commas delimit multiple addresses
- $aliases{$alias} = [ split(/\s*,\s*/, $addr) ];
+ $aliases{$alias} = [ split_addrs($addr) ];
}}},
mailrc => sub { my $fh = shift; while (<$fh>) {
if (/^alias\s+(\S+)\s+(.*)$/) {
@@ -379,7 +386,7 @@ my %parse_alias = (
chomp $x;
$x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);
$x =~ /^(\S+)$f\t\(?([^\t]+?)\)?(:?$f){0,2}$/ or next;
- $aliases{$1} = [ split(/\s*,\s*/, $2) ];
+ $aliases{$1} = [ split_addrs($2) ];
}},
gnus => sub { my $fh = shift; while (<$fh>) {
if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
@@ -588,7 +595,7 @@ if (!@to) {
}
my $to = $_;
- push @to, split /,\s*/, $to;
+ push @to, split_addrs($to);
$prompting++;
}
--
1.6.0.4
^ permalink raw reply related
* Re: Odd merge behaviour involving reverts
From: Nanako Shiraishi @ 2008-12-19 3:44 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alan, git
In-Reply-To: <alpine.LFD.2.00.0812181534310.14014@localhost.localdomain>
Quoting Linus Torvalds <torvalds@linux-foundation.org>:
> On Thu, 18 Dec 2008, Alan wrote:
>>
>> What am i doing wrong here?
>
> Reverting a merge is your problem.
>
> You can do it, but you seem to have done it without understanding what it
> causes.
>
> A revert of a merge becomes a regular commit that just undoes everything
> that the merge did in your branch. When you then do the next merge, you'll
> do that merge with that in mind, so now git will essentially consider the
> previous merge to be the base line, but your revert undid everything that
> that one brought in, so the new merge will really only contain the new
> stuff from the branch you are merging.
>
> So if a merge causes problems, you generally should either undo it
> _entirely_ (ie do a 'git reset --hard ORIG_HEAD'), not revert it.
>
> Of course, if you had already made the merged state public, or done
> development on top of it, you can't really do that. In which case a revert
> works, but if you want it back, you should revert the revert, not merge
> the branch again - because what you merged last time you threw away, and
> won't be applied again.
If I understand Alan's case correctly, I think he does not want to "undo" the revert but wants to merge an updated version of the branch, as if no mistaken merge nor its revert happened in the past.
If you revert the revert on the branch before merging, doesn't it mean that you will be merging what the older version of the branch did (that is in the revert of the revert as a single huge patch) and what the updated version of the branch wants to do? Wouldn't that lead to a mess with huge conflicts?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: git-rm -n leaves .git/index.lock if not allowed to finish
From: Miklos Vajna @ 2008-12-19 3:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: jidanni, git
In-Reply-To: <7v63lhf1cl.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 438 bytes --]
On Thu, Dec 18, 2008 at 04:35:54PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I think you need to have tons of files to cause the pipe buffer to fill up
> with the "rm 'frotz'" output, triggering a SIGPIPE to kill the upstream
> process of the pipe.
Ah, you are right. I did exactly what is in the lockfile.c part of your
patch, but I did not think about "if I don't get a SIGPIPE for 2 lines,
I may get one for 1000 lines". ;-)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Jay Soffian @ 2008-12-19 3:55 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.
Cc: git, Junio C Hamano, Johannes Schindelin, Linus Torvalds, Alan
In-Reply-To: <200812182129.01021.bss@iguanasuicide.net>
On Thu, Dec 18, 2008 at 10:29 PM, Boyd Stephen Smith Jr.
<bss@iguanasuicide.net> wrote:
> At least, it might make someone read the manpage again. Still, I'm unhappy
> with the message, but I didn't want to be too wordy. A URL or manpage
> reference would be nice, but I didn't know of a good guide that explained the
> dangers of reverting a merge commit as well as Linus's emails.
Put his email in Documentation/howto/undoing-merge-commits.txt and
reference that?
j.
^ permalink raw reply
* Re: Odd merge behaviour involving reverts
From: Linus Torvalds @ 2008-12-19 4:01 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Alan, git
In-Reply-To: <20081219124452.6117@nanako3.lavabit.com>
On Fri, 19 Dec 2008, Nanako Shiraishi wrote:
>
> If you revert the revert on the branch before merging, doesn't it mean
> that you will be merging what the older version of the branch did (that
> is in the revert of the revert as a single huge patch) and what the
> updated version of the branch wants to do? Wouldn't that lead to a mess
> with huge conflicts?
Actually, the reverse is likely true. If the branch you are merging is
actually doing something branch-specific - ie it's a "topic branch", then
it's likely that the new stuff that is on that branch depends on the
previous stuff on the branch.
And thats' the thing that got reverted - so with just a revert, it's quite
likely that you'll get conflicts. But if you revert the revert, now the
new stuff you're merging actually makes more sense, and is less likely to
conflict.
Another way of looking at it is that a merge is something that can be done
both ways: think of the _other_ branch merging yours. The original revert
ends up being a big change-patch that undoes everything that other branch
did, so now if that other branch were to merge the main branch, you'd be
merging a lot of changes. But reverting the revert will undo all those
changes, so again, it's more likely that the merge will succeed.
So revertign a revert is usually going to make subsequent merges easier
rather than the reverse.
The _big_ problem with reverting a whole merge is that it effectively
becomes one commit that does a big change. That's how _normal_ merges tend
to look like in CVS or SVN (ie the "merge" is really just another commit
that brings in a lot of changes), and it's a total and utter f*cking
disaster!
But don't get me wrong - it's not something you can't work with. I'm just
trying to say that it's absolutely not a "good model". It's workable, but
it has all these painful issues.
For example, think about what reverting a merge (and then reverting the
revert) does to bisectability. Ignore the fact that the revert of a revert
is undoing it - just think of it as a "single commit that does a lot".
Because that is what it does.
When you have a problem you are chasing down, and you hit a "revert this
merge", what you're hitting is essentially a single commit that contains
all the changes (but obviously in reverse) of all the commits that got
merged. So it's debugging hell, because now you don't have lots of small
changes that you can try to pinpoint which _part_ of it changes.
But does it all work? Sure it does. You can revert a merge, and from a
purely technical angle, git did it very naturally and had no real
troubles. It just considered it a change from "state before merge" to
"state after merge", and that was it. Nothing complicated, nothing odd,
nothing really dangerous. Git will do it without even thinking about it.
So from a technical angle, there's nothing wrong with reverting a merge,
but from a workflow angle it's something that you generally should try to
avoid.
If at all possible, for example, if you find a problem that got merged
into the main tree, rather than revert the merge, try _really_ hard to
bisect the problem down into the branch you merged, and just fix it, or
try to revert the individual commit that caused it.
Yes, it's more complex, and no, it's not always going to work (sometimes
the answer is: "oops, I really shouldn't have merged it, because it wasn't
ready yet, and I really need to undo _all_ of the merge"). So then you
really should revert the merge, but when you want to re-do the merge, you
now need to do it by reverting the revert.
Linus
^ permalink raw reply
* Re: Odd merge behaviour involving reverts
From: Jay Soffian @ 2008-12-19 4:18 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nanako Shiraishi, Alan, git
In-Reply-To: <alpine.LFD.2.00.0812181949450.14014@localhost.localdomain>
On Thu, Dec 18, 2008 at 11:01 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Yes, it's more complex, and no, it's not always going to work (sometimes
> the answer is: "oops, I really shouldn't have merged it, because it wasn't
> ready yet, and I really need to undo _all_ of the merge"). So then you
> really should revert the merge, but when you want to re-do the merge, you
> now need to do it by reverting the revert.
Instead of reverting the revert, why not rebase the branch which was
merged to the point after the revert was done?
j.
^ permalink raw reply
* Re: Odd merge behaviour involving reverts
From: Linus Torvalds @ 2008-12-19 4:23 UTC (permalink / raw)
To: Jay Soffian; +Cc: Nanako Shiraishi, Alan, git
In-Reply-To: <76718490812182018x37e3d6fob8d817c0e0b0e293@mail.gmail.com>
On Thu, 18 Dec 2008, Jay Soffian wrote:
>
> Instead of reverting the revert, why not rebase the branch which was
> merged to the point after the revert was done?
Sure, you can do that too. However, that has its own set of problems, ie
all the "ok, who else was working based on this branch" thing.
But no, there are no single right answers. Rebasing may well be a
reasonable approach.
Linus
^ permalink raw reply
* Re: Git with Hudson
From: Dilip M @ 2008-12-19 4:58 UTC (permalink / raw)
To: git
In-Reply-To: <D2F0F023-862A-4BAB-88B9-BFEFC5592D10@strakersoftware.com>
On Fri, Dec 19, 2008 at 12:53 AM, <indy@strakersoftware.com> wrote:
On Fri, Dec 19, 2008 at 3:37 AM, <stephen@exigencecorp.com> wrote:
indy> However, before we do that I wanted to check if anyone has had any
indy> experience/feedback in integrating Git with Hudson CI server?
We want to try it in futrure when GIT will be used for some projects. It
will be nice to share the knowledge on hudson tool. :)
B.W, what version of hudson you are using? Java version? TOMCAT
version?
It is stable? Have you seen something like "(appears to be stuck)
entries"...
stephen> We eventually wrote our own Hudson git plugin that is simpler and
stephen> doesn't do any funny rev-listing/walking. It just stores last hash
stephen> built and rebuilds once that doesn't match the branch tip. Once that
stephen> was in place, it worked great.
I am eager to have it..please publish.
-dm
^ permalink raw reply
* Re: Odd merge behaviour involving reverts
From: Daniel Barkalow @ 2008-12-19 5:24 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nanako Shiraishi, Alan, git
In-Reply-To: <alpine.LFD.2.00.0812181949450.14014@localhost.localdomain>
On Thu, 18 Dec 2008, Linus Torvalds wrote:
> On Fri, 19 Dec 2008, Nanako Shiraishi wrote:
> >
> > If you revert the revert on the branch before merging, doesn't it mean
> > that you will be merging what the older version of the branch did (that
> > is in the revert of the revert as a single huge patch) and what the
> > updated version of the branch wants to do? Wouldn't that lead to a mess
> > with huge conflicts?
>
> Actually, the reverse is likely true. If the branch you are merging is
> actually doing something branch-specific - ie it's a "topic branch", then
> it's likely that the new stuff that is on that branch depends on the
> previous stuff on the branch.
>
> And thats' the thing that got reverted - so with just a revert, it's quite
> likely that you'll get conflicts. But if you revert the revert, now the
> new stuff you're merging actually makes more sense, and is less likely to
> conflict.
>
> Another way of looking at it is that a merge is something that can be done
> both ways: think of the _other_ branch merging yours. The original revert
> ends up being a big change-patch that undoes everything that other branch
> did, so now if that other branch were to merge the main branch, you'd be
> merging a lot of changes. But reverting the revert will undo all those
> changes, so again, it's more likely that the merge will succeed.
>
> So revertign a revert is usually going to make subsequent merges easier
> rather than the reverse.
>
> The _big_ problem with reverting a whole merge is that it effectively
> becomes one commit that does a big change. That's how _normal_ merges tend
> to look like in CVS or SVN (ie the "merge" is really just another commit
> that brings in a lot of changes), and it's a total and utter f*cking
> disaster!
Another option is to do the big revert on the master branch, and on the
side branch merge the parent of the revert (normally), and then merge the
revert but take the side branch tree.
That last merge puts the revert in the branch's history, but rejects its
effects. Now merging the branch again will find the revert to be the
common ancestor, and see the full change of the side branch as the
contribution of that side. Also, blame will come down the side branch,
ignore the merge (on the side branch, the merge contributed no content
changes), and go into the original side branch commits.
You can also think of it as the side branch saying, "I noticed that you
reverted my changes, but I'm keeping them anyway". When the final merge
comes, git will see that the revert isn't new information to the side
branch, and nullify its effect. (Of course, the side branch needs to merge
the parent of the revert so that it isn't rejecting the other changes made
on the main branch before the revert)
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] Simplified GIT usage guide
From: Paul E. McKenney @ 2008-12-19 5:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: David Howells, torvalds, git, linux-kernel
In-Reply-To: <alpine.DEB.1.00.0812190336450.14632@racer>
On Fri, Dec 19, 2008 at 03:38:58AM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 18 Dec 2008, Paul E. McKenney wrote:
>
> > On Fri, Dec 12, 2008 at 07:57:38PM +0100, Johannes Schindelin wrote:
> >
> > > On Fri, 12 Dec 2008, David Howells wrote:
> > >
> > > > Documentation/git-haters-guide.txt | 1283 ++++++++++++++++++++++++++++++++++++
> > >
> > > I am sure we want to have something like that in git.git.
> >
> > So am I. Except that I am not being sarcastic. ;-)
>
> Good idea. Ask somebody to put up some advertisement _against_ her own
> product into their shop window.
>
> Very good idea.
Actually works fairly well in many cases. But it is your shop, not mine.
Do what you like.
> Ciao,
> Dscho
>
> P.S.: Can I ask you to shoot yourself in the foot?
Sure, go ahead.
> P.P.S.: If you are really a Git hater, why are you lurking here? Go away,
> Subversion is not that bad.
My friend, you are shooting yourself in the foot.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-19 5:54 UTC (permalink / raw)
To: Jay Soffian
Cc: git, Junio C Hamano, Johannes Schindelin, Linus Torvalds, Alan
In-Reply-To: <76718490812181955u5f56180en47b3a8268c3538bb@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
On Thursday 2008 December 18 21:55:13 Jay Soffian wrote:
> On Thu, Dec 18, 2008 at 10:29 PM, Boyd Stephen Smith Jr.
>
> <bss@iguanasuicide.net> wrote:
> > At least, it might make someone read the manpage again. Still, I'm
> > unhappy with the message, but I didn't want to be too wordy. A URL or
> > manpage reference would be nice, but I didn't know of a good guide that
> > explained the dangers of reverting a merge commit as well as Linus's
> > emails.
>
> Put his email in Documentation/howto/undoing-merge-commits.txt and
> reference that?
Okay, I've got a documentation patch brewing, but it's too late here to work
on it more. I'll post it over the weekend.
In addition, I think a one-time-per-user warning would be nice, but I'm not
sure the best way to implement that. My initial thoughts would be reading a
boolean config option, if unset/true issuing the warning and then if unset
set it to false. However, that seems a bit... unclean and I fear there might
be a policy against writing ~/.gitconfig configuration options from a
subcommand other than 'git config'. Any suggestions on the implementation?
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Simplified GIT usage guide
From: Willy Tarreau @ 2008-12-19 6:33 UTC (permalink / raw)
To: sverre; +Cc: David Howells, Jakub Narebski, torvalds, git, linux-kernel
In-Reply-To: <bd6139dc0812121716w73ea1145w7f870e887e00adc0@mail.gmail.com>
On Sat, Dec 13, 2008 at 02:16:34AM +0100, Sverre Rabbelier wrote:
> On Sat, Dec 13, 2008 at 02:04, David Howells <dhowells@redhat.com> wrote:
> > (3) You put some non-basic stuff in the basic section (branching - this isn't
> > ordinarily useful, IMHO), but you miss other stuff out ('git rm' for
> > example).
>
> Erm, branching is not ordinarily useful? I think you're Doing It Wrong
> (TM) then, since branching is a Big Thing (also TM) in DVC, not using
> branches would be a bit like only using the first 4 gears in a car;
> sure, it's possible, but you're missing all that extra power!
People who want to use it as a CVS replacement don't realize that they're
using branches. They work in their local "master" branch, and don't realize
that when they fetch updates, they fetch them into a different branch.
CVS people are afraid of branches, so trying to explain them how they can
do what they're used to is better than telling them they'll have to do
what they're afraid of.
I found David's howto quite understandable. I've taught Git to people who
only knew about SVN and to people who had never heard about any SCM at all. I
tried to use the same approach each time, and while I noticed that explaining
how Git works and why it works like that appeared obvious to the newbies,
it was very awkward with SVN users. I switched to something more like
David's approach for those people and it helped a lot. They have plenty
of time after that to discover the tool by themselves.
I really think that David should maintain his doc after applying a few
fixes to it, just like Jeff maintains his own. Also, having several docs
out of the tree is better for a user looking for different analysis of
the tool than having everything offered as the product's documentation,
provided the links are easy to find (might be linked to from the Git doc).
Willy
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-19 6:35 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.
Cc: Jay Soffian, git, Johannes Schindelin, Linus Torvalds, Alan
In-Reply-To: <200812182354.16269.bss@iguanasuicide.net>
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> In addition, I think a one-time-per-user warning would be nice, but I'm not
> sure the best way to implement that. My initial thoughts would be reading a
> boolean config option, if unset/true issuing the warning and then if unset
> set it to false. However, that seems a bit... unclean and I fear there might
> be a policy against writing ~/.gitconfig configuration options from a
> subcommand other than 'git config'. Any suggestions on the implementation?
As an end user, I find one-time-per-user warning more frustrating than it
is worth. I may see the warning issued for the first time of my using
certain feature, and because I am so novice to the program suite that I do
not fully understand what the warning is trying to say when I see it.
Thanks to the "one-time-per-user"-ness, that is the only chance for me to
see the message --- which often means that I won't see the warning before
the gravity of it has any chance to really sink in my mind.
"You can set i-know-what-i-am-doing in your ~/.xyzzyconfig file to squelch
this message" is slightly better, as (1) I can control when I stop seeing
it, and (2) because setting that in my config is done by me, as opposed to
the tool doing behind my back, it is much more likely for me to recall how
to get the warning back when I choose to see it again.
The above discussion is "in general". In this particular case, I am not
convinced if the warning itself is worth it, though.
^ permalink raw reply
* Re: [PATCH] git-send-email: handle email address with quoted comma
From: Matt Kraai @ 2008-12-19 6:38 UTC (permalink / raw)
To: Wu Fengguang, git
Howdy,
> +sub split_addrs($) {
> + my ($addrs) = @_;
> +
> + return "ewords('\s*,\s*', 1, $addrs);
> +}
> +
According to the documentation of Text::ParseWords,
The &*quotewords() functions simply call &parse_line(), so if you're
only splitting one line you can call &parse_line() directly and save
a function call.
so quotewords could be replaced by parse_line. I don't know if that's
less readable, though.
--
Matt http://ftbfs.org/
^ permalink raw reply
* Re: [PATCH] git-send-email: handle email address with quoted comma
From: Junio C Hamano @ 2008-12-19 6:40 UTC (permalink / raw)
To: Wu Fengguang; +Cc: git
In-Reply-To: <1229658012-9240-1-git-send-email-fengguang.wu@intel.com>
Wu Fengguang <fengguang.wu@intel.com> writes:
> Correctly handle email addresses containing quoted commas, e.g.
>
> "Zhu, Yi" <yi.zhu@intel.com>, "Li, Shaohua" <shaohua.li@intel.com>
>
> Here the commas inside the double quotes are NOT email separators.
Thanks.
> @@ -359,6 +360,12 @@ foreach my $entry (@bcclist) {
> die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
> }
>
> +sub split_addrs($) {
> + my ($addrs) = @_;
> +
> + return "ewords('\s*,\s*', 1, $addrs);
> +}
> +
Does it add real value (e.g. type safety, simplified interface to the
caller, etc.) to force scalar context to the callers? It has been my
experience that use of prototypes (aka "parameter context templates") in
Perl programs tend to make the code less readable and more error prone in
longer term. I would further say that, even though you do not have any
existing caller of split_addrs sub that uses it for more than two values,
not using the prototype would be a better way to write this sub in this
particular case, because it would allow callers to say [*1*]:
@addrs = split_addr(@list_of_addr_lines);
It also is a bit funny-looking to invoke &function() (it is Perl4 style,
isn't it?)
IOW, wouldn't this be a better alternative?
sub split_addrs {
return quotewords('\s*,\s*', 1, @_);
}
[Footnote]
*1* This program demonstrates why use of prototype in this case is more
confusing than it is worth.
-- >8 --
#!/usr/bin/perl -w
use Text::ParseWords;
sub foo ($) { my ($addrs) = @_; return quotewords('\s*,\s*', 1, $addrs); }
sub bar { return quotewords('\s*,\s*', 1, @_); }
my @addrs = ('Frotz, "Xyzzy, Zork", Nitfol', 'Yomin, Rezrov');
my @addr = ($addrs[0]);
for (foo($addrs[0])) {
print "foo(\$addrs[0]) <<$_>>\n";
}
for (foo(@addr)) {
print "foo(\@addr) <<$_>>\n";
}
for (bar($addrs[0])) {
print "bar(\$addrs[0]) <<$_>>\n";
}
for (bar(@addr)) {
print "bar(\@addr) <<$_>>\n";
}
-- 8< --
The output from the above (the fourth one is the most interesting) looks
like this.
foo($addrs[0]) <<Frotz>>
foo($addrs[0]) <<"Xyzzy, Zork">>
foo($addrs[0]) <<Nitfol>>
foo(@addr) <<1>>
bar($addrs[0]) <<Frotz>>
bar($addrs[0]) <<"Xyzzy, Zork">>
bar($addrs[0]) <<Nitfol>>
bar(@addr) <<Frotz>>
bar(@addr) <<"Xyzzy, Zork">>
bar(@addr) <<Nitfol>>
*2* A more detailed discussion on Perl's "prototypes" is found here:
http://web.archive.org/web/20080210085941/http://library.n0i.net/programming/perl/articles/fm_prototypes/
^ 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