* 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: 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: [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: 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] 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: [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] 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
* [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
* [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 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
* [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 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 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
* 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
* 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: [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: [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: 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: Git Notes - Track rebase/etc + reverse-lookup for bugs ideas
From: Jeff King @ 2008-11-11 0:31 UTC (permalink / raw)
To: Thomas Harning; +Cc: Johannes Schindelin, git
In-Reply-To: <74EBEC20-FC39-444E-8B96-31E5B36894B8@gmail.com>
On Mon, Nov 10, 2008 at 03:26:52PM -0500, Thomas Harning wrote:
> What I intended is that if notes are attached to 'A', A` (after a
> rebase) will have the exact same note.
I think that may have been brought up at the GitTogether. It would be
very easy, when rebasing A to A', to copy any notes for A to A'
(retroactively applying notes for A' to A (or vice versa) is much
harder, since we don't maintain any mapping between the two).
_But_ that is not necessarily a good idea in all cases, because your
notes may say certain things about A like "I tested this on system X".
But you _haven't_ tested A', and it might not pass your test. Leaving
aside editing the commits with "rebase -i", there might just be a bad
interaction with the commit you rebased onto.
So I think it would probably make sense to add a "--copy-notes" option
to rebase for those times when the user knows it makes sense, but doing
so by default is probably a mistake.
-Peff
^ permalink raw reply
* Re: [PATCH] git-diff: Add --staged as a synonym for --cached.
From: Junio C Hamano @ 2008-11-11 1:11 UTC (permalink / raw)
To: David Symonds
Cc: Jeff King, Junio C Hamano, Björn Steinbrink,
Johannes Schindelin, git, Stephan Beyer
In-Reply-To: <ee77f5c20811101537u6061e5b4w420e9692e0cefad3@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> 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?
I do not think anybody is fundamentally opposed to introduce a consistent
set of new synonyms. I do not think anybody disagrees that the word
"stage" will be involved in that set, either.
I however have a suspicion that people would regret having applied this
"diff --staged" patch, after they realize that other commands need two
options "--staged-only" and "--staged-too", and would wish this patch were
to introduce a synonym "diff --staged-only", not "diff --staged", for
uniformity's sake.
I doubt "Is there any further discussion on THIS ONE ALONE?" is a valid
question to ask. What are the other command options we are introducing
synonyms for? There is no need for two variants of staged for "diff" (you
don't have --staged-too option but instead you give a committish argument,
e.g. HEAD), so --staged-only can be abbreviated to --staged without
risking any ambiguity. But at least a fully-spelled-out --staged-only
should also be accepted, shouldn't it?
^ permalink raw reply
* Re: [PATCH] git-diff: Add --staged as a synonym for --cached.
From: Jeff King @ 2008-11-11 1:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: David Symonds, Björn Steinbrink, Johannes Schindelin, git,
Stephan Beyer
In-Reply-To: <7vljvr2hjn.fsf@gitster.siamese.dyndns.org>
On Mon, Nov 10, 2008 at 05:11:08PM -0800, Junio C Hamano wrote:
> I doubt "Is there any further discussion on THIS ONE ALONE?" is a valid
> question to ask. What are the other command options we are introducing
> synonyms for? There is no need for two variants of staged for "diff" (you
> don't have --staged-too option but instead you give a committish argument,
> e.g. HEAD), so --staged-only can be abbreviated to --staged without
> risking any ambiguity. But at least a fully-spelled-out --staged-only
> should also be accepted, shouldn't it?
I'm not sure that "staged-only" really makes sense here. In modification
commands like "apply", it is about "do this one thing to the working
tree, to both the index and the working tree, or to just the index".
But here, you are selecting two points for comparison. So while it is
tempting to say "the default for diff just happens to work on the index
and the working tree, so we don't need --staged-too", I don't think that
is right. Doing "--staged-only" is _not_ about saying "do the thing we
would have done to the working tree and the index to just the index." It
is about "use HEAD as one of the points instead of the working tree
(and reverse the order of points :) )".
To me, what is really being asked with "git diff --staged" (or "git
diff --cached" for that matter), is "what is staged?" That is, diff is
not about an operation on a data location (like HEAD, index, or working
tree), but rather an operatoin on a data _relationship_. So you ask for
"what is not staged" (the relationship between index and working tree),
"what is staged" (the relationship between HEAD and index), "what is
different between the working tree and HEAD", or "what is different
between these two trees".
-Peff
^ permalink raw reply
* [ANNOUNCE] darcs-fast-export
From: Miklos Vajna @ 2008-11-11 1:53 UTC (permalink / raw)
To: git; +Cc: bazaar, mercurial, darcs-users
[-- Attachment #1.1: Type: text/plain, Size: 886 bytes --]
Hi,
I'm happy to announce a small project I recently stated to work on. It's
a darcs backend for fast importers.
This means that it allows you to export patches/commits from a darcs
repo and import to any version control system where a fast importer is
implemented.
Current features:
- According to its name, it tries to be really fast, even for large
repos.
- Independent. It has been tested with git fast-import, hg fastimport
and bzr fast-import. (Let me know if you know other fast-importers!)
- It supports both (darcs1 and darcs2) darcs repo formats.
- It supports incremental conversions.
More info on the homepage:
http://vmiklos.hu/project/darcs-fast-export/
Including a list of alternatives, where I try to compare them to
darcs-fast-export.
Any feedback or comment is appreciated.
I'm not on the bazaar and the mercurial list, so there please CC me.
Thanks.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 148 bytes --]
_______________________________________________
darcs-users mailing list
darcs-users@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-users
^ permalink raw reply
* Re: [PATCH] git send-email: edit recipient addresses with the --compose flag
From: Tait @ 2008-11-11 1:49 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811091344480.20499@sys-0.hiltweb.site>
> > > + if ($c_file =~ /^To:\s*+(.+)\s*\nCc:/ism) {
> >
> > Greedy operators are only supported with perl 5.10 or more... I think it's a
> > bad idea to use them...
>
> The problem here was that a space should follow the field, but it may
> not. The user may unwarily backup over it. "\s*" would match this
> case.
>
> But if there is a space, it is included in the "(.+)".
Not in any version of Perl to which I have access.
Why doesn't
/^To:\s*(.+)\s*\nCc:/ism
work?
^ permalink raw reply
* RE: why not TortoiseGit
From: Li Frank @ 2008-11-11 1:51 UTC (permalink / raw)
To: Nigel Magnay; +Cc: Scott Chacon, Andreas Ericsson, Ian Hilt, git
In-Reply-To: <320075ff0811030200q606b76a8i16496cf7b8b1b7d2@mail.gmail.com>
Nigel Magnay:
I create a tortoisegit project at repo.or.cz.
http://repo.or.cz/w/TortoiseGit.git
It is coming from TortoiseSVN. It is very early stage. The context menu
have worked. TortoiseGitProc.exe help command have been worked.
Welcome contribute.
Best regards
Frank Li
-----Original Message-----
From: Nigel Magnay [mailto:nigel.magnay@gmail.com]
Sent: Monday, November 03, 2008 6:00 PM
To: Li Frank-B20596
Cc: Scott Chacon; Andreas Ericsson; Ian Hilt; git@vger.kernel.org
Subject: Re: why not TortoiseGit
> I read some code of TortoiseSVN and TortoiseHg Code.
> At beginning, TortoiseGit can git command to get information like
Qgit.
> After linkable library ready, replace "git command".
>
> I think TortoiseGit can start base on below way.
>
> 1. Base on TortoiseHg, It is python Script. Replace below hg
> operator with Git.
> 2. Base on TortoiseSVN, It is developed with C++. Need VS2008.
> ToritoiseSVN provide some built in diff and merge tools.
> 3. Base on Qgit, which provide some basic UI, such comment dialogbox,
> history view and file annotate.
>
>
TortoiseSVN is a good place to start because it separates out the
windows icon decorators into a separate DLL (shared with TortoiseCVS).
This is significant, as these are a finite resource in the windows
shell, and so having a TortoiseSVN + TortoiseGIT on one machine and you
might run out, and I'd imagine lots of people wanting both.
On the minus side, building (Tortoise)SVN requires a lot of environment
setup just to get it to build - most of which can be immediately thrown
away as it's specific to SVN.
But it doesn't look like a hard project to me, just requires stripping
out a lot of junk and re-patching callouts to a git executable (which
could be the standard git tools) and a minimal git library that knows if
files are dirty.
^ permalink raw reply
* Re: [ANNOUNCE] darcs-fast-export
From: zooko @ 2008-11-11 2:39 UTC (permalink / raw)
To: Miklos Vajna; +Cc: bazaar, mercurial, git, darcs-users
In-Reply-To: <20081111015313.GB24201@genesis.frugalware.org>
Hello Miklos Vajna:
Thanks for announcing this project.
I looked at the "alternatives" section, and you wrote:
"tailor is a nice any2any VCS converter, but the end result can
easily differ to the original repo because tailor replays all the
changesets itself, while darcs-fast-export just leaves this task to
darcs."
If you can show an example of the end result differing from the
original repo then that is a bug in tailor, and please report it!
Thanks.
Regards,
Zooko
---
http://allmydata.org -- Tahoe, the Least-Authority Filesystem
http://allmydata.com -- back up all your files for $10/month
^ 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