* [PATCH] Use warning function instead of fprintf(stderr, "Warning: ...").
From: Thiago Farina @ 2010-01-02 20:24 UTC (permalink / raw)
To: git
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
bisect.c | 2 +-
builtin-mv.c | 4 +---
http.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/bisect.c b/bisect.c
index dc18db8..d1f8c42 100644
--- a/bisect.c
+++ b/bisect.c
@@ -813,7 +813,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
char *bad_hex = sha1_to_hex(current_bad_sha1);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
- fprintf(stderr, "Warning: the merge base between %s and [%s] "
+ warning("the merge base between %s and [%s] "
"must be skipped.\n"
"So we cannot be sure the first bad commit is "
"between %s and %s.\n"
diff --git a/builtin-mv.c b/builtin-mv.c
index f633d81..8ad7245 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -169,9 +169,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- fprintf(stderr, "Warning: %s;"
- " will overwrite!\n",
- bad);
+ warning("%s; will overwrite!\n", bad);
bad = NULL;
} else
bad = "Cannot overwrite";
diff --git a/http.c b/http.c
index ed6414a..afce7c3 100644
--- a/http.c
+++ b/http.c
@@ -1244,7 +1244,7 @@ int finish_http_object_request(struct http_object_request *freq)
process_http_object_request(freq);
if (freq->http_code == 416) {
- fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n");
+ warning("requested range invalid; we may already have all the data.\n");
} else if (freq->curl_result != CURLE_OK) {
if (stat(freq->tmpfile, &st) == 0)
if (st.st_size == 0)
--
1.6.6.rc3
^ permalink raw reply related
* Re: [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"
From: Junio C Hamano @ 2010-01-02 19:50 UTC (permalink / raw)
To: Christian Couder
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20100102053934.30066.85625.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> The purpose of this new option is to discard some of the last commits
> but to keep current changes in the work tree.
>
> The use case is when you work on something and commit that work. And
> then you work on something else that touches other files, but you don't
> commit it yet. Then you realize that what you commited when you worked
> on the first thing is not good or belongs to another branch.
>
> So you want to get rid of the previous commits (at least in the current
> branch) but you want to make sure that you keep the changes you have in
> the work tree. And you are pretty sure that your changes are independent
> from what you previously commited, so you don't want the reset to
> succeed if the previous commits changed a file that you also changed in
> your work tree.
>
> The table below shows what happens when running "git reset --option
> target" to reset the HEAD to another commit (as a special case "target"
> could be the same as HEAD) in the cases where "--merge" and "--keep"
> behave differently.
I think this new option is unrelated to "--merge"; iow, the only relation
to it is that it is an option to the same command "git reset", so it is
related but it is related the same way and to the degree as "--mixed" is.
Thinking about it even more, if the number of commits you are resetting
away is zero in your use case (i.e. target is HEAD), shouldn't this new
mode of operation degenerate to "--mixed"? So in that sense, it might
make sense to contrast it with "--mixed".
But let's try not to contrast it with anything else, and see how well it
stands on its own. The below is my attempt.
> working index HEAD target working index HEAD
> ----------------------------------------------------
> A B C D --keep (disallowed)
> A B C C --keep A C C
> B B C D --keep (disallowed)
> B B C C --keep B C C
Let's give an explanation of the above in terms of what this means to the
end users.
When you have changes to a path that the accumulated commits between
target..HEAD touch, you don't want to discard them. It doesn't matter
if the changes in the work tree has been partially added (A != B) or
fully added (A == B) to the index. In both cases, the operation is
disallowed, just like "checkout $another_branch" stops in such a case.
But if you have local modifications based on one version and dropping
the accumulated commits between target..HEAD does not involve that
path, we can safely "transplant" that change to the target.
Presented this way, a future direction (iow, I am not suggesting you to do
this before the current series solidifies) might be to allow users to do
something similar to "checkout -m $the_other_branch". IOW, instead of
disallowing "I have changed from C to B and switching to D" case, perform
a three-way merge to bring the work tree file to (D+(B-C)), transplanting
the local change you made on top of the target.
> The following table shows what happens on unmerged entries:
>
> working index HEAD target working index HEAD
> ----------------------------------------------------
> X U A B --keep (disallowed)
> X U A A --keep X A A
In a sense, this is consistent with the above; the local change attempted
happens to be an unmerged result.
But it is inconsistent with the intended use case you presented, which
leaves no room for unmerged entries to enter in the index to begin with.
It might be safer to error out on any unmerged entry in the index. I
dunno.
^ permalink raw reply
* Re: [RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries
From: Junio C Hamano @ 2010-01-02 19:46 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.95746.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> From: Junio C Hamano <gitster@pobox.com>
>
> Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed
> "git reset --merge" when there was unmerged entries. It acknowledged
> that this is not the best possible behavior, and that it would be better
> if unmerged entries were reset as if --hard (instead of --merge) has
> been used.
>
> Recently another commit (reset: use "unpack_trees()" directly instead of
> "git read-tree", 2009-12-30) changed the behavior of --merge to accept
> resetting unmerged entries if they are reset to a different state than
> HEAD, but it did not reset the changes in the work tree. So the behavior
> was kind of improved, but it was not yet as if --hard has been used.
It would be more honest if we said something like:
It changed a safer "I can't do as asked, please do it by hand"
into a more dangerous "I pretend that I did so but I didn't do the
full job; you need to fix up the result but I am not telling you
that you have to", which is a lot worse.
here instead (that is one reason why I said my fix-up was "squashable").
But that is a minor issue.
I have been thinking about two issues on this --merge change.
> - Updates merged_entry() and deleted_entry() so that they pay attention
> to cache entries with null sha1 (note that we _might_ want to use a
> new in-core ce->ce_flags instead of using the null-sha1 hack). They
> are previously unmerged entries, and the files in the work tree that
> correspond to them are resetted away by oneway_merge() to the version
> from the tree we are resetting to.
One is the use of ce_flags instead of relying on the 20-byte comparison I
said above, for both performance (minor) and future maintainability (much
bigger) concern. I have a feeling that we will regret later that we used
the null_sha1 trick here, when we want to express another "special" kind
of cache entry in unrelated situations. The use of null_sha1 hack was
expedient but I fell victim of the same mentality of declaring that this
is the _last_ such kind of special index entry and closing the door to
others who want to extend the system later with different kind of special
cache entry, which I often complain about myself to patches from other
people.
Another is that it _might_ make sense to use two-tree form of read-tree
machinery (but using a different version of unpack-trees.c::twoway_merge()
function), instead of the one-tree form of "we don't bother checking if
the index is consistent with HEAD and assume it is, and jump to the
target."
"git reset --merge $there" is about the situation where you started a
"mergy" operation (e.g. "git merge", "git am -3", "git rebase", ...) while
you had unrelated local changes in the work tree, and you want to go back
to the state before that operation $there (which is HEAD if the mergy
operation is "merge", but is different from HEAD if it was "am -3" and you
have successfully applied a patch or more already). Linus may know and
won't use "reset --merge" in a situation where it is not suitable, but not
everybody is Linus. Even though "reset --merge" may "correctly" work with
respect to the table you added to "git reset" documentation, it would do
something that may not "make sense" from the end-user's point of view when
used in a situation that it wasn't designed for. Using the two-tree form
allows the machinery to inspect the difference between HEAD and the index,
and detect cases where "reset --merge" was attempted when it shouldn't.
For example, if stage #2 of an unmerged path does not match HEAD, we know
there is something wrong.
This latter issue is much bigger and needs a lot more thought, and I don't
think it should block the series from going forward at all. But I think
it is worth keeping in the back of our heads.
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2010-01-02 19:45 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <fcaeb9bf1001021115j7b23264n42cfba7855c2253e@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> > +support_external_grep() {
>> > + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
>> > + *"(default)"*) return 0;;
>> > + *"(ignored by this build)"*) return 1;;
>> > + *) test_expect_success 'External grep check is broken' 'false';;
>> > + esac
>> > +}
>>
>>
>> Heh, clever.
>>
>> git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
>>
>> may be sufficient, though.
>
> Yes, until somebody changes help text in builtin-grep.c and all
> external grep tests become disable. I wanted to catch that case too.
Ok, that is a worthwhile thing to do.
Then please at least make the second grep "grep ext-grep", droping "-e --"
from it. We assume some implementation of external grep to lack "-e"
(e.g. Solaris).
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Nguyen Thai Ngoc Duy @ 2010-01-02 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtyv4cpna.fsf@alter.siamese.dyndns.org>
On 1/3/10, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
> > On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>
> >> > This looks a bit wrong for a couple of reasons:
> >> >
> >> > - external_grep() is designed to return negative without running external
> >> > grep when it shouldn't be used (see the beginning of the function for
> >> > how it refuses to run when opt->extended is set and other conditions).
> >> > The new logic seems to belong there, i.e. "in addition to the existing
> >> > case we decline, if ce_skip_worktree() entry exists in the cache, we
> >> > decline";
> >>
> >> IOW, something like this instead of your patch. You would want to tests
> >> to demonstrate the original breakage, perhaps?
> >
> > Your patch works great. By the way I think we should move "cached"
> > check from grep_cache() into external_grep() too, for consistency.
>
>
> I think what I gave you is more consistent.
>
> "cached" is about "are we searching in the index or in the work tree?" and
> "external_grep()" is about "it often is faster to run external grep when
> we are searching in the work tree, so do that if the other constraints
> allow us to". An example of such a constraint is that we must be able to
> express the operation on the command line of a traditional grep, and use
> of git extended grep synatx makes it unfeasible, hence the function says
> "I can't" in such a case.
>
> You can change the definition of "external_grep()" to "try to use external
> grep somewhere, if the other constraints allow us to", and have the caller
> and the function do an "this time, please grep for this pattern in the
> index---I can't" exchange, but I don't see much point. We _know_ no
> external grep will ever be able to read from the index.
>
> We should simply drop "cached" argument from external_grep().
OK.
> > diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> > index abd14bf..f77970c 100755
> > --- a/t/t7002-grep.sh
> > +++ b/t/t7002-grep.sh
> > @@ -8,6 +8,14 @@ test_description='git grep various.
> >
> > . ./test-lib.sh
> >
> > +support_external_grep() {
> > + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
> > + *"(default)"*) return 0;;
> > + *"(ignored by this build)"*) return 1;;
> > + *) test_expect_success 'External grep check is broken' 'false';;
> > + esac
> > +}
>
>
> Heh, clever.
>
> git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
>
> may be sufficient, though.
>
Yes, until somebody changes help text in builtin-grep.c and all
external grep tests become disable. I wanted to catch that case too.
Or maybe just add another option --show-features to "git rev-parse"
(or extend git version string, is the version format fixed?) to list
all built-time features that a git build supports. Some comes to mind:
external-grep, iconv, ipv6, http (unlikely), threading... Much less
tricky to test.
--
Duy
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2010-01-02 18:44 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <20100102115041.GA32381@do>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > This looks a bit wrong for a couple of reasons:
>> >
>> > - external_grep() is designed to return negative without running external
>> > grep when it shouldn't be used (see the beginning of the function for
>> > how it refuses to run when opt->extended is set and other conditions).
>> > The new logic seems to belong there, i.e. "in addition to the existing
>> > case we decline, if ce_skip_worktree() entry exists in the cache, we
>> > decline";
>>
>> IOW, something like this instead of your patch. You would want to tests
>> to demonstrate the original breakage, perhaps?
>
> Your patch works great. By the way I think we should move "cached"
> check from grep_cache() into external_grep() too, for consistency.
I think what I gave you is more consistent.
"cached" is about "are we searching in the index or in the work tree?" and
"external_grep()" is about "it often is faster to run external grep when
we are searching in the work tree, so do that if the other constraints
allow us to". An example of such a constraint is that we must be able to
express the operation on the command line of a traditional grep, and use
of git extended grep synatx makes it unfeasible, hence the function says
"I can't" in such a case.
You can change the definition of "external_grep()" to "try to use external
grep somewhere, if the other constraints allow us to", and have the caller
and the function do an "this time, please grep for this pattern in the
index---I can't" exchange, but I don't see much point. We _know_ no
external grep will ever be able to read from the index.
We should simply drop "cached" argument from external_grep().
> I thought of tests when I wrote the patch, but it was hard to find a
> reliable way to detect if a git build supports external grep.
Ah, you are right. I forgot about that issue.
> diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> index abd14bf..f77970c 100755
> --- a/t/t7002-grep.sh
> +++ b/t/t7002-grep.sh
> @@ -8,6 +8,14 @@ test_description='git grep various.
>
> . ./test-lib.sh
>
> +support_external_grep() {
> + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
> + *"(default)"*) return 0;;
> + *"(ignored by this build)"*) return 1;;
> + *) test_expect_success 'External grep check is broken' 'false';;
> + esac
> +}
Heh, clever.
git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
may be sufficient, though.
^ permalink raw reply
* Re: [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Junio C Hamano @ 2010-01-02 18:43 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
In-Reply-To: <1262412622-sup-7473@utwig>
Alex Vandiver <alex@chmrr.net> writes:
> most common usage model. It seems to me like the more correct fix
> would be to move the diff options to later in the file (after the
> options that are `git log`-specific), or to remove them entirely, and
> replace them with a pointer to git diff's options.
IIRC, the apporach we took earlier that people hated and resulted in the
current "include" approach was to say "It also takes many of options that
diff family of commands take". So reordering may work better but dropping
and referring would not, I suspect.
^ permalink raw reply
* Re: Filename quoting / parsing problem
From: Junio C Hamano @ 2010-01-02 18:37 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: git
In-Reply-To: <201001021236.26947.agruen@suse.de>
Andreas Gruenbacher <agruen@suse.de> writes:
> On Friday 01 January 2010 09:01:19 pm Junio C Hamano wrote:
>> > Both "b file" and "c file " are parsed by "git apply" perfectly fine.
>
> Right, the "diff --git" lines are technically still parseable when the file
> name stays the same. With renames, lines like "diff --git a/f a/f b/f" or
> "diff --git a/f b/f b/f" are possible, but then there will also be "renamed
> from" and "renamed to" headers which will disambiguate things. Still, it
> doesn't seem like a good idea to allow such ambiguities in the first place.
You already realized that there is no ambiguity because "diff --git" lines
are parsable and renames have explicit names. Why do you still maintain
that we are allowing such "ambiguities" when there is none?
>> Having said all that, I don't think we would mind a change to treat a
>> pathname with trailing SP a bit specially (iow, quoting "c file " in the
>> above failed attempt to reproduce the issue).
>
> I would prefer quoting file names which contain spaces anywhere,...
The only reason I said I don't think we would mind changing the trailing
SP case is because the reduced risk of getting our patches corrupted by
MUA _might_ outweigh the benefit of not quoting to avoid an eyesore [*1*].
But what you said would add to eyesore of quoted names (which you omitted
from your quote) without any justification other than "I would prefer".
The pros-and-cons in such a change is quite different; as we have already
established that there is no ambiguity, "disambuguation" is not a "pro" in
this comparison.
[Footnote]
*1* Strictly speaking, it is not just "an eyesore" that is an issue. Our
diff output without renames are designed to be grokkable by other people's
patch implementations (e.g. GNU patch), and the quoted pathnames are not
understandable by them. Even though our final version of quoted path
format came from the GNU diff/patch maintainer (back then, at least):
http://article.gmane.org/gmane.comp.version-control.git/10103
I don't think it happened in the GNU land yet, and you would be the person
to know about it ;-).
^ permalink raw reply
* Re: subtree merge tries to merge into wrong directory
From: Nils Adermann @ 2010-01-02 18:17 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git
In-Reply-To: <c10155f8a7a2dd451f0b74e323f3a989.squirrel@www.naderman.de>
naderman@naderman.de schrieb:
> So I finally got around to trying this. This was my first result:
>
> $ git merge -Xsubtree=lib/ezc/trunk/Reflection/ FETCH_HEAD
> fatal: entry not found in tree 60270661e0d2a5ee03b24609fac5c6d00d048988
>
Turns out that only happens because of the trailing slash.
lib/ezc/trunk/Reflection works reliably.
Nils
^ permalink raw reply
* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Daniel Convissor @ 2010-01-02 17:14 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.76552.chriscool@tuxfamily.org>
On Sat, Jan 02, 2010 at 06:39:32AM +0100, Christian Couder wrote:
> +++ b/Documentation/git-reset.txt
...
> +--keep::
> + Resets the index to match the tree recorded by the named commit,
> + but keep changes in the working tree. Aborts if the reset would
> + change files that are already changes in the working tree.
Grammar suggestion for the last line there. Change "already changes" to
"already changed". Or better yet, use a different word, such as "already
modified", since "change" based words have been used so many times
already in the sentence.
Thanks,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409
^ permalink raw reply
* [PATCH 2/2] git-gui: Add a special diff popup menu for submodules
From: Jens Lehmann @ 2010-01-02 16:58 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <4B3F7AE2.10007@web.de>
To make it easier for users to deal with submodules, a special diff popup
menu has been added for submodules. The "Show Less Context" and "Show More
Context" entries have been removed, as they don't make any sense for a
submodule summary. Four new entries are added to the top of the popup menu
to gain access to more detailed information about the changes in a
submodule than the plain summary does offer. These are:
- "Visualize These Changes In The Submodule"
starts gitk showing the selected commit range
- "Visualize These Changes In The Submodule"
starts gitk showing the whole submodule history of the current branch
- "Visualize All Branch History In The Submodule"
starts gitk --all in the submodule
- "Start git gui In The Submodule"
guess what :-)
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
git-gui/git-gui.sh | 127 +++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 110 insertions(+), 17 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e0dd5b5..ee80d7d 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1923,7 +1923,9 @@ proc incr_font_size {font {amt 1}} {
set starting_gitk_msg [mc "Starting gitk... please wait..."]
-proc do_gitk {revs} {
+proc do_gitk {revs {is_submodule false}} {
+ global current_diff_path file_states current_diff_side ui_index
+
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
#
@@ -1941,14 +1943,72 @@ proc do_gitk {revs} {
}
set pwd [pwd]
- cd [file dirname [gitdir]]
- set env(GIT_DIR) [file tail [gitdir]]
-
+ if {!$is_submodule} {
+ cd [file dirname [gitdir]]
+ set env(GIT_DIR) [file tail [gitdir]]
+ } else {
+ cd $current_diff_path
+ if {$revs eq {--}} {
+ set s $file_states($current_diff_path)
+ set old_sha1 {}
+ set new_sha1 {}
+ switch -glob -- [lindex $s 0] {
+ M_ { set old_sha1 [lindex [lindex $s 2] 1] }
+ _M { set old_sha1 [lindex [lindex $s 3] 1] }
+ MM {
+ if {$current_diff_side eq $ui_index} {
+ set old_sha1 [lindex [lindex $s 2] 1]
+ set new_sha1 [lindex [lindex $s 3] 1]
+ } else {
+ set old_sha1 [lindex [lindex $s 3] 1]
+ }
+ }
+ }
+ set revs $old_sha1...$new_sha1
+ }
+ if {[info exists env(GIT_DIR)]} {
+ unset env(GIT_DIR)
+ }
+ }
eval exec $cmd $revs &
- if {$old_GIT_DIR eq {}} {
+ if {$old_GIT_DIR ne {}} {
+ set env(GIT_DIR) $old_GIT_DIR
+ }
+ cd $pwd
+
+ ui_status $::starting_gitk_msg
+ after 10000 {
+ ui_ready $starting_gitk_msg
+ }
+ }
+}
+
+proc do_git_gui {} {
+ global current_diff_path
+
+ # -- Always start git gui through whatever we were loaded with. This
+ # lets us bypass using shell process on Windows systems.
+ #
+ set exe [_which git]
+ if {$exe eq {}} {
+ error_popup [mc "Couldn't find git gui in PATH"]
+ } else {
+ global env
+
+ if {[info exists env(GIT_DIR)]} {
+ set old_GIT_DIR $env(GIT_DIR)
unset env(GIT_DIR)
} else {
+ set old_GIT_DIR {}
+ }
+
+ set pwd [pwd]
+ cd $current_diff_path
+
+ eval exec $exe gui &
+
+ if {$old_GIT_DIR ne {}} {
set env(GIT_DIR) $old_GIT_DIR
}
cd $pwd
@@ -3143,15 +3203,6 @@ $ui_diff tag raise sel
proc create_common_diff_popup {ctxm} {
$ctxm add command \
- -label [mc "Show Less Context"] \
- -command show_less_context
- lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
- $ctxm add command \
- -label [mc "Show More Context"] \
- -command show_more_context
- lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
- $ctxm add separator
- $ctxm add command \
-label [mc Refresh] \
-command reshow_diff
lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
@@ -3206,6 +3257,15 @@ $ctxm add command \
set ui_diff_applyline [$ctxm index last]
lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
$ctxm add separator
+$ctxm add command \
+ -label [mc "Show Less Context"] \
+ -command show_less_context
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add command \
+ -label [mc "Show More Context"] \
+ -command show_more_context
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add separator
create_common_diff_popup $ctxm
set ctxmmg .vpane.lower.diff.body.ctxmmg
@@ -3228,9 +3288,40 @@ $ctxmmg add command \
-command {merge_resolve_one 1}
lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
$ctxmmg add separator
+$ctxmmg add command \
+ -label [mc "Show Less Context"] \
+ -command show_less_context
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add command \
+ -label [mc "Show More Context"] \
+ -command show_more_context
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add separator
create_common_diff_popup $ctxmmg
-proc popup_diff_menu {ctxm ctxmmg x y X Y} {
+set ctxmsm .vpane.lower.diff.body.ctxmsm
+menu $ctxmsm -tearoff 0
+$ctxmsm add command \
+ -label [mc "Visualize These Changes In The Submodule"] \
+ -command {do_gitk -- true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add command \
+ -label [mc "Visualize Current Branch History In The Submodule"] \
+ -command {do_gitk {} true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add command \
+ -label [mc "Visualize All Branch History In The Submodule"] \
+ -command {do_gitk --all true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add separator
+$ctxmsm add command \
+ -label [mc "Start git gui In The Submodule"] \
+ -command {do_git_gui}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add separator
+create_common_diff_popup $ctxmsm
+
+proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
global current_diff_path file_states
set ::cursorX $x
set ::cursorY $y
@@ -3241,6 +3332,8 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
}
if {[string first {U} $state] >= 0} {
tk_popup $ctxmmg $X $Y
+ } elseif {$::is_submodule_diff} {
+ tk_popup $ctxmsm $X $Y
} else {
if {$::ui_index eq $::current_diff_side} {
set l [mc "Unstage Hunk From Commit"]
@@ -3249,7 +3342,7 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
set l [mc "Stage Hunk For Commit"]
set t [mc "Stage Line For Commit"]
}
- if {$::is_3way_diff || $::is_submodule_diff
+ if {$::is_3way_diff
|| $current_diff_path eq {}
|| {__} eq $state
|| {_O} eq $state
@@ -3264,7 +3357,7 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
tk_popup $ctxm $X $Y
}
}
-bind_button3 $ui_diff [list popup_diff_menu $ctxm $ctxmmg %x %y %X %Y]
+bind_button3 $ui_diff [list popup_diff_menu $ctxm $ctxmmg $ctxmsm %x %y %X %Y]
# -- Status Bar
#
--
1.6.6.339.g7cacc
^ permalink raw reply related
* [PATCH 1/2] git-gui: Unstaging a partly staged entry didn't update file_states correctly
From: Jens Lehmann @ 2010-01-02 16:58 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <4B3F7AE2.10007@web.de>
When unstaging a partly staged file or submodule, the file_states list
was not updated properly (unless unstaged linewise). Its index_info part
did not contain the former head_info as it should have.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
git-gui/git-gui.sh | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 718277a..e0dd5b5 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1613,6 +1613,9 @@ proc merge_state {path new_state {head_info {}} {index_info {}}} {
} elseif {$s0 ne {_} && [string index $state 0] eq {_}
&& $head_info eq {}} {
set head_info $index_info
+ } elseif {$s0 eq {_} && [string index $state 0] ne {_}} {
+ set index_info $head_info
+ set head_info {}
}
set file_states($path) [list $s0$s1 $icon \
--
1.6.6.339.g7cacc
^ permalink raw reply related
* [PATCH 0/2] git-gui: Add a special diff popup menu for submodules
From: Jens Lehmann @ 2010-01-02 16:57 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
The first patch is a resend (first sent on December 7th 2009) and
necessary for the second patch to work properly.
Jens Lehmann (2):
git-gui: Unstaging a partly staged entry didn't update file_states
correctly
git-gui: Add a special diff popup menu for submodules
git-gui/git-gui.sh | 130 +++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 113 insertions(+), 17 deletions(-)
^ permalink raw reply
* [PATCH] stash: mention --patch in usage string.
From: Matthieu Moy @ 2010-01-02 16:35 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
git-stash.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index f796c2f..3a0685f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE="list [<options>]
or: $dashless drop [-q|--quiet] [<stash>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
- or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
+ or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
--
1.6.6.76.gd6b23.dirty
^ permalink raw reply related
* RFC: display dirty submodule working directory in git gui and gitk
From: Jens Lehmann @ 2010-01-02 15:33 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Shawn O. Pearce,
Paul Mackerras, Heiko Voigt, Lars Hjemli
Now that we have much better output when displaying diffs of
submodules in git gui and gitk (many thanks to all involved!),
another usability issue shows up: A dirty working directory of
a submodule isn't visible in git gui or gitk.
So you might think a "submodule update" would be ok - as you
see no changes - just too see it fail because the submodules
working directory is dirty.
Or - even worse - you /think/ you committed your changes in
a submodule while you didn't. That can lead to 'interesting'
problems which can be pretty hard to diagnose (like breaking
builds on other peoples machines).
A possible solution could look like this:
AFAICS, git gui and gitk use "git diff-files" both to get the
file names of unstaged local changes and to later display the
actual differences.
If they could tell the diff core to also check the submodule
working directories and to output an extra line - maybe
something like "Submodule <name> contains uncommitted local
changes" - when a submodules working directory is dirty,
git gui and gitk could show the submodules state adequately.
What do you think about this approach?
^ permalink raw reply
* User-wide Git config directory (was Re: [PATCH v2] Let core.excludesfile default to ~/.gitexcludes.)
From: Matthieu Moy @ 2010-01-02 12:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vk4w4z1h6.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Nanako Shiraishi <nanako3@lavabit.com> writes:
>>
>>> Junio, could you tell us what happened to this thread?
>>
>> I'm not Junio, but I can try ...
>> ...
>> So, I dropped the patch until we get closer to a consensus on what the
>> default value should be.
>
> Thanks for status updates. Should we start trying to reach a consensus on
> how to move forward, or is it not your itch?
I wouldn't say it's not my itch. But I'm not sure I'm the best person
to do the job (job = comming close to a consensus, not writting the
code, which shouldn't be hard). Still, I can at least participate ;-).
> It is not _too_ bad to treat ~/.gitconfig specially and support reading
> from ~/.$SOMEGITTTYNAME/{excludes,attributes} files.
Let's throw in some ideas about what ~/$SOMEGITTTYNAME/ could be :
1) ~/.git/ => no, that would clash with people versioning their $HOME.
2) ~/.gitconfig/ => no, there's already a file ~/.gitconfig
3) ~/.gitglobal/ => that's an option.
4) ~/.config/git/ or, if set, $XDG_CONFIG_HOME/git/ (see
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html )
=> my prefered one, already discussed in a different context here :
http://thread.gmane.org/gmane.comp.version-control.git/135447/focus=135559
Then, there's the question of what to put in this directory. I can see
two and a half options:
a) ~/.$SOMEGITTTYNAME/{excludes,attributes} as you propose.
a') ~/.$SOMEGITTTYNAME/{ignore,attributes} => I think "ignore" is the
advertized vocabulary most of the time in porcelain, and "excludes"
exists mostly for historical reasons.
b) ~/.$SOMEGITTTYNAME/<same thing as the content of $GIT_DIR>, that
is:
~/.$SOMEGITTTYNAME/info/exclude
~/.$SOMEGITTTYNAME/info/attributes
and perhaps
~/.$SOMEGITTTYNAME/config
we could imagine also
~/.$SOMEGITTTYNAME/hooks
If I were really happy with the layout of $GIT_DIR, I'd vote for b)
for consistancy. But I'm not _that_ happy with it: why are exclude and
attributes not in the same directory as config? why is exclude
singular and attributes plural?
So, I dunno.
> We can also add support for ~/.$SOMEGITTYNAME/config and try reading
> from it as well, but even if we did so I don't think we should stop
> reading from ~/.gitconfig.
Yes, people having a ~/.gitconfig around should be able to continue to
use it (for years at least, and most likely forever).
And you're right to present it as a different problem. Perhaps we
should solve the problem above before starting debating about this
one.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Nguyen Thai Ngoc Duy @ 2010-01-02 11:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl4zy5z3.fsf@alter.siamese.dyndns.org>
On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > This looks a bit wrong for a couple of reasons:
> >
> > - external_grep() is designed to return negative without running external
> > grep when it shouldn't be used (see the beginning of the function for
> > how it refuses to run when opt->extended is set and other conditions).
> > The new logic seems to belong there, i.e. "in addition to the existing
> > case we decline, if ce_skip_worktree() entry exists in the cache, we
> > decline";
>
> IOW, something like this instead of your patch. You would want to tests
> to demonstrate the original breakage, perhaps?
Your patch works great. By the way I think we should move "cached"
check from grep_cache() into external_grep() too, for consistency.
I thought of tests when I wrote the patch, but it was hard to find a
reliable way to detect if a git build supports external grep. Perhaps
this on top of yours? The way to check for external grep support isn't
nice, but it works.
diff --git a/builtin-grep.c b/builtin-grep.c
index f093b60..bc4500a 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -222,6 +222,7 @@ static int exec_grep(int argc, const char **argv)
int status;
argv[argc] = NULL;
+ trace_argv_printf(argv, "trace: grep:");
pid = fork();
if (pid < 0)
return pid;
@@ -371,7 +372,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
struct grep_pat *p;
if (opt->extended || (opt->relative && opt->prefix_length)
- || has_skip_worktree_entry(opt, paths))
+ || cached || has_skip_worktree_entry(opt, paths))
return -1;
len = nr = 0;
push_arg("grep");
@@ -509,7 +510,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
* we grep through the checked-out files. It tends to
* be a lot more optimized
*/
- if (!cached && external_grep_allowed) {
+ if (external_grep_allowed) {
hit = external_grep(opt, paths, cached);
if (hit >= 0)
return hit;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index abd14bf..f77970c 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -8,6 +8,14 @@ test_description='git grep various.
. ./test-lib.sh
+support_external_grep() {
+ case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
+ *"(default)"*) return 0;;
+ *"(ignored by this build)"*) return 1;;
+ *) test_expect_success 'External grep check is broken' 'false';;
+ esac
+}
+
cat >hello.c <<EOF
#include <stdio.h>
int main(int argc, const char **argv)
@@ -426,4 +434,20 @@ test_expect_success 'grep -Fi' '
test_cmp expected actual
'
+if support_external_grep; then
+
+test_expect_success 'external grep' '
+ GIT_TRACE=2 git grep foo >/dev/null 2>actual &&
+ grep "trace: grep:.*foo" actual >/dev/null
+'
+test_expect_success 'no external grep when skip-worktree entries exist' '
+ git update-index --skip-worktree file &&
+ GIT_TRACE=2 git grep foo >/dev/null 2>actual &&
+ ! grep "trace: grep:" actual >/dev/null &&
+ git update-index --no-skip-worktree file
+'
+
+else
+ say "External grep tests skipped"
+fi
test_done
--
Duy
^ permalink raw reply related
* Re: Filename quoting / parsing problem
From: Andreas Gruenbacher @ 2010-01-02 11:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr5q9lhm8.fsf@alter.siamese.dyndns.org>
On Friday 01 January 2010 09:01:19 pm Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > I used "cat -e" to make it easier to see that "c file " not only has SP
> > in it but it has trailing space. Let's try the result.
> >
> > $ git diff --cached | cat -e
> > diff --git "a/a\001file" "b/a\001file"$
> > new file mode 100644$
> > index 0000000..e69de29$
> > diff --git a/b file b/b file$
> > new file mode 100644$
> > index 0000000..e69de29$
> > diff --git a/c file b/c file $
> > new file mode 100644$
> > index 0000000..e69de29$
> > $ git diff --cached >P.diff
> >
> > And as you described, "b file" and "c file " are not quoted and they do
> > not have ---/+++ lines.
> >
> > But observe this:
> > ...
> > We are now back in the state without any of these files, and P.diff
> > records a patch to recreate these three files, one with quoting and the
> > other two without.
> >
> > $ git apply --index P.diff
> > $ git ls-files -s | cat -e
> > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0
> > "a\001file"$ 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 b
> > file$ 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 c file $
> > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 hello$
> >
> > This demonstrates that The claim below is false, doesn't it?
> >
> > > Not parseable:
> > > diff --git a/baz b/baz
> > > new file mode 100644
> > > index 0000000..e69de29
> >
> > Both "b file" and "c file " are parsed by "git apply" perfectly fine.
Right, the "diff --git" lines are technically still parseable when the file
name stays the same. With renames, lines like "diff --git a/f a/f b/f" or
"diff --git a/f b/f b/f" are possible, but then there will also be "renamed
from" and "renamed to" headers which will disambiguate things. Still, it
doesn't seem like a good idea to allow such ambiguities in the first place.
> Having said all that, I don't think we would mind a change to treat a
> pathname with trailing SP a bit specially (iow, quoting "c file " in the
> above failed attempt to reproduce the issue).
I would prefer quoting file names which contain spaces anywhere, not only at
the end. If quoting helps to disambiguate a program's output, I'm all for it.
People who can't be bothered with such details can always use a pretty printer
(side by side view, whatever).
Thanks,
Andreas
^ permalink raw reply
* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Andreas Schwab @ 2010-01-02 9:06 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.76552.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> +--keep::
> + Resets the index to match the tree recorded by the named commit,
> + but keep changes in the working tree. Aborts if the reset would
> + change files that are already changes in the working tree.
s/changes/changed/ in the last sentence.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Alex Vandiver @ 2010-01-02 6:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbr6phlx.fsf@alter.siamese.dyndns.org>
At Thu Dec 31 23:35:38 -0500 2009, Junio C Hamano wrote:
> [snip]
Thinking about this more, I'm more convinced that this is just a
symptom of a bigger problem -- why does the help for `git log` start
off with the _diff_ options, which do nothing unless you also use the
-p option? It adds ~230 lines of options that are irrelevant to the
most common usage model. It seems to me like the more correct fix
would be to move the diff options to later in the file (after the
options that are `git log`-specific), or to remove them entirely, and
replace them with a pointer to git diff's options.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply
* Re: [PATCH v6 3/4] reset: add a few tests for "git reset --merge"
From: Christian Couder @ 2010-01-02 5:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <7v7hs2o16j.fsf@alter.siamese.dyndns.org>
On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Commit 9e8eceab ("Add 'merge' mode to 'git reset'", 2008-12-01),
> > added the --merge option to git reset, but there were no test cases
> > for it.
> >
> > This was not a big problem because "git reset" was just forking and
> > execing "git read-tree", but this will change in a following patch.
> >
> > So let's add a few test cases to make sure that there will be no
> > regression.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> Looks good.
Thanks again.
> > +# The next test will test the following:
> > +#
> > +# working index HEAD target working index HEAD
> > +# ----------------------------------------------------
> > +# file1: C C C D --merge D D D
> > +# file2: C D D D --merge C D D
> > +test_expect_success 'reset --merge is ok with changes in file it does
> > not touch' ' + git reset --merge HEAD^ &&
> > + ! grep 4 file1 &&
> > + grep 4 file2 &&
> > + test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> > + test -z "$(git diff --cached)"
> > +'
> > ...
> > +# The next test will test the following:
> > +#
> > +# working index HEAD target working index HEAD
> > +# ----------------------------------------------------
> > +# file1: C C C D --merge D D D
> > +# file2: C C D D --merge D D D
> > +test_expect_success 'reset --merge discards changes added to index
> > (2)' ' + git reset --hard second &&
> > + echo "line 4" >> file2 &&
> > + git add file2 &&
> > + git reset --merge HEAD^ &&
> > + ! grep 4 file2 &&
> > + test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> > + test -z "$(git diff)" &&
> > + test -z "$(git diff --cached)"
> > +'
>
> These two seem to duplicate the same case for file1; is it necessary?
No. I think I just copied the previous test and added the "git add file2"
line.
> I am not pointing it out as something that needs to be removed; I am just
> puzzled and wondering if there is some interaction between the ways two
> paths are handled and the test is trying to check that (which I do not
> think is the case).
Best regards,
Christian.
^ permalink raw reply
* Re: [PATCH v6 2/4] Documentation: reset: add some tables to describe the different options
From: Christian Couder @ 2010-01-02 5:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <7vd41uo16x.fsf@alter.siamese.dyndns.org>
On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > This patch adds a DISCUSSION section that contains some tables to
> > show how the different "git reset" options work depending on the
> > states of the files in the working tree, the index, HEAD and the
> > target commit.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> Much nicer.
Thanks.
> > Documentation/git-reset.txt | 66
> > +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 66
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> > index 2d27e40..c9044c9 100644
> > --- a/Documentation/git-reset.txt
> > +++ b/Documentation/git-reset.txt
> > @@ -67,6 +67,72 @@ linkgit:git-add[1]).
> > <commit>::
> > Commit to make the current HEAD. If not given defaults to HEAD.
> >
> > +DISCUSSION
> > +----------
> > +
> > +The tables below show what happens when running:
> > +
> > +----------
> > +git reset --option target
> > +----------
> > +
> > +to reset the HEAD to another commit (`target`) with the different
> > +reset options depending on the state of the files.
>
> Together with these "mechanical definitions", I think the readers would
> benefit from reading why some are disallowed.
Ok, I will add some explanations along what you write below.
> > + working index HEAD target working index HEAD
> > + ----------------------------------------------------
> > + A B C D --soft A B D
> > + --mixed A D D
> > + --hard D D D
> > + --merge (disallowed)
>
> "reset --merge" is meant to be used when resetting out of a conflicted
> merge. Because any mergy operation guarantees that the work tree file
> that is involved in the merge does not have local change wrt the index
> before it starts, and that it writes the result out to the work tree, the
> fact that we see difference between the index and the HEAD and also
> between the index and the work tree means that we are not seeing a state
> that a mergy operation left after failing with a conflict. That is why
> we disallow --merge option in this case, and the next one.
>
> > + working index HEAD target working index HEAD
> > + ----------------------------------------------------
> > + A B C C --soft A B C
> > + --mixed A C C
> > + --hard C C C
> > + --merge (disallowed)
>
> The same as above, but you are resetting to the same commit.
>
> > + working index HEAD target working index HEAD
> > + ----------------------------------------------------
> > + B B C D --soft B B D
> > + --mixed B D D
> > + --hard D D D
> > + --merge D D D
> >
> > + working index HEAD target working index HEAD
> > + ----------------------------------------------------
> > + B B C C --soft B B C
> > + --mixed B C C
> > + --hard C C C
> > + --merge C C C
>
> As this table is not only about "--merge" but to explain "reset", I think
> other interesting cases should also be covered.
>
> w=A i=B H=B t=B
>
> This is "we had local change in the work tree that was unrelated to the
> merge", and "reset --merge" should be a no-op for this path.
>
> w=A i=B H=B t=C
>
> This "reset --merge" is like "using checkout to switch to a commit that
> has C but we have changes in the work tree", and should fail just like
> "checkout branch" in such a situation fails without "-m" option.
Ok I will add these cases too.
Thanks,
Christian.
^ permalink raw reply
* [RFC/PATCH 5/5] reset: disallow "reset --keep" outside a work tree
From: Christian Couder @ 2010-01-02 5:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>
It is safer and consistent with "--merge" and "--hard" resets to disallow
"git reset --keep" outside a work tree.
So let's just do that and add some tests while at it.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 2 +-
t/t7103-reset-bare.sh | 25 +++++++++++++++++--------
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index da61f20..52584af 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -319,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == NONE)
reset_type = MIXED; /* by default */
- if (reset_type == HARD || reset_type == MERGE)
+ if (reset_type != SOFT && reset_type != MIXED)
setup_work_tree();
if (reset_type == MIXED && is_bare_repository())
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index afb55b3..1eef93c 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -11,21 +11,26 @@ test_expect_success 'setup non-bare' '
git commit -a -m two
'
-test_expect_success 'hard reset requires a worktree' '
+test_expect_success '"hard" reset requires a worktree' '
(cd .git &&
test_must_fail git reset --hard)
'
-test_expect_success 'merge reset requires a worktree' '
+test_expect_success '"merge" reset requires a worktree' '
(cd .git &&
test_must_fail git reset --merge)
'
-test_expect_success 'mixed reset is ok' '
+test_expect_success '"keep" reset requires a worktree' '
+ (cd .git &&
+ test_must_fail git reset --keep)
+'
+
+test_expect_success '"mixed" reset is ok' '
(cd .git && git reset)
'
-test_expect_success 'soft reset is ok' '
+test_expect_success '"soft" reset is ok' '
(cd .git && git reset --soft)
'
@@ -40,19 +45,23 @@ test_expect_success 'setup bare' '
cd bare.git
'
-test_expect_success 'hard reset is not allowed in bare' '
+test_expect_success '"hard" reset is not allowed in bare' '
test_must_fail git reset --hard HEAD^
'
-test_expect_success 'merge reset is not allowed in bare' '
+test_expect_success '"merge" reset is not allowed in bare' '
test_must_fail git reset --merge HEAD^
'
-test_expect_success 'mixed reset is not allowed in bare' '
+test_expect_success '"keep" reset is not allowed in bare' '
+ test_must_fail git reset --keep HEAD^
+'
+
+test_expect_success '"mixed" reset is not allowed in bare' '
test_must_fail git reset --mixed HEAD^
'
-test_expect_success 'soft reset is allowed in bare' '
+test_expect_success '"soft" reset is allowed in bare' '
git reset --soft HEAD^ &&
test "`git show --pretty=format:%s | head -n 1`" = "one"
'
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2010-01-02 5:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>
The purpose of this new option is to discard some of the
last commits but to keep current changes in the work tree.
The use case is when you work on something and commit
that work. And then you work on something else that touches
other files, but you don't commit it yet. Then you realize
that what you commited when you worked on the first thing
is not good or belongs to another branch.
So you want to get rid of the previous commits (at least in
the current branch) but you want to make sure that you keep
the changes you have in the work tree. And you are pretty
sure that your changes are independent from what you
previously commited, so you don't want the reset to succeed
if the previous commits changed a file that you also
changed in your work tree.
The table below shows what happens when running
"git reset --option target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD) in the cases where "--merge" and "--keep" behave
differently.
working index HEAD target working index HEAD
----------------------------------------------------
A B C D --keep (disallowed)
--merge (disallowed)
A B C C --keep A C C
--merge (disallowed)
B B C D --keep (disallowed)
--merge D D D
B B C C --keep B C C
--merge C C C
In this table, A, B and C are some different states of
a file. For example the last line of the table means
that if a file is in state B in the working tree and
the index, and in a different state C in HEAD and in
the target, then "git reset --merge target" will put
the file in state C in the working tree, in the index
and in HEAD.
The following table shows what happens on unmerged entries:
working index HEAD target working index HEAD
----------------------------------------------------
X U A B --keep (disallowed)
--merge B B B
X U A A --keep X A A
--merge A A A
In this table X can be any state and U means an unmerged
entry.
Though the error message when "reset --keep" is disallowed
on unmerged entries is something like:
error: Entry 'file1' would be overwritten by merge. Cannot merge.
fatal: Could not reset index file to revision 'HEAD^'.
which is not very nice.
A following patch will add some test cases for
"--keep", where the differences between "--merge" and
"--keep" can also be seen.
The "--keep" option is implemented by doing a 2 way merge
between HEAD and the reset target, and if this succeeds
by doing a mixed reset to the target.
The code comes from the sequencer GSoC project, where
such an option was developed by Stephan Beyer:
git://repo.or.cz/git/sbeyer.git
(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
But in the sequencer project the "reset" flag was set
in the "struct unpack_trees_options" passed to
"unpack_trees()". With this flag the changes in the
working tree were discarded if the file was different
between HEAD and the reset target.
Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 29 ++++++++++++++++++++++++-----
1 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index 0f5022e..da61f20 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -22,13 +22,15 @@
#include "cache-tree.h"
static const char * const git_reset_usage[] = {
- "git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
+ "git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]",
"git reset [--mixed] <commit> [--] <paths>...",
NULL
};
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
-static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
+enum reset_type { MIXED, SOFT, HARD, MERGE, KEEP, NONE };
+static const char *reset_type_names[] = {
+ "mixed", "soft", "hard", "merge", "keep", NULL
+};
static char *args_to_str(const char **argv)
{
@@ -71,6 +73,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
if (!quiet)
opts.verbose_update = 1;
switch (reset_type) {
+ case KEEP:
case MERGE:
opts.update = 1;
break;
@@ -85,6 +88,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
read_cache_unmerged();
+ if (reset_type == KEEP) {
+ unsigned char head_sha1[20];
+ if (get_sha1("HEAD", head_sha1))
+ return error("You do not have a valid HEAD.");
+ if (!fill_tree_descriptor(desc, head_sha1))
+ return error("Failed to find tree of HEAD.");
+ nr++;
+ opts.fn = twoway_merge;
+ }
+
if (!fill_tree_descriptor(desc + nr - 1, sha1))
return error("Failed to find tree of %s.", sha1_to_hex(sha1));
if (unpack_trees(nr, desc, &opts))
@@ -229,6 +242,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
"reset HEAD, index and working tree", HARD),
OPT_SET_INT(0, "merge", &reset_type,
"reset HEAD, index and working tree", MERGE),
+ OPT_SET_INT(0, "keep", &reset_type,
+ "reset HEAD but keep local changes", KEEP),
OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
OPT_END()
};
@@ -317,9 +332,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == SOFT) {
if (is_merge() || read_cache() < 0 || unmerged_cache())
die("Cannot do a soft reset in the middle of a merge.");
+ } else {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP)
+ err = err || reset_index_file(sha1, MIXED, quiet);
+ if (err)
+ die("Could not reset index file to revision '%s'.", rev);
}
- else if (reset_index_file(sha1, reset_type, quiet))
- die("Could not reset index file to revision '%s'.", rev);
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2010-01-02 5:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>
and give an example to show how it can be used.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-reset.txt | 41 ++++++++++++++++++++++++++++++++++++++---
1 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index b78e2e1..3d7c206 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
--------
[verse]
-'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
+'git reset' [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]
'git reset' [-q] [<commit>] [--] <paths>...
'git reset' --patch [<commit>] [--] [<paths>...]
@@ -52,6 +52,11 @@ OPTIONS
and updates the files that are different between the named commit
and the current commit in the working tree.
+--keep::
+ Resets the index to match the tree recorded by the named commit,
+ but keep changes in the working tree. Aborts if the reset would
+ change files that are already changes in the working tree.
+
-p::
--patch::
Interactively select hunks in the difference between the index
@@ -86,6 +91,7 @@ reset options depending on the state of the files.
--mixed A D D
--hard D D D
--merge (disallowed)
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -93,6 +99,7 @@ reset options depending on the state of the files.
--mixed A C C
--hard C C C
--merge (disallowed)
+ --keep A C C
working index HEAD target working index HEAD
----------------------------------------------------
@@ -100,6 +107,7 @@ reset options depending on the state of the files.
--mixed B D D
--hard D D D
--merge D D D
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -107,13 +115,14 @@ reset options depending on the state of the files.
--mixed B C C
--hard C C C
--merge C C C
+ --keep B C C
In these tables, A, B, C and D are some different states of a
file. For example, the last line of the last table means that if a
file is in state B in the working tree and the index, and in a
different state C in HEAD and in the target, then "git reset
---merge target" will put the file in state C in the working tree,
-in the index and in HEAD.
+--keep target" will put the file in state B in the working tree,
+and in state C in the index and in HEAD.
The following tables show what happens when there are unmerged
entries:
@@ -124,6 +133,7 @@ entries:
--mixed X B B
--hard B B B
--merge B B B
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -131,6 +141,7 @@ entries:
--mixed X A A
--hard A A A
--merge A A A
+ --keep X A A
X means any state and U means an unmerged index.
@@ -302,6 +313,30 @@ $ git add frotz.c <3>
<2> This commits all other changes in the index.
<3> Adds the file to the index again.
+Keep changes in working tree while discarding some previous commits::
++
+Suppose you are working on something and you commit it, and then you
+continue working a bit more, but now you think that what you have in
+your working tree should be in another branch that has nothing to do
+with what you commited previously. You can start a new branch and
+reset it while keeping the changes in your work tree.
++
+------------
+$ git tag start
+$ git branch branch1
+$ edit
+$ git commit ... <1>
+$ edit
+$ git branch branch2 <2>
+$ git reset --keep start <3>
+------------
++
+<1> This commits your first edits in branch1.
+<2> This creates branch2, but unfortunately it contains the previous
+commit that you don't want in this branch.
+<3> This removes the unwanted previous commit, but this keeps the
+changes in your working tree.
+
Author
------
Written by Junio C Hamano <gitster@pobox.com> and Linus Torvalds <torvalds@osdl.org>
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
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