* Re: [PATCH] 3-way merge with file move fails when diff.renames = copies
From: Junio C Hamano @ 2008-11-11 0:15 UTC (permalink / raw)
To: David D. Kilzer; +Cc: git, Johannes Schindelin
In-Reply-To: <7v63mv3zww.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> "David D. Kilzer" <ddkilzer@kilzer.net> writes:
>
>> With diff.renames = copies, a 3-way merge (e.g. "git rebase") would
>> fail with the following error:
By the way, I think the real issue with this one is that we currently do
not disable diff.renames configuration while rebase internally runs
"format-patch" to feed "am -3".
The end user configuration for "diff" should not affect the result
produced by the higher level command that is related to "diff" only
because internally it is implemented in terms of it.
For that matter, I have a feeling that format-patch should not even look
at diff.renames, but we seem to have been doing this for a long time so
there is no easy way to fix this thinko.
In any case, here is a much straightforward fix for "rebase".
Running "am -3" on a copying patch would still need a patch to the
index-info codepath, and my earlier comment on it still stands, but it is
irrelevant/orthogonal to your particular test script.
git-rebase.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git c/git-rebase.sh w/git-rebase.sh
index 023a6dc..159ccb3 100755
--- c/git-rebase.sh
+++ w/git-rebase.sh
@@ -429,7 +429,7 @@ fi
if test -z "$do_merge"
then
git format-patch -k --stdout --full-index --ignore-if-in-upstream \
- "$upstream..$orig_head" |
+ --no-renames "$upstream..$orig_head" |
git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
move_to_original_branch
ret=$?
^ permalink raw reply related
* Re: [PATCH] git-diff: Add --staged as a synonym for --cached.
From: Jeff King @ 2008-11-11 0:15 UTC (permalink / raw)
To: David Symonds
Cc: Junio C Hamano, Björn Steinbrink, Johannes Schindelin, git,
Stephan Beyer
In-Reply-To: <ee77f5c20811101537u6061e5b4w420e9692e0cefad3@mail.gmail.com>
On Tue, Nov 11, 2008 at 10:37:37AM +1100, David Symonds wrote:
> > And I am not proposing a change here (except to perhaps "git diff
> > --staged" instead of "--cached"). Just pointing out that it does not
> > follow the "--staged operates on both, --staged-only operates on just
> > the index" rule.
>
> So apart from the wider discussion, I think this patch by itself is a
> nice step forward towards improving the UI of this part of git. Is
> there any further discussion on this one alone?
I think --staged for --cached is definitely an improvement. Even if we
don't have a set rule for "this is when you use --staged, and this is
when you use --staged-only", I think everyone so far agrees that any
such rule will have to incorporate "diff --staged" somehow, since it
reads pretty clearly.
So yes, I would like to see this applied. However, there are two
possible improvements:
1. Your documentation update says --staged is a synonym. Should
--staged perhaps be the "new" way of doing this, and --cached is
left as a historical alias? IOW, point users at the name we think
is more sensible.
2. Part of the point of this is to avoid using the world "cached" in
instructional materials. The tutorial and user manual should
probably be updated to use --staged (and it is reasonably safe to
do so immediately, since they are tied to the installed git
version; I hope Scott will update his materials eventually, but
lagging makes sense there as people will still be using existing
versions for some time).
And of course, such updates would be a small part of any larger
decision to call the index something else (the "stage" or
whatever) in those materials, but I think your point is to do this
one positive thing and not wait for the other bits.
-Peff
^ permalink raw reply
* Re: [PATCH] 3-way merge with file move fails when diff.renames = copies
From: David D. Kilzer @ 2008-11-11 0:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7v63mv3zww.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> If new_name that is not related at all to old_name happens to exist in the
> current tree you are applying the patch to, you can grab the contents of
> the unrelated file as the preimage and try to merge the changes in.
>
> When running --index-info for the purpose of "am -3" (hence rebase), the
> expectation is that the tree you are applying the changes to is _similar_
> to the preimage of the change, i.e. old_name. Shouldn't missing old_name
> be treated as a fatal condition? new_name does not have to even exist
> because otherwise you cannot accept a patch that creates the path.
>
> Wouldn't this be a better patch, I wonder...
>
> builtin-apply.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git i/builtin-apply.c w/builtin-apply.c
> index 4c4d1e1..7de70e9 100644
> --- i/builtin-apply.c
> +++ w/builtin-apply.c
> @@ -2573,8 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const
> char *filename)
> else if (get_sha1(patch->old_sha1_prefix, sha1))
> /* git diff has no index line for mode/type changes */
> if (!patch->lines_added && !patch->lines_deleted) {
> - if (get_current_sha1(patch->new_name, sha1) ||
> - get_current_sha1(patch->old_name, sha1))
> + if (get_current_sha1(patch->old_name, sha1))
> die("mode change for %s, which is not "
> "in current HEAD", name);
> sha1_ptr = sha1;
Yes, this change makes more sense to me, but I'll defer to Johannes' opinion before submitting another patch.
Dave
^ permalink raw reply
* Re: git commit -v does not removes the patch
From: Jeff King @ 2008-11-11 0:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <7vej1j40x5.fsf@gitster.siamese.dyndns.org>
On Mon, Nov 10, 2008 at 03:27:18PM -0800, Junio C Hamano wrote:
> > It is exactly as you described. I'll try in other systems.
>
> Guess in the dark... by any chance are you enabling color unconditionally?
I thought I covered that case explicitly...
...yes, I can't reproduce with color set to "always". And indeed,
looking at the diff for 4f672ad shows that we always turn off color for
the message that goes to the editor (since we can't trust our isatty
setting _anyway_, since we're not going to the tty).
So that's not it.
-Peff
^ permalink raw reply
* Re: overly smart rebase - bug or feature?
From: Junio C Hamano @ 2008-11-10 23:57 UTC (permalink / raw)
To: Roman.Shaposhnick; +Cc: git
In-Reply-To: <20081110233649.GI6799@sun.com>
Fedor Sergeev <Fedor.Sergeev@Sun.COM> writes:
> On Mon, Nov 10, 2008 at 03:14:42PM -0800, Junio C Hamano wrote:
>> Fedor Sergeev <Fedor.Sergeev@Sun.COM> writes:
>>
>> > I have recently hit a behavior which might well be a feature,
>> > but it was very surprising (in a bad sense) to me.
>>
>> It is a feature misfiring.
>>
>> Rebase is essentially a repeated cherry-pick, and a cherry-pick of commit
>
> But cherry-pick does fail, as shown in my original mail!
>
>> A on top of commit B is done by a simplified 3-way merge between A and B
>> using the parent of A as the common ancestor.
>>
>> A A'
>> / /
>> A^... pseudo history ...---B
>
> Well, my history is exactly that, not pseudo (and I dont quite follow your reasoning
> yet to understand whether this is important or not):
>
> A B
> \ /
> A^
>
> A^ *is* a common ancestor of both A and B.
>
>>
>> When your history has renamed Makefile to Makefile2 (thereby losing
>> Makefile)
>
> My history did not rename Makefile.
> There were three identical Makefiles (in A^)
> After that one was deleted (in B).
> On alternative branch it was edited (in A).
>
> If I do *merge* A into B then it fails.
> If I do *cherry-pick* A into B then it fails.
> If I do *rebase* A onto B then it succeeds.
>
>> while transition from A^ to A modified Makefile, the difference
>> between A^ to A that is applied to B to produce A' contains only the
>> change about Makefile (and does not talk about the unchangedness of
>> Makefile1 nor Makefile2 --- in fact, when A' is created, the machinery
>> does not even know if A^ and A had Makefile1 or Makefile2).
>>
>> When applying the change to Makefile, it notices that B does not have
>> Makefile, but there is a path that is _identical_ to the preimage your
>> change applies to (namely, Makefile2). To support people who rename
>> Makefile to Makefile2 in the history that led to B
>
> There was no rename. There was a copy in initial commit (and you cant say if it
> was Makefile copied into Makefile2 or vice verse).
> I dont believe it should really be called "rename", even if one of the copies was killed later.
>
>>, rebase (actually the
>> underlying "am -3" it calls is where this rename detection smart lies)
>> applies the changes to the "renamed" path.
>
> In this given case both Makefile1 and Makefile2 were absolutely equal.
> If rebase chose to edit Makefile2 why didnt it change Makefile1?
>
>>
>> You might be able to work this around by forcing rebase not to use the
>> simplified 3-way merge, by saying "rebase -m".
>
> Yeah, it worked.
> ...
> CONFLICT (delete/modify): Makefile deleted in master and modified in HEAD~0. Version HEAD~0 of Makefile left in tree.
> ...
>
> Though it does make me wonder why *simplified* 3-way merge is smarter than git merge ;)))
Simplified one is not _smarter_. It is merely _faster_, exactly because
it only looks at the paths between A^..A and nothing else.
And that is why it cannot tell between the case where both A^ and A have
Makefile2 or they both lack it. And that is exactly why application of
this change on top of B is mistaken as a patch to a renamed path. From
B's point of view:
- Incoming change says "I changed Makefile from this shape to that
shape", and nothing else;
- B does not have Makefile, but it happens to have the contents at path
Makefile2 that is identical to the preimage of that incoming change.
which makes it guess (when falling back to 3-way merge) that somewhere
leading to B what used to be at Makefile (which is what the incoming
change claims to change) may have been renamed to Makefile2 (because B
does not have Makefile but does have it).
^ permalink raw reply
* [PATCH 4/4] git send-email: ask less questions when --compose is used.
From: Pierre Habouzit @ 2008-11-10 23:54 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1226361242-2516-4-git-send-email-madcoder@debian.org>
When --compose is used, we can grab the From/Subject/In-Reply-To from the
edited summary, let it be so and don't ask the user silly questions.
The summary templates gets quite revamped, and includes the list of
patches subjects that are going to be sent with this batch.
When having a body full of empty lines, the summary isn't sent. Document
that in the git-send-email manpage fully.
Note: It doesn't deal with To/Cc/Bcc yet.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-send-email.txt | 9 ++
git-send-email.perl | 187 +++++++++++++++++++++++---------------
2 files changed, 123 insertions(+), 73 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 66d5f4c..acf8bf4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -45,6 +45,15 @@ The --cc option must be repeated for each user you want on the cc list.
--compose::
Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
introductory message for the patch series.
++
+When compose is in used, git send-email gets less interactive will use the
+values of the headers you set there. If the body of the email (what you type
+after the headers and a blank line) only contains blank (or GIT: prefixed)
+lines, the summary won't be sent, but git-send-email will still use the
+Headers values if you don't removed them.
++
+If it wasn't able to see a header in the summary it will ask you about it
+interactively after quitting your editor.
--from::
Specify the sender of the emails. This will default to
diff --git a/git-send-email.perl b/git-send-email.perl
index ccb3b18..9039cfd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -162,9 +162,17 @@ my $multiedit;
my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
sub do_edit {
if (defined($multiedit) && !$multiedit) {
- map { system('sh', '-c', $editor.' "$@"', $editor, $_); } @_;
+ map {
+ system('sh', '-c', $editor.' "$@"', $editor, $_);
+ if (($? & 127) || ($? >> 8)) {
+ die("the editor exited uncleanly, aborting everything");
+ }
+ } @_;
} else {
system('sh', '-c', $editor.' "$@"', $editor, @_);
+ if (($? & 127) || ($? >> 8)) {
+ die("the editor exited uncleanly, aborting everything");
+ }
}
}
@@ -450,6 +458,108 @@ if (@files) {
usage();
}
+sub get_patch_subject($) {
+ my $fn = shift;
+ open (my $fh, '<', $fn);
+ while (my $line = <$fh>) {
+ next unless ($line =~ /^Subject: (.*)$/);
+ close $fh;
+ return "GIT: $1\n";
+ }
+ close $fh;
+ die "No subject line in $fn ?";
+}
+
+if ($compose) {
+ # Note that this does not need to be secure, but we will make a small
+ # effort to have it be unique
+ open(C,">",$compose_filename)
+ or die "Failed to open for writing $compose_filename: $!";
+
+
+ my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
+ my $tpl_subject = $initial_subject || '';
+ my $tpl_reply_to = $initial_reply_to || '';
+
+ print C <<EOT;
+From $tpl_sender # This line is ignored.
+GIT: Lines beginning in "GIT: " will be removed.
+GIT: Consider including an overall diffstat or table of contents
+GIT: for the patch you are writing.
+GIT:
+GIT: Clear the body content if you don't wish to send a summary.
+From: $tpl_sender
+Subject: $tpl_subject
+In-Reply-To: $tpl_reply_to
+
+EOT
+ for my $f (@files) {
+ print C get_patch_subject($f);
+ }
+ close(C);
+
+ my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+
+ if ($annotate) {
+ do_edit($compose_filename, @files);
+ } else {
+ do_edit($compose_filename);
+ }
+
+ open(C2,">",$compose_filename . ".final")
+ or die "Failed to open $compose_filename.final : " . $!;
+
+ open(C,"<",$compose_filename)
+ or die "Failed to open $compose_filename : " . $!;
+
+ my $need_8bit_cte = file_has_nonascii($compose_filename);
+ my $in_body = 0;
+ my $summary_empty = 1;
+ while(<C>) {
+ next if m/^GIT: /;
+ if ($in_body) {
+ $summary_empty = 0 unless (/^\n$/);
+ } elsif (/^\n$/) {
+ $in_body = 1;
+ if ($need_8bit_cte) {
+ print C2 "MIME-Version: 1.0\n",
+ "Content-Type: text/plain; ",
+ "charset=utf-8\n",
+ "Content-Transfer-Encoding: 8bit\n";
+ }
+ } elsif (/^MIME-Version:/i) {
+ $need_8bit_cte = 0;
+ } elsif (/^Subject:\s*(.+)\s*$/i) {
+ $initial_subject = $1;
+ my $subject = $initial_subject;
+ $_ = "Subject: " .
+ ($subject =~ /[^[:ascii:]]/ ?
+ quote_rfc2047($subject) :
+ $subject) .
+ "\n";
+ } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
+ $initial_reply_to = $1;
+ next;
+ } elsif (/^From:\s*(.+)\s*$/i) {
+ $sender = $1;
+ next;
+ } elsif (/^(?:To|Cc|Bcc):/i) {
+ print "To/Cc/Bcc fields are not interpreted yet, they have been ignored\n";
+ next;
+ }
+ print C2 $_;
+ }
+ close(C);
+ close(C2);
+
+ if ($summary_empty) {
+ print "Summary email is empty, skipping it\n";
+ $compose = -1;
+ }
+} elsif ($annotate) {
+ do_edit(@files);
+}
+
my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
@@ -494,17 +604,6 @@ sub expand_aliases {
@initial_cc = expand_aliases(@initial_cc);
@bcclist = expand_aliases(@bcclist);
-if (!defined $initial_subject && $compose) {
- while (1) {
- $_ = $term->readline("What subject should the initial email start with? ", $initial_subject);
- last if defined $_;
- print "\n";
- }
-
- $initial_subject = $_;
- $prompting++;
-}
-
if ($thread && !defined $initial_reply_to && $prompting) {
while (1) {
$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ", $initial_reply_to);
@@ -531,64 +630,6 @@ if (!defined $smtp_server) {
}
if ($compose) {
- # Note that this does not need to be secure, but we will make a small
- # effort to have it be unique
- open(C,">",$compose_filename)
- or die "Failed to open for writing $compose_filename: $!";
- print C "From $sender # This line is ignored.\n";
- printf C "Subject: %s\n\n", $initial_subject;
- printf C <<EOT;
-GIT: Please enter your email below.
-GIT: Lines beginning in "GIT: " will be removed.
-GIT: Consider including an overall diffstat or table of contents
-GIT: for the patch you are writing.
-
-EOT
- close(C);
-
- my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
-
- if ($annotate) {
- do_edit($compose_filename, @files);
- } else {
- do_edit($compose_filename);
- }
-
- open(C2,">",$compose_filename . ".final")
- or die "Failed to open $compose_filename.final : " . $!;
-
- open(C,"<",$compose_filename)
- or die "Failed to open $compose_filename : " . $!;
-
- my $need_8bit_cte = file_has_nonascii($compose_filename);
- my $in_body = 0;
- while(<C>) {
- next if m/^GIT: /;
- if (!$in_body && /^\n$/) {
- $in_body = 1;
- if ($need_8bit_cte) {
- print C2 "MIME-Version: 1.0\n",
- "Content-Type: text/plain; ",
- "charset=utf-8\n",
- "Content-Transfer-Encoding: 8bit\n";
- }
- }
- if (!$in_body && /^MIME-Version:/i) {
- $need_8bit_cte = 0;
- }
- if (!$in_body && /^Subject: ?(.*)/i) {
- my $subject = $1;
- $_ = "Subject: " .
- ($subject =~ /[^[:ascii:]]/ ?
- quote_rfc2047($subject) :
- $subject) .
- "\n";
- }
- print C2 $_;
- }
- close(C);
- close(C2);
-
while (1) {
$_ = $term->readline("Send this email? (y|n) ");
last if defined $_;
@@ -600,9 +641,9 @@ EOT
exit(0);
}
- @files = ($compose_filename . ".final", @files);
-} elsif ($annotate) {
- do_edit(@files);
+ if ($compose > 0) {
+ @files = ($compose_filename . ".final", @files);
+ }
}
# Variables we set as part of the loop over files
--
1.6.0.4.859.g7ecd.dirty
^ permalink raw reply related
* [PATCH 3/4] git send-email: add --annotate option
From: Pierre Habouzit @ 2008-11-10 23:54 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1226361242-2516-3-git-send-email-madcoder@debian.org>
This allows to review every patch (and fix various aspects of them, or
comment them) in an editor just before being sent. Combined to the fact
that git send-email can now process revision lists, this makes git
send-email and efficient way to review and send patches interactively.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-send-email.txt | 11 +++++++++++
git-send-email.perl | 26 ++++++++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0beaad4..66d5f4c 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -37,6 +37,11 @@ The --bcc option must be repeated for each user you want on the bcc list.
+
The --cc option must be repeated for each user you want on the cc list.
+--annotate::
+ Review each patch you're about to send in an editor. The setting
+ 'sendemail.multiedit' defines if this will spawn one editor per patch
+ or one for all of them at once.
+
--compose::
Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
introductory message for the patch series.
@@ -210,6 +215,12 @@ sendemail.aliasfiletype::
Format of the file(s) specified in sendemail.aliasesfile. Must be
one of 'mutt', 'mailrc', 'pine', or 'gnus'.
+sendemail.multiedit::
+ If true (default), a single editor instance will be spawned to edit
+ files you have to edit (patches when '--annotate' is used, and the
+ summary when '--compose' is used). If false, files will be edited one
+ after the other, spawning a new editor each time.
+
Author
------
diff --git a/git-send-email.perl b/git-send-email.perl
index 6f5a613..ccb3b18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -51,6 +51,7 @@ git send-email [options] <file | directory | rev-list options >
--bcc <str> * Email Bcc:
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
+ --annotate * Review each patch that will be sent in an editor.
--compose * Open an editor for introduction.
Sending:
@@ -132,7 +133,8 @@ sub cleanup_compose_files();
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@initial_cc,@bcclist,@xh,
- $initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
+ $initial_reply_to,$initial_subject,@files,
+ $author,$sender,$smtp_authpass,$annotate,$compose,$time);
my $envelope_sender;
@@ -155,6 +157,17 @@ my ($quiet, $dry_run) = (0, 0);
my $format_patch;
my $compose_filename = $repo->repo_path() . "/.gitsendemail.msg.$$";
+# Handle interactive edition of files.
+my $multiedit;
+my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+sub do_edit {
+ if (defined($multiedit) && !$multiedit) {
+ map { system('sh', '-c', $editor.' "$@"', $editor, $_); } @_;
+ } else {
+ system('sh', '-c', $editor.' "$@"', $editor, @_);
+ }
+}
+
# Variables with corresponding config settings
my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
@@ -184,6 +197,7 @@ my %config_settings = (
"aliasesfile" => \@alias_files,
"suppresscc" => \@suppress_cc,
"envelopesender" => \$envelope_sender,
+ "multiedit" => \$multiedit,
);
# Handle Uncouth Termination
@@ -226,6 +240,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"smtp-ssl" => sub { $smtp_encryption = 'ssl' },
"smtp-encryption=s" => \$smtp_encryption,
"identity=s" => \$identity,
+ "annotate" => \$annotate,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
@@ -532,7 +547,12 @@ EOT
close(C);
my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
- system('sh', '-c', $editor.' "$@"', $editor, $compose_filename);
+
+ if ($annotate) {
+ do_edit($compose_filename, @files);
+ } else {
+ do_edit($compose_filename);
+ }
open(C2,">",$compose_filename . ".final")
or die "Failed to open $compose_filename.final : " . $!;
@@ -581,6 +601,8 @@ EOT
}
@files = ($compose_filename . ".final", @files);
+} elsif ($annotate) {
+ do_edit(@files);
}
# Variables we set as part of the loop over files
--
1.6.0.4.859.g7ecd.dirty
^ permalink raw reply related
* [PATCH 2/4] git send-email: interpret unknown files as revision lists
From: Pierre Habouzit @ 2008-11-10 23:54 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1226361242-2516-2-git-send-email-madcoder@debian.org>
Filter out all the arguments git-send-email doesn't like to a
git format-patch command, that dumps its content to a safe directory.
Barf when a file/revision conflict occurs, allow it to be overriden
--[no-]format-patch.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-send-email.txt | 8 +++++-
git-send-email.perl | 47 +++++++++++++++++++++++++++++++++----
t/t9001-send-email.sh | 8 ++++++
3 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 82f5056..0beaad4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -8,7 +8,7 @@ git-send-email - Send a collection of patches as emails
SYNOPSIS
--------
-'git send-email' [options] <file|directory> [... file|directory]
+'git send-email' [options] <file|directory|rev-list options>...
DESCRIPTION
@@ -183,6 +183,12 @@ Administering
--[no-]validate::
Perform sanity checks on patches.
Currently, validation means the following:
+
+--[no-]format-patch::
+ When an argument may be understood either as a reference or as a file name,
+ choose to understand it as a format-patch argument ('--format-patch')
+ or as a file name ('--no-format-patch'). By default, when such a conflict
+ occurs, git send-email will fail.
+
--
* Warn of patches that contain lines longer than 998 characters; this
diff --git a/git-send-email.perl b/git-send-email.perl
index aaace02..6f5a613 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -22,8 +22,12 @@ use Term::ReadLine;
use Getopt::Long;
use Data::Dumper;
use Term::ANSIColor;
+use File::Temp qw/ tempdir /;
+use Error qw(:try);
use Git;
+Getopt::Long::Configure qw/ pass_through /;
+
package FakeTerm;
sub new {
my ($class, $reason) = @_;
@@ -38,7 +42,7 @@ package main;
sub usage {
print <<EOT;
-git send-email [options] <file | directory>...
+git send-email [options] <file | directory | rev-list options >
Composing:
--from <str> * Email From:
@@ -73,6 +77,8 @@ git send-email [options] <file | directory>...
--quiet * Output one line of info per email.
--dry-run * Don't actually send the emails.
--[no-]validate * Perform patch sanity checks. Default on.
+ --[no-]format-patch * understand any non optional arguments as
+ `git format-patch` ones.
EOT
exit(1);
@@ -146,6 +152,7 @@ if ($@) {
# Behavior modification variables
my ($quiet, $dry_run) = (0, 0);
+my $format_patch;
my $compose_filename = $repo->repo_path() . "/.gitsendemail.msg.$$";
# Variables with corresponding config settings
@@ -229,6 +236,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"envelope-sender=s" => \$envelope_sender,
"thread!" => \$thread,
"validate!" => \$validate,
+ "format-patch!" => \$format_patch,
);
unless ($rc) {
@@ -363,23 +371,52 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
($sender) = expand_aliases($sender) if defined $sender;
+# returns 1 if the conflict must be solved using it as a format-patch argument
+sub check_file_rev_conflict($) {
+ my $f = shift;
+ try {
+ $repo->command('rev-parse', '--verify', '--quiet', $f);
+ if (defined($format_patch)) {
+ print "foo\n";
+ return $format_patch;
+ }
+ die(<<EOF);
+File '$f' exists but it could also be the range of commits
+to produce patches for. Please disambiguate by...
+
+ * Saying "./$f" if you mean a file; or
+ * Giving --format-patch option if you mean a range.
+EOF
+ } catch Git::Error::Command with {
+ return 0;
+ }
+}
+
# Now that all the defaults are set, process the rest of the command line
# arguments and collect up the files that need to be processed.
-for my $f (@ARGV) {
- if (-d $f) {
+my @rev_list_opts;
+while (my $f = pop @ARGV) {
+ if ($f eq "--") {
+ push @rev_list_opts, "--", @ARGV;
+ @ARGV = ();
+ } elsif (-d $f and !check_file_rev_conflict($f)) {
opendir(DH,$f)
or die "Failed to opendir $f: $!";
push @files, grep { -f $_ } map { +$f . "/" . $_ }
sort readdir(DH);
closedir(DH);
- } elsif (-f $f or -p $f) {
+ } elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
push @files, $f;
} else {
- print STDERR "Skipping $f - not found.\n";
+ push @rev_list_opts, $f;
}
}
+if (@rev_list_opts) {
+ push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);
+}
+
if ($validate) {
foreach my $f (@files) {
unless (-p $f) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 561ae7d..b4bddd1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -292,4 +292,12 @@ test_expect_success '--compose adds MIME for utf8 subject' '
grep "^Subject: =?utf-8?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1
'
+test_expect_success 'detects ambiguous reference/file conflict' '
+ echo master > master &&
+ git add master &&
+ git commit -m"add master" &&
+ test_must_fail git send-email --dry-run master > errors &&
+ grep disambiguate errors
+'
+
test_done
--
1.6.0.4.859.g7ecd.dirty
^ permalink raw reply related
* [PATCH 1/4] git send-email: make the message file name more specific.
From: Pierre Habouzit @ 2008-11-10 23:53 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1226361242-2516-1-git-send-email-madcoder@debian.org>
This helps editors choosing their syntax hilighting properly.
Also make the file live under the git directory.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-send-email.perl | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 94ca5c8..aaace02 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -124,9 +124,6 @@ my $auth;
sub unique_email_list(@);
sub cleanup_compose_files();
-# Constants (essentially)
-my $compose_filename = ".msg.$$";
-
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@initial_cc,@bcclist,@xh,
$initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
@@ -149,6 +146,7 @@ if ($@) {
# Behavior modification variables
my ($quiet, $dry_run) = (0, 0);
+my $compose_filename = $repo->repo_path() . "/.gitsendemail.msg.$$";
# Variables with corresponding config settings
my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
--
1.6.0.4.859.g7ecd.dirty
^ permalink raw reply related
* [take 2] git send-email updates
From: Pierre Habouzit @ 2008-11-10 23:53 UTC (permalink / raw)
To: git
In-Reply-To: <1225450632-7230-1-git-send-email-madcoder@debian.org>
The last patch is dropped for now (the automatic --compose stuff)
because I'm not sure which option to add, and that I don't care enough
about it to spend more time on it.
I think I've incorporated most of the stuff people asked about in this
series.
[PATCH 1/4] git send-email: make the message file name more specific.
[PATCH 2/4] git send-email: interpret unknown files as revision lists
[PATCH 3/4] git send-email: add --annotate option
[PATCH 4/4] git send-email: ask less questions when --compose is used.
^ permalink raw reply
* [PATCH] Fix 3-way merge with file move when diff.renames = copies
From: David D. Kilzer @ 2008-11-10 23:53 UTC (permalink / raw)
To: git; +Cc: David D. Kilzer, Johannes Schindelin, gitster
In-Reply-To: <alpine.DEB.1.00.0811110039280.30769@pacific.mpi-cbg.de>
With diff.renames = copies, a 3-way merge (e.g. "git rebase") with a
file move would fail with the following error:
fatal: mode change for <file>, which is not in current HEAD
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001.
The bug is a logic error added in ece7b749, which attempts to find
an sha1 for a patch with no index line in build_fake_ancestor().
Instead of failing unless an sha1 is found for both the old file and
the new file, a failure should only be reported if neither the old
file nor the new file is found.
Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
---
Same as "[PATCH] 3-way merge with file move fails when diff.renames = copies"
but with an updated subject and initial sentence (added "with a file move").
builtin-apply.c | 2 +-
t/t3400-rebase.sh | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 4c4d1e1..cfeb6cc 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2573,7 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
else if (get_sha1(patch->old_sha1_prefix, sha1))
/* git diff has no index line for mode/type changes */
if (!patch->lines_added && !patch->lines_deleted) {
- if (get_current_sha1(patch->new_name, sha1) ||
+ if (get_current_sha1(patch->new_name, sha1) &&
get_current_sha1(patch->old_name, sha1))
die("mode change for %s, which is not "
"in current HEAD", name);
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b7a670e..a156850 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -84,4 +84,21 @@ test_expect_success 'rebase a single mode change' '
GIT_TRACE=1 git rebase master
'
+test_expect_success 'rebase a single file move with diff.renames = copies' '
+ git config diff.renames copies &&
+ git checkout master &&
+ echo 1 > Y &&
+ git add Y &&
+ test_tick &&
+ git commit -m "prepare file move" &&
+ git checkout -b filemove HEAD^ &&
+ echo 1 > Y &&
+ git add Y &&
+ mkdir D &&
+ git mv A D/A &&
+ test_tick &&
+ git commit -m filemove &&
+ GIT_TRACE=1 git rebase master
+'
+
test_done
--
1.6.0
^ permalink raw reply related
* Re: [PATCH] 3-way merge with file move fails when diff.renames = copies
From: Junio C Hamano @ 2008-11-10 23:49 UTC (permalink / raw)
To: David D. Kilzer; +Cc: git, Johannes Schindelin
In-Reply-To: <1226355970-2542-1-git-send-email-ddkilzer@kilzer.net>
"David D. Kilzer" <ddkilzer@kilzer.net> writes:
> With diff.renames = copies, a 3-way merge (e.g. "git rebase") would
> fail with the following error:
>
> fatal: mode change for <file>, which is not in current HEAD
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001.
>
> The bug is a logic error added in ece7b749, which attempts to find
> an sha1 for a patch with no index line in build_fake_ancestor().
> Instead of failing unless an sha1 is found for both the old file and
> the new file, a failure should only be reported if neither the old
> file nor the new file is found.
>
> Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
> ---
> builtin-apply.c | 2 +-
> t/t3400-rebase.sh | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 4c4d1e1..cfeb6cc 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -2573,7 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
> else if (get_sha1(patch->old_sha1_prefix, sha1))
> /* git diff has no index line for mode/type changes */
> if (!patch->lines_added && !patch->lines_deleted) {
> - if (get_current_sha1(patch->new_name, sha1) ||
> + if (get_current_sha1(patch->new_name, sha1) &&
> get_current_sha1(patch->old_name, sha1))
> die("mode change for %s, which is not "
> "in current HEAD", name);
Hmm.
The logic introduced by the blamed commit makes the --index-info
unreliable (I'd rather see it fail reliably if it does not have enough
information rather than pretending everything is Ok), and I think the
patch makes it slightly more so.
If new_name that is not related at all to old_name happens to exist in the
current tree you are applying the patch to, you can grab the contents of
the unrelated file as the preimage and try to merge the changes in.
When running --index-info for the purpose of "am -3" (hence rebase), the
expectation is that the tree you are applying the changes to is _similar_
to the preimage of the change, i.e. old_name. Shouldn't missing old_name
be treated as a fatal condition? new_name does not have to even exist
because otherwise you cannot accept a patch that creates the path.
Wouldn't this be a better patch, I wonder...
builtin-apply.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git i/builtin-apply.c w/builtin-apply.c
index 4c4d1e1..7de70e9 100644
--- i/builtin-apply.c
+++ w/builtin-apply.c
@@ -2573,8 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
else if (get_sha1(patch->old_sha1_prefix, sha1))
/* git diff has no index line for mode/type changes */
if (!patch->lines_added && !patch->lines_deleted) {
- if (get_current_sha1(patch->new_name, sha1) ||
- get_current_sha1(patch->old_name, sha1))
+ if (get_current_sha1(patch->old_name, sha1))
die("mode change for %s, which is not "
"in current HEAD", name);
sha1_ptr = sha1;
^ permalink raw reply related
* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
From: Tuncer Ayaz @ 2008-11-10 23:43 UTC (permalink / raw)
To: git, Junio C Hamano
In-Reply-To: <1226028387-23867-1-git-send-email-tuncer.ayaz@gmail.com>
On Fri, Nov 7, 2008 at 4:26 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> Implement git-pull --quiet and git-pull --verbose by
> adding the options to git-pull and fixing verbosity
> handling in git-fetch.
Junio,
is there anything still missing in this patch?
Maybe the name of the test-case is bad.
Maybe you've queued it for review already.
Regards,
Tuncer Ayaz
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
> Documentation/merge-options.txt | 8 +++++
> builtin-fetch.c | 21 +++++++------
> builtin-merge.c | 22 ++++++++++----
> git-pull.sh | 10 ++++--
> t/t5521-pull-options.sh | 60 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 101 insertions(+), 20 deletions(-)
> create mode 100755 t/t5521-pull-options.sh
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 007909a..427cdef 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -1,3 +1,11 @@
> +-q::
> +--quiet::
> + Operate quietly.
> +
> +-v::
> +--verbose::
> + Be verbose.
> +
> --stat::
> Show a diffstat at the end of the merge. The diffstat is also
> controlled by the configuration option merge.stat.
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index f151cfa..efc5801 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -22,16 +22,17 @@ enum {
> TAGS_SET = 2
> };
>
> -static int append, force, keep, update_head_ok, verbose, quiet;
> +static int append, force, keep, update_head_ok;
> static int tags = TAGS_DEFAULT;
> static const char *depth;
> static const char *upload_pack;
> static struct strbuf default_rla = STRBUF_INIT;
> static struct transport *transport;
> +static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
>
> static struct option builtin_fetch_options[] = {
> - OPT__QUIET(&quiet),
> - OPT__VERBOSE(&verbose),
> + OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
> + OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
> OPT_BOOLEAN('a', "append", &append,
> "append to .git/FETCH_HEAD instead of overwriting"),
> OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
> @@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
>
> static int update_local_ref(struct ref *ref,
> const char *remote,
> - int verbose,
> char *display)
> {
> struct commit *current = NULL, *updated;
> @@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
> die("object %s not found", sha1_to_hex(ref->new_sha1));
>
> if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
> - if (verbose)
> + if (verbosity == VERBOSE)
> sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
> "[up to date]", REFCOL_WIDTH, remote,
> pretty_ref);
> @@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
> note);
>
> if (ref)
> - rc |= update_local_ref(ref, what, verbose, note);
> + rc |= update_local_ref(ref, what, note);
> else
> sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
> SUMMARY_WIDTH, *kind ? kind : "branch",
> REFCOL_WIDTH, *what ? what : "HEAD");
> if (*note) {
> - if (!shown_url) {
> + if (verbosity > QUIET && !shown_url) {
> fprintf(stderr, "From %.*s\n",
> url_len, url);
> shown_url = 1;
> }
> - fprintf(stderr, " %s\n", note);
> + if (verbosity > QUIET)
> + fprintf(stderr, " %s\n", note);
> }
> }
> fclose(fp);
> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> remote = remote_get(argv[0]);
>
> transport = transport_get(remote, remote->url[0]);
> - if (verbose >= 2)
> + if (verbosity == VERBOSE)
> transport->verbose = 1;
> - if (quiet)
> + if (verbosity == QUIET)
> transport->verbose = -1;
> if (upload_pack)
> set_option(TRANS_OPT_UPLOADPACK, upload_pack);
> diff --git a/builtin-merge.c b/builtin-merge.c
> index 5e7910b..76e2890 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
> static struct strategy **use_strategies;
> static size_t use_strategies_nr, use_strategies_alloc;
> static const char *branch;
> +static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
>
> static struct strategy all_strategy[] = {
> { "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },
> @@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
> }
>
> static struct option builtin_merge_options[] = {
> + OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
> + OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
> { OPTION_CALLBACK, 'n', NULL, NULL, NULL,
> "do not show a diffstat at the end of the merge",
> PARSE_OPT_NOARG, option_parse_n },
> @@ -250,7 +253,8 @@ static void restore_state(void)
> /* This is called when no merge was necessary. */
> static void finish_up_to_date(const char *msg)
> {
> - printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
> + if (verbosity > QUIET)
> + printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
> drop_save();
> }
>
> @@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
> if (!msg)
> strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
> else {
> - printf("%s\n", msg);
> + if (verbosity > QUIET)
> + printf("%s\n", msg);
> strbuf_addf(&reflog_message, "%s: %s",
> getenv("GIT_REFLOG_ACTION"), msg);
> }
> if (squash) {
> squash_message();
> } else {
> - if (!merge_msg.len)
> + if (verbosity > QUIET && !merge_msg.len)
> printf("No merge message -- not updating HEAD\n");
> else {
> const char *argv_gc_auto[] = { "gc", "--auto", NULL };
> @@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
> argc = parse_options(argc, argv, builtin_merge_options,
> builtin_merge_usage, 0);
> + if (verbosity > QUIET)
> + show_diffstat = 0;
>
> if (squash) {
> if (!allow_fast_forward)
> @@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
> strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
>
> - printf("Updating %s..%s\n",
> - hex,
> - find_unique_abbrev(remoteheads->item->object.sha1,
> - DEFAULT_ABBREV));
> + if (verbosity > QUIET)
> + printf("Updating %s..%s\n",
> + hex,
> + find_unique_abbrev(remoteheads->item->object.sha1,
> + DEFAULT_ABBREV));
> strbuf_addstr(&msg, "Fast forward");
> if (have_message)
> strbuf_addstr(&msg,
> diff --git a/git-pull.sh b/git-pull.sh
> index 664fe34..8866f2a 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -16,13 +16,17 @@ cd_to_toplevel
> test -z "$(git ls-files -u)" ||
> die "You are in the middle of a conflicted merge."
>
> -strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
> +strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
> curr_branch=$(git symbolic-ref -q HEAD)
> curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
> rebase=$(git config --bool branch.$curr_branch_short.rebase)
> while :
> do
> case "$1" in
> + -q|--quiet)
> + verbosity=-q ;;
> + -v|--verbose)
> + verbosity=-v ;;
> -n|--no-stat|--no-summary)
> no_stat=-n ;;
> --stat|--summary)
> @@ -121,7 +125,7 @@ test true = "$rebase" && {
> "refs/remotes/$origin/$reflist" 2>/dev/null)"
> }
> orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
> -git fetch --update-head-ok "$@" || exit 1
> +git fetch $verbosity --update-head-ok "$@" || exit 1
>
> curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
> if test -n "$orig_head" && test "$curr_head" != "$orig_head"
> @@ -182,4 +186,4 @@ test true = "$rebase" &&
> exec git-rebase $strategy_args --onto $merge_head \
> ${oldremoteref:-$merge_head}
> exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
> - "$merge_name" HEAD $merge_head
> + "$merge_name" HEAD $merge_head $verbosity
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> new file mode 100755
> index 0000000..83e2e8a
> --- /dev/null
> +++ b/t/t5521-pull-options.sh
> @@ -0,0 +1,60 @@
> +#!/bin/sh
> +
> +test_description='pull options'
> +
> +. ./test-lib.sh
> +
> +D=`pwd`
> +
> +test_expect_success 'setup' '
> + mkdir parent &&
> + (cd parent && git init &&
> + echo one >file && git add file &&
> + git commit -m one)
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -q' '
> + mkdir clonedq &&
> + cd clonedq &&
> + git pull -q "$D/parent" >out 2>err &&
> + test ! -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull' '
> + mkdir cloned &&
> + cd cloned &&
> + git pull "$D/parent" >out 2>err &&
> + test -s out
> +'
> +cd "$D"
> +
> +test_expect_success 'git pull -v' '
> + mkdir clonedv &&
> + cd clonedv &&
> + git pull -v "$D/parent" >out 2>err &&
> + test -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -v -q' '
> + mkdir clonedvq &&
> + cd clonedvq &&
> + git pull -v -q "$D/parent" >out 2>err &&
> + test ! -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -q -v' '
> + mkdir clonedqv &&
> + cd clonedqv &&
> + git pull -q -v "$D/parent" >out 2>err &&
> + test -s out
> +'
> +
> +test_done
> --
> 1.6.0.2.GIT
>
>
^ permalink raw reply
* Re: [PATCH] git-diff: Add --staged as a synonym for --cached.
From: David Symonds @ 2008-11-10 23:37 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Björn Steinbrink, Johannes Schindelin, git,
Stephan Beyer
In-Reply-To: <20081103071420.GD10772@coredump.intra.peff.net>
On Mon, Nov 3, 2008 at 6:14 PM, Jeff King <peff@peff.net> wrote:
> And I am not proposing a change here (except to perhaps "git diff
> --staged" instead of "--cached"). Just pointing out that it does not
> follow the "--staged operates on both, --staged-only operates on just
> the index" rule.
So apart from the wider discussion, I think this patch by itself is a
nice step forward towards improving the UI of this part of git. Is
there any further discussion on this one alone?
Dave.
^ permalink raw reply
* Re: overly smart rebase - bug or feature?
From: Fedor Sergeev @ 2008-11-10 23:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Roman.Shaposhnick, git
In-Reply-To: <7vod0n41i5.fsf@gitster.siamese.dyndns.org>
On Mon, Nov 10, 2008 at 03:14:42PM -0800, Junio C Hamano wrote:
> Fedor Sergeev <Fedor.Sergeev@Sun.COM> writes:
>
> > I have recently hit a behavior which might well be a feature,
> > but it was very surprising (in a bad sense) to me.
>
> It is a feature misfiring.
>
> Rebase is essentially a repeated cherry-pick, and a cherry-pick of commit
But cherry-pick does fail, as shown in my original mail!
> A on top of commit B is done by a simplified 3-way merge between A and B
> using the parent of A as the common ancestor.
>
> A A'
> / /
> A^... pseudo history ...---B
Well, my history is exactly that, not pseudo (and I dont quite follow your reasoning
yet to understand whether this is important or not):
A B
\ /
A^
A^ *is* a common ancestor of both A and B.
>
> When your history has renamed Makefile to Makefile2 (thereby losing
> Makefile)
My history did not rename Makefile.
There were three identical Makefiles (in A^)
After that one was deleted (in B).
On alternative branch it was edited (in A).
If I do *merge* A into B then it fails.
If I do *cherry-pick* A into B then it fails.
If I do *rebase* A onto B then it succeeds.
> while transition from A^ to A modified Makefile, the difference
> between A^ to A that is applied to B to produce A' contains only the
> change about Makefile (and does not talk about the unchangedness of
> Makefile1 nor Makefile2 --- in fact, when A' is created, the machinery
> does not even know if A^ and A had Makefile1 or Makefile2).
>
> When applying the change to Makefile, it notices that B does not have
> Makefile, but there is a path that is _identical_ to the preimage your
> change applies to (namely, Makefile2). To support people who rename
> Makefile to Makefile2 in the history that led to B
There was no rename. There was a copy in initial commit (and you cant say if it
was Makefile copied into Makefile2 or vice verse).
I dont believe it should really be called "rename", even if one of the copies was killed later.
>, rebase (actually the
> underlying "am -3" it calls is where this rename detection smart lies)
> applies the changes to the "renamed" path.
In this given case both Makefile1 and Makefile2 were absolutely equal.
If rebase chose to edit Makefile2 why didnt it change Makefile1?
>
> You might be able to work this around by forcing rebase not to use the
> simplified 3-way merge, by saying "rebase -m".
Yeah, it worked.
...
CONFLICT (delete/modify): Makefile deleted in master and modified in HEAD~0. Version HEAD~0 of Makefile left in tree.
...
Though it does make me wonder why *simplified* 3-way merge is smarter than git merge ;)))
best regards,
Fedor..
^ permalink raw reply
* Re: [PATCH] 3-way merge with file move fails when diff.renames = copies
From: Johannes Schindelin @ 2008-11-10 23:41 UTC (permalink / raw)
To: David D. Kilzer; +Cc: git, gitster
In-Reply-To: <1226355970-2542-1-git-send-email-ddkilzer@kilzer.net>
Hi,
except for the commit subject I like it, especially the commit body. So:
s/(3-way.*) fails/Fix &/
Ciao,
Dscho
^ permalink raw reply
* Re: overly smart rebase - bug or feature?
From: Avery Pennarun @ 2008-11-10 23:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fedor Sergeev, Roman.Shaposhnick, git
In-Reply-To: <7vod0n41i5.fsf@gitster.siamese.dyndns.org>
On Mon, Nov 10, 2008 at 6:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
> When applying the change to Makefile, it notices that B does not have
> Makefile, but there is a path that is _identical_ to the preimage your
> change applies to (namely, Makefile2). To support people who rename
> Makefile to Makefile2 in the history that led to B, rebase (actually the
> underlying "am -3" it calls is where this rename detection smart lies)
> applies the changes to the "renamed" path.
But isn't rename detection in this case rather suspicious, since:
- the preimage already had Makefile, Makefile1, and Makefile2, thus it
is not a rename, but at most a copy, and not even a newly-created copy
in either branch;
- *two* different files match the original Makefile, but rebase has
randomly selected one but not the other;
- (I haven't verified this claim) cherry-pick and merge both correctly
identify the problem as a delete/modify conflict?
It seems that rebase should have bailed out for at least one of these
three reasons.
Avery
^ permalink raw reply
* Re: JGIT: discuss: diff/patch implementation
From: Johannes Schindelin @ 2008-11-10 23:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Francis Galiegue, Git Mailing List, Shawn O. Pearce,
Robin Rosenberg
In-Reply-To: <7v63mv5mro.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 10 Nov 2008, Junio C Hamano wrote:
> Francis Galiegue <fg@one2team.net> writes:
>
> > A very nice git feature, without even going as far as merges, is the
> > cherry pick feature.
>
> I thought cherry-picking needs to be done in terms of 3-way merge, not
> diff piped to patch, for correctness's sake.
I haven't checked how RCS merge does it, but I know how xdiff/xmerge.c
does it ;-)
Basically, it takes the two diffs relative to the base file and works on
the overlapping hunks (i.e. on hunks where the ranges in the base file
overlap).
So we need a diff algorithm very much if we were to imitate that code in
JGit, which I very much plan to do.
Ciao,
Dscho
^ permalink raw reply
* Re: git commit -v does not removes the patch
From: Junio C Hamano @ 2008-11-10 23:27 UTC (permalink / raw)
To: Santi Béjar; +Cc: Jeff King, Git Mailing List
In-Reply-To: <adf1fd3d0811101434j658b2e8aj83d8cbe2293f5021@mail.gmail.com>
"Santi Béjar" <santi@agolina.net> writes:
> On Mon, Nov 10, 2008 at 7:10 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Nov 10, 2008 at 04:20:51PM +0100, Santi Béjar wrote:
>>
>>> Hi *,
>>>
>>> $subject since:
>>>
>>> 4f672ad (wt-status: load diff ui config, 2008-10-26)
>>>
>>> I tried to make a test case, but failed. I think because it is a bit
>>> tricky the fake_editor/stdin/stdout stuff, so at the end I bisected it
>>> by hand
>>
>> Sorry, I don't quite understand what the problem is. From reading your
>> subject line, I expected that "git commit -v" would show the diff in
>> your editor, but then accidentally also include it in the final commit
>> message. But I can't seem to reproduce that.
>>
>> Can you describe the problem in more detail?
>
> It is exactly as you described. I'll try in other systems.
Guess in the dark... by any chance are you enabling color unconditionally?
^ permalink raw reply
* Re: [PATCH] git push: Interpret $GIT_DIR/branches in a Cogito compatible way
From: Junio C Hamano @ 2008-11-10 23:25 UTC (permalink / raw)
To: Martin Koegler; +Cc: git
In-Reply-To: <1226353631-3716-1-git-send-email-mkoegler@auto.tuwien.ac.at>
Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
> Current git versions ignore everything after # (called <head> in the
> following) when pushing. Older versions (before cf818348f1ab57),
> interpret #<head> as part of the URL, which make git bail out.
>
> Ignoring the <head> part for push (fetch respects them) is unlogical.
> As branches origin from Cogito, it is the best to correct this by
> using the behaviour of cg-push:
>
> push HEAD to remote refs/heads/<head>
>
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
This message was addressed to me, but is it meant for inclusion?
I do not recall seeing an agreement on what the desired behaviour should
be from (ex-)Cogito users, if this change of behaviour hurts real world
usage of existing git users, andr if so how we ease this change in to the
release.
While I'd personally agree matching with whatever cg-push used to do might
make the most sense in the end, I am not sure changing of behaviour
abruptly like this is a good idea.
I am also not so sure url#branch is illogical; I'd suggest dropping that
line from the commit log message in any case.
> +
> +Depending on the operation, git will use one of the following
> +refsprecs, if you don't provide one on the command line.
> +`<branch>` is the name of this file in `$GIT_DIR/branches` and
> +`<head>` defaults to `master`.
> +
> +git fetch uses:
> +
> +------------
> + refs/heads/<head>:refs/heads/<branch>
> +------------
> +
> +git push uses:
>
> ------------
> - refs/heads/<head>:<repository>
> + HEAD:refs/heads/<head>
> ------------
Why isn't this "refs/heads/<head>:refs/heads/<head>", by the way?
^ permalink raw reply
* Re: overly smart rebase - bug or feature?
From: Junio C Hamano @ 2008-11-10 23:14 UTC (permalink / raw)
To: Fedor Sergeev; +Cc: Roman.Shaposhnick, git
In-Reply-To: <20081110212333.GU6799@sun.com>
Fedor Sergeev <Fedor.Sergeev@Sun.COM> writes:
> I have recently hit a behavior which might well be a feature,
> but it was very surprising (in a bad sense) to me.
It is a feature misfiring.
Rebase is essentially a repeated cherry-pick, and a cherry-pick of commit
A on top of commit B is done by a simplified 3-way merge between A and B
using the parent of A as the common ancestor.
A A'
/ /
A^... pseudo history ...---B
When your history has renamed Makefile to Makefile2 (thereby losing
Makefile) while transition from A^ to A modified Makefile, the difference
between A^ to A that is applied to B to produce A' contains only the
change about Makefile (and does not talk about the unchangedness of
Makefile1 nor Makefile2 --- in fact, when A' is created, the machinery
does not even know if A^ and A had Makefile1 or Makefile2).
When applying the change to Makefile, it notices that B does not have
Makefile, but there is a path that is _identical_ to the preimage your
change applies to (namely, Makefile2). To support people who rename
Makefile to Makefile2 in the history that led to B, rebase (actually the
underlying "am -3" it calls is where this rename detection smart lies)
applies the changes to the "renamed" path.
You might be able to work this around by forcing rebase not to use the
simplified 3-way merge, by saying "rebase -m".
^ permalink raw reply
* Re: Stgit and refresh-temp
From: Jon Smirl @ 2008-11-10 23:10 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Catalin Marinas, Git Mailing List
In-Reply-To: <9e4733910811101508h3cb30752o77b61926aeefed5b@mail.gmail.com>
On Mon, Nov 10, 2008 at 6:08 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On Fri, Nov 7, 2008 at 12:44 AM, Karl Hasselström <kha@treskal.com> wrote:
>> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>>
>>> I hit a case when refreshing a buried patch that needed a merge
>>> conflict sorted out. I'm unable to recover out of the state.
>>
>> Hmm, so what you're saying is basically that you did something with
>> "stg refresh -p" that caused a merge conflict, and that messed things
>> up so that you needed to run "stg repair". Is that right?
>
> Missed the reply.
>
> Yes, that is what happed.
>
> I think the problem was this:
>
> File - xxxxxxx
> Patch A adds a line
> File - xxxxxxxa
> Patch B in the middle adds a line
> File - xxxxxxxab
> I edit it and add a line
> File - xxxxxxxabc
> Line c needs to be patch A
> stg refresh -p A
> ..messed up tree
>
>
>
>>
>> Have you been able to reproduce it? (I would like to add the failing
>> case to the test suite.)
Yes, it happens every time the 'stg refresh -p' triggers a merge conflict.
stg repair seem to fix it and then I touch things up manually.
>>
>> --
>> Karl Hasselström, kha@treskal.com
>> www.treskal.com/kalle
>>
>
>
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Stgit and refresh-temp
From: Jon Smirl @ 2008-11-10 23:08 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Catalin Marinas, Git Mailing List
In-Reply-To: <20081107054419.GA27146@diana.vm.bytemark.co.uk>
On Fri, Nov 7, 2008 at 12:44 AM, Karl Hasselström <kha@treskal.com> wrote:
> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>
>> I hit a case when refreshing a buried patch that needed a merge
>> conflict sorted out. I'm unable to recover out of the state.
>
> Hmm, so what you're saying is basically that you did something with
> "stg refresh -p" that caused a merge conflict, and that messed things
> up so that you needed to run "stg repair". Is that right?
Missed the reply.
Yes, that is what happed.
I think the problem was this:
File - xxxxxxx
Patch A adds a line
File - xxxxxxxa
Patch B in the middle adds a line
File - xxxxxxxab
I edit it and add a line
File - xxxxxxxabc
Line c needs to be patch A
stg refresh -p A
..messed up tree
>
> Have you been able to reproduce it? (I would like to add the failing
> case to the test suite.)
>
> --
> Karl Hasselström, kha@treskal.com
> www.treskal.com/kalle
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] 3-way merge with file move fails when diff.renames = copies
From: David D. Kilzer @ 2008-11-10 22:26 UTC (permalink / raw)
To: git; +Cc: David D. Kilzer, Johannes Schindelin, gitster
With diff.renames = copies, a 3-way merge (e.g. "git rebase") would
fail with the following error:
fatal: mode change for <file>, which is not in current HEAD
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001.
The bug is a logic error added in ece7b749, which attempts to find
an sha1 for a patch with no index line in build_fake_ancestor().
Instead of failing unless an sha1 is found for both the old file and
the new file, a failure should only be reported if neither the old
file nor the new file is found.
Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
---
builtin-apply.c | 2 +-
t/t3400-rebase.sh | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 4c4d1e1..cfeb6cc 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2573,7 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
else if (get_sha1(patch->old_sha1_prefix, sha1))
/* git diff has no index line for mode/type changes */
if (!patch->lines_added && !patch->lines_deleted) {
- if (get_current_sha1(patch->new_name, sha1) ||
+ if (get_current_sha1(patch->new_name, sha1) &&
get_current_sha1(patch->old_name, sha1))
die("mode change for %s, which is not "
"in current HEAD", name);
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b7a670e..a156850 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -84,4 +84,21 @@ test_expect_success 'rebase a single mode change' '
GIT_TRACE=1 git rebase master
'
+test_expect_success 'rebase a single file move with diff.renames = copies' '
+ git config diff.renames copies &&
+ git checkout master &&
+ echo 1 > Y &&
+ git add Y &&
+ test_tick &&
+ git commit -m "prepare file move" &&
+ git checkout -b filemove HEAD^ &&
+ echo 1 > Y &&
+ git add Y &&
+ mkdir D &&
+ git mv A D/A &&
+ test_tick &&
+ git commit -m filemove &&
+ GIT_TRACE=1 git rebase master
+'
+
test_done
--
1.6.0
^ permalink raw reply related
* Re: git commit -v does not removes the patch
From: Santi Béjar @ 2008-11-10 22:34 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20081110181023.GA22753@coredump.intra.peff.net>
On Mon, Nov 10, 2008 at 7:10 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 10, 2008 at 04:20:51PM +0100, Santi Béjar wrote:
>
>> Hi *,
>>
>> $subject since:
>>
>> 4f672ad (wt-status: load diff ui config, 2008-10-26)
>>
>> I tried to make a test case, but failed. I think because it is a bit
>> tricky the fake_editor/stdin/stdout stuff, so at the end I bisected it
>> by hand
>
> Sorry, I don't quite understand what the problem is. From reading your
> subject line, I expected that "git commit -v" would show the diff in
> your editor, but then accidentally also include it in the final commit
> message. But I can't seem to reproduce that.
>
> Can you describe the problem in more detail?
It is exactly as you described. I'll try in other systems.
Santi
^ 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