* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: Jeff King @ 2009-12-08 5:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: James Vega, git
In-Reply-To: <7v3a3mqhhd.fsf@alter.siamese.dyndns.org>
On Mon, Dec 07, 2009 at 07:20:30PM -0800, Junio C Hamano wrote:
> An update. I tried your reproduction recipe with 1.6.5.2 and it doesn't
> reproduce, but with 1.6.5.3 it does.
Thanks, both, for a very helpful bug report. 24ab81a was totally bogus,
but we lacked a test for deleting a non-empty file. That test and a fix
for the problem are in the patch below.
I am still slightly concerned that James's
git diff | sed '/^deleted file/d' | git apply --cached
behaves as it does. What should git-apply do with a patch like:
diff --git a/foo b/foo
index 257cc56..0000000
--- a/foo
+++ /dev/null
@@ -1 +0,0 @@
-foo
? I can see either turning it into a deletion patch (because /dev/null
is special) or barfing (because /dev/null as a special case should have
appeared in the "diff" line). But creating a dev/null file seems very
wrong.
But maybe it is not worth worrying about too much. That patch format is
not generated intentionally by any known software.
Here is the fix directly on top of 24ab81a.
-- >8 --
Subject: [PATCH] add-interactive: fix deletion of non-empty files
Commit 24ab81a fixed the deletion of empty files, but broke
deletion of non-empty files. The approach it took was to
factor out the "deleted" line from the patch header into its
own hunk, the same way we do for mode changes. However,
unlike mode changes, we only showed the special "delete this
file" hunk if there were no other hunks. Otherwise, the user
would annoyingly be presented with _two_ hunks: one for
deleting the file and one for deleting the content.
Instead, this patch takes a separate approach. We leave the
deletion line in the header, so it will be used as usual by
non-empty files if their deletion hunk is staged. For empty
files, we create a deletion hunk with no content; it doesn't
add anything to the patch, but by staging it we trigger the
application of the header, which does contain the deletion.
Signed-off-by: Jeff King <peff@peff.net>
---
There is a slightly different approach we could take, too: keep the
"deletion" hunk as a first-class hunk, and just meld the content hunk's
output into it. Then both cases would get the "Stage deletion" question
instead of the "Stage this hunk" you get now for non-empty files (which
just happens to trigger a deletion due to the headers).
That would take some refactoring, though, as pulling the deletion hunk
out means we are re-ordering the headers. So right now if you did that
your ($head, @hunk) output would be something like:
diff --git a/foo b/foo
index 257cc56..0000000
--- a/foo
+++ /dev/null
deleted file mode 100644
@@ -1 +0,0 @@
-foo
which is pretty weird. On the other hand, we already do that funny
ordering for mode hunks, and git-apply is just fine with it. A mode hunk
with content change looks like this:
diff --git a/foo b/foo
index 257cc56..19c6cc1
--- a/foo
+++ b/foo
old mode 100644
new mode 100755
And it also opens the door to editing the hunk to stop the deletion, but
still tweak the content change. Right now if you edit a deletion patch,
you can't remove the 'deleted' bit, and if your edit result keeps any
content in the file, apply will complain. I'm not sure that particular
feature would be useful though (I have certainly never wanted it).
git-add--interactive.perl | 18 +++++++++++-------
t/t3701-add-interactive.sh | 20 ++++++++++++++++++++
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 35f4ef1..f4b95b1 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -731,17 +731,19 @@ sub parse_diff_header {
my $head = { TEXT => [], DISPLAY => [], TYPE => 'header' };
my $mode = { TEXT => [], DISPLAY => [], TYPE => 'mode' };
- my $deletion = { TEXT => [], DISPLAY => [], TYPE => 'deletion' };
+ my $is_deletion;
for (my $i = 0; $i < @{$src->{TEXT}}; $i++) {
my $dest =
$src->{TEXT}->[$i] =~ /^(old|new) mode (\d+)$/ ? $mode :
- $src->{TEXT}->[$i] =~ /^deleted file/ ? $deletion :
$head;
push @{$dest->{TEXT}}, $src->{TEXT}->[$i];
push @{$dest->{DISPLAY}}, $src->{DISPLAY}->[$i];
+ if ($src->{TEXT}->[$i] =~ /^deleted file/) {
+ $is_deletion = 1;
+ }
}
- return ($head, $mode, $deletion);
+ return ($head, $mode, $is_deletion);
}
sub hunk_splittable {
@@ -1209,7 +1211,7 @@ sub patch_update_file {
my ($ix, $num);
my $path = shift;
my ($head, @hunk) = parse_diff($path);
- ($head, my $mode, my $deletion) = parse_diff_header($head);
+ ($head, my $mode, my $is_deletion) = parse_diff_header($head);
for (@{$head->{DISPLAY}}) {
print;
}
@@ -1217,8 +1219,8 @@ sub patch_update_file {
if (@{$mode->{TEXT}}) {
unshift @hunk, $mode;
}
- if (@{$deletion->{TEXT}} && !@hunk) {
- @hunk = ($deletion);
+ if ($is_deletion && !@hunk) {
+ @hunk = ({TEXT => [], DISPLAY => [], TYPE => 'deletion'});
}
$num = scalar @hunk;
@@ -1441,14 +1443,16 @@ sub patch_update_file {
@hunk = coalesce_overlapping_hunks(@hunk);
my $n_lofs = 0;
+ my $hunks_used = 0;
my @result = ();
for (@hunk) {
if ($_->{USE}) {
push @result, @{$_->{TEXT}};
+ $hunks_used++;
}
}
- if (@result) {
+ if ($hunks_used) {
my $fh;
my @patch = (@{$head->{TEXT}}, @result);
my $apply_routine = $patch_mode_flavour{APPLY};
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index aa5909b..0926b91 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -215,6 +215,26 @@ test_expect_success 'add first line works' '
'
cat >expected <<EOF
+diff --git a/non-empty b/non-empty
+deleted file mode 100644
+index d95f3ad..0000000
+--- a/non-empty
++++ /dev/null
+@@ -1 +0,0 @@
+-content
+EOF
+test_expect_success 'deleting a non-empty file' '
+ git reset --hard &&
+ echo content >non-empty &&
+ git add non-empty &&
+ git commit -m non-empty &&
+ rm non-empty &&
+ echo y | git add -p non-empty &&
+ git diff --cached >diff &&
+ test_cmp expected diff
+'
+
+cat >expected <<EOF
diff --git a/empty b/empty
deleted file mode 100644
index e69de29..0000000
--
1.6.5.1.g24ab.dirty
^ permalink raw reply related
* Re: [PATCH] git svn: Don't create empty directories whose parents were deleted
From: Eric Wong @ 2009-12-08 4:59 UTC (permalink / raw)
To: Greg Price, Junio C Hamano; +Cc: git, Alex Vandiver
In-Reply-To: <20091208032831.GL30538@dr-wily.mit.edu>
Greg Price <price@ksplice.com> wrote:
> This is a regression in v1.6.6-rc0, so it would be good to fix before v1.6.6.
Thanks Greg,
I found another git svn bug (fixed in a patch below) while writing a
test case for this.
Junio:
The following are pushed out to git://git.bogomips.org/git-svn and
should be ready for v1.6.6:
Alex Vandiver (1):
git-svn: sort svk merge tickets to account for minimal parents
Eric Wong (1):
git svn: log removals of empty directories
Greg Price (1):
git svn: Don't create empty directories whose parents were deleted
Thanks all
>From f9ad77a739c0d012ee58b64eda2d7ec0d4e1df9d Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 7 Dec 2009 20:49:38 -0800
Subject: [PATCH] git svn: log removals of empty directories
This also adds a test case for:
"git svn: Don't create empty directories whose parents were deleted"
which was the reason we found this bug in the first place.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 2 +-
t/t9146-git-svn-empty-dirs.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index bdd1f96..5a52068 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3891,11 +3891,11 @@ sub delete_entry {
}
print "\tD\t$gpath/\n" unless $::_q;
command_close_pipe($ls, $ctx);
- $self->{empty}->{$path} = 0
} else {
$self->{gii}->remove($gpath);
print "\tD\t$gpath\n" unless $::_q;
}
+ $self->{empty}->{$path} = 0;
undef;
}
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 70c52c1..9b8d046 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -105,4 +105,14 @@ test_expect_success 'empty directories in trunk exist' '
)
'
+test_expect_success 'remove a top-level directory from svn' '
+ svn_cmd rm -m "remove d" "$svnrepo"/d
+'
+
+test_expect_success 'removed top-level directory does not exist' '
+ git svn clone "$svnrepo" removed &&
+ test ! -e removed/d
+
+'
+
test_done
--
Eric Wong
^ permalink raw reply related
* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: James Vega @ 2009-12-08 3:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4wyqigf.fsf@alter.siamese.dyndns.org>
On Mon, Dec 07, 2009 at 06:59:28PM -0800, Junio C Hamano wrote:
> James Vega <vega.james@gmail.com> writes:
>
> > It looks like this may have introduced a bug when staging a file
> > removal. Here's an example git session showing the issue:
> > <<snipped>>
>
> Thanks for a report, but I cannot get the evidence that the said patch has
> anything to do with the issue you illustrated.
Right, I incorrectly assumed the problem was with git-apply when I saw
Steve's patch since the symptoms seemed similar.
I just finished a bisect, though, and the problem is in removing a
non-empty file with "git add -p". This wasn't caught by existing tests
because they only try to remove an empty file.
This was introduced in
8f0bef6 (git-apply--interactive: Refactor patch mode code, 2009-08-13)
--
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <vega.james@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Junio C Hamano @ 2009-12-08 3:28 UTC (permalink / raw)
To: Nanako Shiraishi
Cc: Johannes Schindelin, Matthieu Moy, Michael J Gruber,
Michael Haggerty, git
In-Reply-To: <20091208121314.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Teach a new option, --autosquash, to the interactive rebase.
> When the commit log message begins with "!fixup ...", and there
> is a commit whose title begins with the same ..., automatically
> modify the todo list of rebase -i so that the commit marked for
> squashing come right after the commit to be modified, and change
> the action of the moved commit from pick to squash.
>
> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
Hmph, did you forget to retitle the message, or keep in-body "Subject:"?
^ permalink raw reply
* [PATCH] git svn: Don't create empty directories whose parents were deleted
From: Greg Price @ 2009-12-08 3:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
Commit 6111b93 "git svn: attempt to create empty dirs on clone+rebase"
will create empty directories 'a/b' and 'a/c' if they were previously
created in SVN, even if their parent directory 'a' was deleted.
For example, unhandled.log may contain lines like this:
r32
+empty_dir: packages/sipb-xen-remctl-auto/sipb-xen-remctl-auto/files/etc/remctl/sipb-xen-auto/acl
+empty_dir: packages/sipb-xen-remctl-auto/sipb-xen-remctl-auto/files/etc/remctl/sipb-xen-auto/machine.d
+empty_dir: packages/sipb-xen-remctl-auto/sipb-xen-remctl-auto/files/etc/remctl/sipb-xen-auto/moira-acl
[...]
r314
-empty_dir: packages/sipb-xen-remctl-auto
Reported-by: Evan Broder <broder@mit.edu>
Signed-off-by: Greg Price <price@ksplice.com>
---
This is a regression in v1.6.6-rc0, so it would be good to fix before v1.6.6.
git-svn.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 957d44e..5c35494 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2748,7 +2748,7 @@ sub mkemptydirs {
} elsif (/^ \+empty_dir: (.+)$/) {
$empty_dirs{$1} = 1;
} elsif (/^ \-empty_dir: (.+)$/) {
- delete $empty_dirs{$1};
+ delete @empty_dirs{grep {m[^\Q$1\E(/|$)]} (keys %empty_dirs)};
}
}
close $fh;
--
1.6.4.4
^ permalink raw reply related
* Re: [BUG] git config does not reuse section name
From: Junio C Hamano @ 2009-12-08 3:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Yakup Akbay, git
In-Reply-To: <alpine.DEB.1.00.0912080258010.4985@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> IIRC, due to technical limitations, the config machinery only recognizes
> sections if there is at least _one_ entry in them. This is because
> git_config() is used to determine (from the current file position) where
> the section begins.
Ah, the reading side should be able to get by with that parsing logic, as
an empty section is totally ignorable anyway. And the parser for the
writing side reuses that logic. Asking for removal code to notice the
empty section and remove it needs a bit of restructuring of the parsing
logic as currently it doesn't even see an empty section.
Thanks---the explanation makes sense (I am not saying "it justifies it"; I
only mean "it explains why the code behaves like that very well").
I think we have kept the original parsing structure since repo-config
added the write support, and "fixing" the issue is not that urgent, but it
would be nice to get it fixed. Perhaps somebody can find some time over
the upcoming holidays ;-)
^ permalink raw reply
* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: Junio C Hamano @ 2009-12-08 3:20 UTC (permalink / raw)
To: James Vega; +Cc: git, Jeff King
In-Reply-To: <7vk4wyqigf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> James Vega <vega.james@gmail.com> writes:
>
>> It looks like this may have introduced a bug when staging a file
>> removal. Here's an example git session showing the issue:
An update. I tried your reproduction recipe with 1.6.5.2 and it doesn't
reproduce, but with 1.6.5.3 it does.
$ git init test
Initialized empty Git repository in /local_disk/tmp/test/.git/
$ cd test
$ echo "foo" > foo
$ git add foo
$ git commit -m 'Add foo'
[master (root-commit) 3643b5d] Add foo
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 foo
$ mv foo bar
$ git add -p
diff --git a/foo b/foo
index 257cc56..0000000
--- a/foo
+++ /dev/null
@@ -1 +0,0 @@
-foo
Stage this hunk [y,n,q,a,d,/,e,?]? y
$ git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD ..." to unstage)
#
# new file: dev/null
# deleted: foo
#
A quick bisection of the original issue points at
24ab81a (add-interactive: handle deletion of empty files, 2009-10-27)
^ permalink raw reply related
* Re: [RFC/PATCHv10 08/11] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-12-08 3:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, spearce
In-Reply-To: <7vk4wy1p8d.fsf@alter.siamese.dyndns.org>
On Monday 07 December 2009, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > Created by a simple cleanup and rename of lookup_notes().
> >
> > Signed-off-by: Johan Herland <johan@herland.net>
> > ---
> > notes.c | 15 ++++++++-------
> > notes.h | 3 +++
> > 2 files changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/notes.c b/notes.c
> > index 79bfa24..110404a 100644
> > --- a/notes.c
> > +++ b/notes.c
> > @@ -379,12 +379,13 @@ void add_note(const unsigned char *object_sha1,
> > const unsigned char *note_sha1) note_tree_insert(&root_node, 0, l,
> > PTR_TYPE_NOTE);
> > }
> >
> > -static unsigned char *lookup_notes(const unsigned char *object_sha1)
> > +const unsigned char *get_note(const unsigned char *object_sha1)
>
> Is there a need to find "note for this commit in the set of notes 3 days
> ago"? IOW, reading note for the given commit not from the tip of the
> history of the refs/notes/commits but from say refs/notes/commits~4?
> Similarly, is there a need to ask for a history of notes for a given
> commit, something like "git log refs/notes/commit/$this_commit" in a
> world without any fanout?
>
> Obviously, "there is no need because..." is the best answer I'd be happy
> with. "There may be in the future but we haven't identified a good use
> case and we don't implement what we do not need now." is also perfectly
> acceptable.
There may be in the future but we haven't identified a good use case and we
don't implement what we do not need now.
;)
> IOW, I am not suggesting to change it---I just want to know how much
> thought went in before deciding to implement the interface this way.
Well, this later part of the series (from patch #6) was built mainly to
support the fast-import patch (which is no longer based on this API), but
also with an eye towards keeping things fairly flexible and generic.
Furthermore I expect to use most of these patches when I get around to
builtin-ifying the git-notes shell script (which currently is oblivious the
notes API and things like fanout and rebalancing).
In any case, if you look at patch #10/11, you'll see I introduce the concept
of multiple notes trees. This was originally done to allow fast-import to
edit notes in several branches simultaneously, but it now occurs to me that
this is exactly what we need to answer your questions above: If you want to
look at an older version of your notes tree, you simply instantiate another
notes tree with:
struct notes_tree my_notes;
init_notes(my_notes, "refs/notes/commits~4", ...);
and you can now compare notes between my_notes and the current (or any
other) notes tree.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive", [PATCH] rebase -i --autosquash: auto-squash commits
From: Nanako Shiraishi @ 2009-12-08 3:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nanako Shiraishi, Johannes Schindelin, Matthieu Moy,
Michael J Gruber, Michael Haggerty, git
In-Reply-To: <7vd42t6f9i.fsf@alter.siamese.dyndns.org>
Teach a new option, --autosquash, to the interactive rebase.
When the commit log message begins with "!fixup ...", and there
is a commit whose title begins with the same ..., automatically
modify the todo list of rebase -i so that the commit marked for
squashing come right after the commit to be modified, and change
the action of the moved commit from pick to squash.
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> If Michael rolls his second round with your "--autosquash", or you do so
> yourself on top of his patch, I think it _might_ be safer to mark the ones
> automatically moved as "squash", and not as "fix", and have the users
> explicitly change the "squash" they want to "fix" themselves.
> Alternatively, you can also use two magic tokens (i.e. instead of one
> "fixup!", allow people to use "squash!" and "fixup!") and change the
> action chosen for the moved commits to "squash" and "fixup" respectively.
Here is a rebased and updated version of my patch from June
2009. It should apply cleanly on top of Michael's patch.
Documentation/git-rebase.txt | 10 +++++++
git-rebase--interactive.sh | 43 +++++++++++++++++++++++++++++++
t/t3415-rebase-autosquash.sh | 58 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+), 0 deletions(-)
create mode 100755 t/t3415-rebase-autosquash.sh
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9b648ec..87cb62d 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -308,6 +308,16 @@ which makes little sense.
root commits will be rewritten to have <newbase> as parent
instead.
+--autosquash::
+ When the commit log message begins with "!squash ..." (or
+ "!fixup ..."), and there is a commit whose title begins with
+ the same ..., automatically modify the todo list of rebase -i
+ so that the commit marked for quashing come right after the
+ commit to be modified, and change the action of the moved
+ commit from `pick` to `squash` (or `fixup`).
++
+This option is only valid when '--interactive' option is used.
+
include::merge-strategies.txt[]
NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 30de96e..b014231 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,6 +28,7 @@ abort abort rebasing process and restore original branch
skip skip current patch and continue rebasing process
no-verify override pre-rebase hook from stopping the operation
root rebase all reachable commmits up to the root(s)
+autosquash move commits that begin with !squash/!fixup
"
. git-sh-setup
@@ -46,6 +47,7 @@ ONTO=
VERBOSE=
OK_TO_SKIP_PRE_REBASE=
REBASE_ROOT=
+AUTOSQUASH=
GIT_CHERRY_PICK_HELP=" After resolving the conflicts,
mark the corrected paths with 'git add <paths>', and
@@ -519,6 +521,43 @@ get_saved_options () {
test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
}
+# Rearrange the todo list that has both "pick sha1 msg" and
+# "pick sha1 !fixup/!squash msg" appears in it so that the latter
+# comes immediately after the former, and change "pick" to
+# "fixup"/"squash".
+rearrange_squash () {
+ sed -n -e 's/^pick \([0-9a-f]*\) !\(squash\) /\1 \2 /p' \
+ -e 's/^pick \([0-9a-f]*\) !\(fixup\) /\1 \2 /p' \
+ "$1" >"$1.sq"
+ test -s "$1.sq" || return
+
+ sed -e '/^pick [0-9a-f]* !squash /d' \
+ -e '/^pick [0-9a-f]* !fixup /d' \
+ "$1" |
+ (
+ used=
+ while read pick sha1 message
+ do
+ echo "$pick $sha1 $message"
+ while read squash action msg
+ do
+ case " $used" in
+ *" $squash "*)
+ continue ;;
+ esac
+ case "$message" in
+ "$msg"*)
+ echo "$action $squash !$action $msg"
+ used="$used$squash "
+ ;;
+ esac
+ done <"$1.sq"
+ done >"$1.rearranged"
+ )
+ cat "$1.rearranged" >"$1"
+ rm -f "$1.sq"
+}
+
while test $# != 0
do
case "$1" in
@@ -624,6 +663,9 @@ first and then run 'git rebase --continue' again."
--root)
REBASE_ROOT=t
;;
+ --autosquash)
+ AUTOSQUASH=t
+ ;;
--onto)
shift
ONTO=$(git rev-parse --verify "$1") ||
@@ -783,6 +825,7 @@ first and then run 'git rebase --continue' again."
fi
test -s "$TODO" || echo noop >> "$TODO"
+ test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
cat >> "$TODO" << EOF
# Rebase $SHORTREVISIONS onto $SHORTONTO
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
new file mode 100755
index 0000000..5ea2073
--- /dev/null
+++ b/t/t3415-rebase-autosquash.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='auto squash'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo 0 > file0 &&
+ git add . &&
+ test_tick &&
+ git commit -m "initial commit" &&
+ echo 0 > file1 &&
+ echo 2 > file2 &&
+ git add . &&
+ test_tick &&
+ git commit -m "first commit" &&
+ echo 3 > file3 &&
+ git add . &&
+ test_tick &&
+ git commit -m "second commit" &&
+ git tag base
+'
+
+test_expect_success 'auto fixup' '
+ git reset --hard base &&
+ echo 1 > file1 &&
+ git add -u &&
+ test_tick &&
+ git commit -m "!fixup first"
+
+ git tag final-fixup &&
+ test_tick &&
+ git rebase --autosquash -i HEAD^^^ &&
+ git log --oneline >actual &&
+ test 3 = $(wc -l <actual) &&
+ git diff --exit-code final-fixup &&
+ test 1 = "$(git cat-file blob HEAD^:file1)" &&
+ test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
+'
+
+test_expect_success 'auto squash' '
+ git reset --hard base &&
+ echo 1 > file1 &&
+ git add -u &&
+ test_tick &&
+ git commit -m "!squash first"
+
+ git tag final-squash &&
+ test_tick &&
+ git rebase --autosquash -i HEAD^^^ &&
+ git log --oneline >actual &&
+ test 3 = $(wc -l <actual) &&
+ git diff --exit-code final-squash &&
+ test 1 = "$(git cat-file blob HEAD^:file1)" &&
+ test 2 = $(git cat-file commit HEAD^ | grep first | wc -l)
+'
+
+test_done
--
1.6.6.rc0.60.g4926
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: Junio C Hamano @ 2009-12-08 2:59 UTC (permalink / raw)
To: James Vega; +Cc: git
In-Reply-To: <loom.20091207T222449-752@post.gmane.org>
James Vega <vega.james@gmail.com> writes:
> It looks like this may have introduced a bug when staging a file
> removal. Here's an example git session showing the issue:
> <<snipped>>
Thanks for a report, but I cannot get the evidence that the said patch has
anything to do with the issue you illustrated.
$ cat >patch0 <<\EOF
diff --git a/foo b/foo
deleted file mode 100644
index 257cc56..0000000
--- a/foo
+++ /dev/null
@@ -1 +0,0 @@
-foo
EOF
$ git apply --numstat patch0
0 1 foo
$ sed -e '/deleted file/d' patch0 | git apply --numstat
0 1 dev/null
The last one is showing the symptom in your message. Git versions 1.4.0
and newer yield the same result, but 1.3.0 gives a funny message:
** warning: file dev/null becomes empty but is not deleted
0 1 foo
So it appears that the bug is somewhere else not in that patch.
^ permalink raw reply related
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-08 2:45 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20091208020134.GC17588@spearce.org>
On Tuesday 08 December 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > > If we're here, isn't it likely that *all* notes are in the wrong
> > > path in the tree, and we need to move them all to a new location?
> > > If that's true then should we instead just build an entirely new
> > > tree and swap the root when we are done?
> >
> > Hmm. Not always. In your earlier scenario where we add 2,000,000 notes
> > in a single commit, the current code would need to rewrite 255 of them
> > from fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout
> > 2. But the vast majority (1,934,465) would not require rewriting
> > (having been added at the correct fanout initially). However, if we
> > build a new tree (by which I assume you mean tree_content_remove() from
> > the old tree and
> > tree_content_set() to the new tree for every single note (and
> > non-note)), we end up processing all 2,000,000 entries.
>
> Well, by processing here you mean we wind up looking at them, only
> to determine they are in the correct place already and skipping past.
No, (as far as I (mis?)understood your idea) by processing here I'm talking
about moving all 2,000,000 entries from the old tree to the new tree.
Here's my understanding of your idea:
- Create a new, empty tree
- For each entry in the old/existing tree:
- If not a note, move[*] verbatim to new tree
- If a correctly placed note, move[*] verbatim to new tree
- Else, move[*] note to the _correct_ place in the new tree
[*]: By "move" I assume you mean tree_content_remove() from the old tree,
followed by tree_content_set() into the new tree.
>From this understanding, I cannot see how your idea improves on the
adding-2M-notes scenario.
> I guess I see your point though. We're fairly bounded on how many
> we might need to move, probably only 65,535, and the rest will be
> at the right position so we're mostly just iterating through to
> confirm they don't have to be moved.
Yep.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-08 2:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, spearce
In-Reply-To: <7vocma1ppc.fsf@alter.siamese.dyndns.org>
On Monday 07 December 2009, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > This patch teaches 'git fast-import' to automatically organize note
> > objects in a fast-import stream into an appropriate fanout structure.
>
> I really hate to sound like a clueless newbie, but that is what I am in
> the area of 'notes', so I have two questions.
>
> - What is the semantics of having more than one note to the same commit
> in the input stream? Does the 'notes' part also have history and the
> latest one overwrite the earlier one by creating a new commit that
> points at the new 'notes' tree?
Yes.
> I've always thought of 'notes' as an
> unversioned metainfo, but I realize that versioning them would make
> sense (you can and obviously would want to inspect the story behind
> the current note attached to one particular commit).
Correct. Since the notes themselves are organized in a regular ref pointing
to a series of commits, the notes for a particular object are indeed
versioned. Thus, the first annotation of a commit will happen as part of a
commit to the notes ref at some point in time, and a change to that
annotation will happen as part of a subsequent commit to the same notes ref
at some later point in time. The latter annotation naturally replaces the
former, in the same way as a regular file change causes a new blob to
replace any blob representing the previous version of the same file.
However, if some object is annotated _twice_ in the _same_ notes commit,
then only the last annotation will be reachable. (again, this is the same
behaviour as if a regular file is changed twice in the same commit).
> - If however 'notes' want to have a history, how would it interact with
> this rebalancing of the tree? Rebalancing makes a lot of sense if the
> 'notes' mechanism deals with the only one latest version because it
> can keep the optimal look-up performance. There were some talks about
> specialized merge strategies that can be made aware of rebalancing, but
> is there a plan to deal with "git log -p notes" side, and how?
For now (at least), most use cases concern themselves only with the last
version of the notes tree, hence no work has been put into prettifying the
history of the notes tree.
The notes rebalancing will become part of the same notes commit as the note
addition/removal that triggers the rebalancing. This does indeed make the
notes commits themselves somewhat uglier, but since the rebalancing only
moves notes verbatim from one location to another, it's still fairly simple
(with judicious use of e.g. "-M") to find the "actual" changes in a notes
commit.
For now, there is no plan to prettify the log of a notes ref, in order to
mask away the fanout restructuring. For that matter, there is also no plan
to hide the fanout structure itself of the notes tree. It is assumed that if
you need to look at a notes tree directly, you can either deal with the
implementation details yourself, (or by using future extensions to the notes
API; see later patches for the beginnings of those...).
With regards to specialized merge strategies: When merging two notes trees
with no specialized strategy, you might end up with two (or more) notes
objects annotating the _same_ commit (located at different fanout levels).
However, this has already been taken care of by the concatenation code at
the tail of the already-merged early part of jh/notes, which automatically
concatenates (non-identical) note objects annotating the same commit. Thus,
no special merge strategy is needed in order to administer notes trees.
Hope this helps,
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-08 2:01 UTC (permalink / raw)
To: Johan Herland; +Cc: git, gitster
In-Reply-To: <200912080244.30390.johan@herland.net>
Johan Herland <johan@herland.net> wrote:
> > If we're here, isn't it likely that *all* notes are in the wrong
> > path in the tree, and we need to move them all to a new location?
> > If that's true then should we instead just build an entirely new
> > tree and swap the root when we are done?
>
> Hmm. Not always. In your earlier scenario where we add 2,000,000 notes in a
> single commit, the current code would need to rewrite 255 of them from
> fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout 2. But the
> vast majority (1,934,465) would not require rewriting (having been added at
> the correct fanout initially). However, if we build a new tree (by which I
> assume you mean tree_content_remove() from the old tree and
> tree_content_set() to the new tree for every single note (and non-note)), we
> end up processing all 2,000,000 entries.
Well, by processing here you mean we wind up looking at them, only
to determine they are in the correct place already and skipping past.
I guess I see your point though. We're fairly bounded on how many
we might need to move, probably only 65,535, and the rest will be
at the right position so we're mostly just iterating through to
confirm they don't have to be moved.
> I'm not sure I get the details here. How can we avoid doing the
> _remove()/_set() from/to the old/new tree for every tree_entry? In other
> words, how do we avoid removing and re-setting the 2,000,000 notes in the
> above example?
You can't. But I realize now what you are saying... for the vast
majority of the notes we only need to validate they are in the
correct path.
--
Shawn.
^ permalink raw reply
* Re: [BUG] git config does not reuse section name
From: Johannes Schindelin @ 2009-12-08 2:05 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, Yakup Akbay, git
In-Reply-To: <fabb9a1e0912071223l21c70e2ax9b0e3c9976ae9d7@mail.gmail.com>
Hi,
On Mon, 7 Dec 2009, Sverre Rabbelier wrote:
> On Mon, Dec 7, 2009 at 21:04, Junio C Hamano <gitster@pobox.com> wrote:
> > If I recall correctly, this hasn't been even
> > noticed/reported/recognized as an issue, ever since the "git
> > repo-config" was introduced (which later was renamed to "git config").
>
> I poked Dscho about it at some point.
>
> > Dscho, do you remember details?
>
> He told me that the 'git config' code is so horrible that it's
> nigh-impossible to change the behavior, hence why he didn't do it :P.
Actually, I said something about the most obvious route being to re-use
git_config() and that this approach had its limitations.
I also said that I earned (probably rightfully) a reputation of lousy code
with Junio, which makes me think that I probably should refrain from ever
contributing code to Git again.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Shawn O. Pearce @ 2009-12-08 1:59 UTC (permalink / raw)
To: Johan Herland; +Cc: git, gitster
In-Reply-To: <200912080255.17568.johan@herland.net>
Johan Herland <johan@herland.net> wrote:
> On Monday 07 December 2009, Shawn O. Pearce wrote:
> > > + if (fanout >= 20)
> > > + die("Too large fanout (%u)", fanout);
> >
> > Shouldn't convert_num_notes_to_fanout have a guard to prevent this
> > case from happening?
>
> Well, it sort of already does (unless uintmax_t is more than 19 * 8 = 152
> bits wide... ;)
Oh, good point. :-)
> Not sure what you're getting at:
>
> - Should I add a "&& fanout < 19" condition to the while loop in
> convert_num_notes_to_fanout()?
That's what I was thinking. But given the 19 * 8 = 152 case above,
this is pointless.
--
Shawn.
^ permalink raw reply
* Re: [BUG] git config does not reuse section name
From: Johannes Schindelin @ 2009-12-08 2:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Yakup Akbay, git
In-Reply-To: <7vy6le35zv.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 7 Dec 2009, Junio C Hamano wrote:
> Yakup Akbay <yakbay@ubicom.com> writes:
>
> > When I repeat the following n times
> >
> > $ git config color.ui always
> > $ git config --unset color.ui
> >
> >
> > it ends up the section name [color] n times in the .git/config file.
> >
> >
> >
> > like this for n=4:
> >
> > [color]
> > [color]
> > [color]
> > [color]
> >
> >
> > Using git version 1.6.5.3 (I don't know whether this is already fixed
> > in in later versions)
>
> If I recall correctly, this hasn't been even noticed/reported/recognized
> as an issue, ever since the "git repo-config" was introduced (which later
> was renamed to "git config"). Dscho, do you remember details?
IIRC, due to technical limitations, the config machinery only recognizes
sections if there is at least _one_ entry in them. This is because
git_config() is used to determine (from the current file position) where
the section begins.
And likewise, due to those technical limitations, the section header is
not removed when the last entry in the section is removed (this was
because I did not want to change the location of the section, but due to
the mentioned limitation, that did not work out).
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-08 1:55 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20091207164311.GE17173@spearce.org>
On Monday 07 December 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > +static unsigned char convert_num_notes_to_fanout(uintmax_t num_notes)
> > +{
> > + unsigned char fanout = 0;
> > + while ((num_notes >>= 8))
> > + fanout++;
> > + return fanout;
> > +}
> > +
> > +static void construct_path_with_fanout(const char *hex_sha1,
> > + unsigned char fanout, char *path)
> > +{
> > + unsigned int i = 0, j = 0;
> > + if (fanout >= 20)
> > + die("Too large fanout (%u)", fanout);
>
> Shouldn't convert_num_notes_to_fanout have a guard to prevent this
> case from happening?
Well, it sort of already does (unless uintmax_t is more than 19 * 8 = 152
bits wide... ;)
Not sure what you're getting at:
- Should I add a "&& fanout < 19" condition to the while loop in
convert_num_notes_to_fanout()?
- Should I remove the "if (fanout >= 20) die(...)"? Of course,
construct_path_with_fanout() is only supposed to be called with values
returned from convert_num_notes_to_fanout(), so the condition only tests a
precondition that we believe to be true (FTR, it was converted from an
equivalent assert() in an earlier iteration), but I normally test for these
things anyway (when they are not blindingly obvious), just to make sure...
(and I believe a die(...) is kinder to the user than a segfault...)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-08 1:44 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20091207164130.GD17173@spearce.org>
On Monday 07 December 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > +static uintmax_t do_change_note_fanout(
> > + struct tree_entry *orig_root, struct tree_entry *root,
> > + char *hex_sha1, unsigned int hex_sha1_len,
> > + char *fullpath, unsigned int fullpath_len,
> > + unsigned char fanout)
>
> I think this function winds up processing all notes twice. Yuck.
>
> tree_content_set() adds a new tree entry to the end of the current
> tree. So when converting "1a9029b006484e8b9aca06ff261beb2324bb9916"
> into "1a" (to go from fanout 0 to fanout 1) we'll place 1a at the
> end of orig_root, and this function will walk 1a/ recursively,
> examining 1a9029b006484e8b9aca06ff261beb2324bb9916 all over again.
Yep, you're right. Still, we only do the tree_content_remove()/set() once
per note, so although performance is probably not abysmal, we are still
clearly suboptimal.
Also, keep in mind that change_note_fanout() is only called when the number
of notes crosses a power of 256. Thus for typical notes trees (which are
assumed to mostly accumulate notes over their lifetime),
change_note_fanout() will be called zero, one or two times (depending on the
final number of notes).
> If we're here, isn't it likely that *all* notes are in the wrong
> path in the tree, and we need to move them all to a new location?
> If that's true then should we instead just build an entirely new
> tree and swap the root when we are done?
Hmm. Not always. In your earlier scenario where we add 2,000,000 notes in a
single commit, the current code would need to rewrite 255 of them from
fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout 2. But the
vast majority (1,934,465) would not require rewriting (having been added at
the correct fanout initially). However, if we build a new tree (by which I
assume you mean tree_content_remove() from the old tree and
tree_content_set() to the new tree for every single note (and non-note)), we
end up processing all 2,000,000 entries.
> As we empty out a tree the object will be recycled into a pool of
> trees which can be reused at a later point. It might actually make
> sense to build the new tree under a different root. We won't scan
> entries we've moved, and the memory difference should be fairly
> small as tree_content_remove() will make a subtree available for
> reuse as soon as its empty. So we're only dealing with a handful
> of additional tree objects as we do the conversion.
I'm not sure I get the details here. How can we avoid doing the
_remove()/_set() from/to the old/new tree for every tree_entry? In other
words, how do we avoid removing and re-setting the 2,000,000 notes in the
above example?
Thanks for the review!
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Generic filters for git archive?
From: Russ Dill @ 2009-12-08 1:06 UTC (permalink / raw)
To: git
I'm trying to add copyright headers to my source files as they are
exported via git archive. eg:
* $Copyright$
to
* Copyright (c) 2003-2009 by Foo Bar
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
And properly handling things like '# $Copyright$', '// $Copyright$',
etc. I have a sed script that does this, but no way to apply it to the
output of git archive. I tried setting up a smudge filter that would
only smudge output on archive exports, but it doesn't appear that the
smudge filters get run on git archive.
I am currently running 1.6.3.3
^ permalink raw reply
* [PATCH 2/2] Make it possible to apply a range of changes at once
From: jepler @ 2009-12-08 0:22 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Heiko Voigt, Peter Baumann, Jeff Epler
In-Reply-To: <1260231763-19194-1-git-send-email-jepler@unpythonic.net>
From: Jeff Epler <jepler@unpythonic.net>
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
git-gui.sh | 15 +++-
lib/diff.tcl | 242 ++++++++++++++++++++++++++++++++++------------------------
2 files changed, 153 insertions(+), 104 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 718277a..803423f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3199,7 +3199,7 @@ set ui_diff_applyhunk [$ctxm index last]
lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
$ctxm add command \
-label [mc "Apply/Reverse Line"] \
- -command {apply_line $cursorX $cursorY; do_rescan}
+ -command {apply_range_or_line $cursorX $cursorY; do_rescan}
set ui_diff_applyline [$ctxm index last]
lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
$ctxm add separator
@@ -3239,12 +3239,21 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
if {[string first {U} $state] >= 0} {
tk_popup $ctxmmg $X $Y
} else {
+ set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}]
if {$::ui_index eq $::current_diff_side} {
set l [mc "Unstage Hunk From Commit"]
- set t [mc "Unstage Line From Commit"]
+ if {$has_range} {
+ set t [mc "Unstage Lines From Commit"]
+ } else {
+ set t [mc "Unstage Line From Commit"]
+ }
} else {
set l [mc "Stage Hunk For Commit"]
- set t [mc "Stage Line For Commit"]
+ if {$has_range} {
+ set t [mc "Stage Lines For Commit"]
+ } else {
+ set t [mc "Stage Line For Commit"]
+ }
}
if {$::is_3way_diff || $::is_submodule_diff
|| $current_diff_path eq {}
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 066755b..5e738e2 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -533,10 +533,23 @@ proc apply_hunk {x y} {
}
}
-proc apply_line {x y} {
+proc apply_range_or_line {x y} {
global current_diff_path current_diff_header current_diff_side
global ui_diff ui_index file_states
+ set selected [$ui_diff tag nextrange sel 0.0]
+
+ if {$selected == {}} {
+ set first [$ui_diff index "@$x,$y"]
+ set last $first
+ } else {
+ set first [lindex $selected 0]
+ set last [lindex $selected 1]
+ }
+
+ set first_l [$ui_diff index "$first linestart"]
+ set last_l [$ui_diff index "$last lineend"]
+
if {$current_diff_path eq {} || $current_diff_header eq {}} return
if {![lock_index apply_hunk]} return
@@ -559,120 +572,147 @@ proc apply_line {x y} {
}
}
- set the_l [$ui_diff index @$x,$y]
+ set wholepatch {}
- # operate only on change lines
- set c1 [$ui_diff get "$the_l linestart"]
- if {$c1 ne {+} && $c1 ne {-}} {
- unlock_index
- return
- }
- set sign $c1
-
- set i_l [$ui_diff search -backwards -regexp ^@@ $the_l 0.0]
- if {$i_l eq {}} {
- unlock_index
- return
- }
- # $i_l is now at the beginning of a line
+ while {$first_l < $last_l} {
+ set i_l [$ui_diff search -backwards -regexp ^@@ $first_l 0.0]
+ if {$i_l eq {}} {
+ # If there's not a @@ above, then the selected range
+ # must have come before the first_l @@
+ set i_l [$ui_diff search -regexp ^@@ $first_l $last_l]
+ }
+ if {$i_l eq {}} {
+ unlock_index
+ return
+ }
+ # $i_l is now at the beginning of a line
- # pick start line number from hunk header
- set hh [$ui_diff get $i_l "$i_l + 1 lines"]
- set hh [lindex [split $hh ,] 0]
- set hln [lindex [split $hh -] 1]
+ # pick start line number from hunk header
+ set hh [$ui_diff get $i_l "$i_l + 1 lines"]
+ set hh [lindex [split $hh ,] 0]
+ set hln [lindex [split $hh -] 1]
- # There is a special situation to take care of. Consider this hunk:
- #
- # @@ -10,4 +10,4 @@
- # context before
- # -old 1
- # -old 2
- # +new 1
- # +new 2
- # context after
- #
- # We used to keep the context lines in the order they appear in the
- # hunk. But then it is not possible to correctly stage only
- # "-old 1" and "+new 1" - it would result in this staged text:
- #
- # context before
- # old 2
- # new 1
- # context after
- #
- # (By symmetry it is not possible to *un*stage "old 2" and "new 2".)
- #
- # We resolve the problem by introducing an asymmetry, namely, when
- # a "+" line is *staged*, it is moved in front of the context lines
- # that are generated from the "-" lines that are immediately before
- # the "+" block. That is, we construct this patch:
- #
- # @@ -10,4 +10,5 @@
- # context before
- # +new 1
- # old 1
- # old 2
- # context after
- #
- # But we do *not* treat "-" lines that are *un*staged in a special
- # way.
- #
- # With this asymmetry it is possible to stage the change
- # "old 1" -> "new 1" directly, and to stage the change
- # "old 2" -> "new 2" by first staging the entire hunk and
- # then unstaging the change "old 1" -> "new 1".
-
- # This is non-empty if and only if we are _staging_ changes;
- # then it accumulates the consecutive "-" lines (after converting
- # them to context lines) in order to be moved after the "+" change
- # line.
- set pre_context {}
-
- set n 0
- set i_l [$ui_diff index "$i_l + 1 lines"]
- set patch {}
- while {[$ui_diff compare $i_l < "end - 1 chars"] &&
- [$ui_diff get $i_l "$i_l + 2 chars"] ne {@@}} {
- set next_l [$ui_diff index "$i_l + 1 lines"]
- set c1 [$ui_diff get $i_l]
- if {[$ui_diff compare $i_l <= $the_l] &&
- [$ui_diff compare $the_l < $next_l]} {
- # the line to stage/unstage
- set ln [$ui_diff get $i_l $next_l]
- if {$c1 eq {-}} {
- set n [expr $n+1]
+ # There is a special situation to take care of. Consider this
+ # hunk:
+ #
+ # @@ -10,4 +10,4 @@
+ # context before
+ # -old 1
+ # -old 2
+ # +new 1
+ # +new 2
+ # context after
+ #
+ # We used to keep the context lines in the order they appear in
+ # the hunk. But then it is not possible to correctly stage only
+ # "-old 1" and "+new 1" - it would result in this staged text:
+ #
+ # context before
+ # old 2
+ # new 1
+ # context after
+ #
+ # (By symmetry it is not possible to *un*stage "old 2" and "new
+ # 2".)
+ #
+ # We resolve the problem by introducing an asymmetry, namely,
+ # when a "+" line is *staged*, it is moved in front of the
+ # context lines that are generated from the "-" lines that are
+ # immediately before the "+" block. That is, we construct this
+ # patch:
+ #
+ # @@ -10,4 +10,5 @@
+ # context before
+ # +new 1
+ # old 1
+ # old 2
+ # context after
+ #
+ # But we do *not* treat "-" lines that are *un*staged in a
+ # special way.
+ #
+ # With this asymmetry it is possible to stage the change "old
+ # 1" -> "new 1" directly, and to stage the change "old 2" ->
+ # "new 2" by first staging the entire hunk and then unstaging
+ # the change "old 1" -> "new 1".
+ #
+ # Applying multiple lines adds complexity to the special
+ # situation. The pre_context must be moved after the entire
+ # first block of consecutive staged "+" lines, so that
+ # staging both additions gives the following patch:
+ #
+ # @@ -10,4 +10,6 @@
+ # context before
+ # +new 1
+ # +new 2
+ # old 1
+ # old 2
+ # context after
+
+ # This is non-empty if and only if we are _staging_ changes;
+ # then it accumulates the consecutive "-" lines (after
+ # converting them to context lines) in order to be moved after
+ # "+" change lines.
+ set pre_context {}
+
+ set n 0
+ set m 0
+ set i_l [$ui_diff index "$i_l + 1 lines"]
+ set patch {}
+ while {[$ui_diff compare $i_l < "end - 1 chars"] &&
+ [$ui_diff get $i_l "$i_l + 2 chars"] ne {@@}} {
+ set next_l [$ui_diff index "$i_l + 1 lines"]
+ set c1 [$ui_diff get $i_l]
+ if {[$ui_diff compare $first_l <= $i_l] &&
+ [$ui_diff compare $i_l < $last_l] &&
+ ($c1 eq {-} || $c1 eq {+})} {
+ # a line to stage/unstage
+ set ln [$ui_diff get $i_l $next_l]
+ if {$c1 eq {-}} {
+ set n [expr $n+1]
+ set patch "$patch$pre_context$ln"
+ set pre_context {}
+ } else {
+ set m [expr $m+1]
+ set patch "$patch$ln"
+ }
+ } elseif {$c1 ne {-} && $c1 ne {+}} {
+ # context line
+ set ln [$ui_diff get $i_l $next_l]
set patch "$patch$pre_context$ln"
+ set n [expr $n+1]
+ set m [expr $m+1]
+ set pre_context {}
+ } elseif {$c1 eq $to_context} {
+ # turn change line into context line
+ set ln [$ui_diff get "$i_l + 1 chars" $next_l]
+ if {$c1 eq {-}} {
+ set pre_context "$pre_context $ln"
+ } else {
+ set patch "$patch $ln"
+ }
+ set n [expr $n+1]
+ set m [expr $m+1]
} else {
- set patch "$patch$ln$pre_context"
- }
- set pre_context {}
- } elseif {$c1 ne {-} && $c1 ne {+}} {
- # context line
- set ln [$ui_diff get $i_l $next_l]
- set patch "$patch$pre_context$ln"
- set n [expr $n+1]
- set pre_context {}
- } elseif {$c1 eq $to_context} {
- # turn change line into context line
- set ln [$ui_diff get "$i_l + 1 chars" $next_l]
- if {$c1 eq {-}} {
- set pre_context "$pre_context $ln"
- } else {
- set patch "$patch $ln"
+ # a change in the opposite direction of
+ # to_context which is outside the range of
+ # lines to apply.
+ set patch "$patch$pre_context"
+ set pre_context {}
}
- set n [expr $n+1]
+ set i_l $next_l
}
- set i_l $next_l
+ set patch "$patch$pre_context"
+ set wholepatch "$wholepatch@@ -$hln,$n +$hln,$m @@\n$patch"
+ set first_l [$ui_diff index "$next_l + 1 lines"]
}
- set patch "$patch$pre_context"
- set patch "@@ -$hln,$n +$hln,[eval expr $n $sign 1] @@\n$patch"
if {[catch {
set enc [get_path_encoding $current_diff_path]
set p [eval git_write $apply_cmd]
fconfigure $p -translation binary -encoding $enc
puts -nonewline $p $current_diff_header
- puts -nonewline $p $patch
+ puts -nonewline $p $wholepatch
close $p} err]} {
error_popup [append $failed_msg "\n\n$err"]
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/2] Fix applying a line when all following lines are deletions
From: jepler @ 2009-12-08 0:22 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Heiko Voigt, Peter Baumann, Jeff Epler
In-Reply-To: <1260231763-19194-1-git-send-email-jepler@unpythonic.net>
From: Jeff Epler <jepler@unpythonic.net>
If a diff looked like
@@
context
-del1
-del2
and you wanted to stage the deletion 'del1', the generated patch wouldn't
apply because it was missing the line 'del2' converted to context, but
this line was counted in the @@-line
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
lib/diff.tcl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index bd5d189..066755b 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -664,6 +664,7 @@ proc apply_line {x y} {
}
set i_l $next_l
}
+ set patch "$patch$pre_context"
set patch "@@ -$hln,$n +$hln,[eval expr $n $sign 1] @@\n$patch"
if {[catch {
--
1.6.3.3
^ permalink raw reply related
* [PATCHv3 0/2] git-gui: (un)stage a range of changes at once
From: jepler @ 2009-12-08 0:22 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Heiko Voigt, Peter Baumann, Jeff Epler
From: Jeff Epler <jepler@unpythonic.net>
This set of patches allows git-gui to stage multiple lines at once by
selecting the range and then using the "apply lines" item in the context
menu.
Compared to the earlier versions, I've fixed all the bugs I became aware of.
I've also added the missing signed-off-by.
The first patch fixes an existing bug in git-gui when staging a deletion
followed by another deletion followed by the end of the file.
Jeff Epler (2):
Fix applying a line when all following lines are deletions
Make it possible to apply a range of changes at once
git-gui.sh | 15 +++-
lib/diff.tcl | 241 ++++++++++++++++++++++++++++++++++------------------------
2 files changed, 153 insertions(+), 103 deletions(-)
^ permalink raw reply
* Re: [PATCH] Add commit.status, --status, and --no-status
From: James Pickens @ 2009-12-08 0:39 UTC (permalink / raw)
To: James P. Howard, II; +Cc: git
In-Reply-To: <1260225927-33612-1-git-send-email-jh@jameshoward.us>
On Mon, Dec 7, 2009, James P. Howard, II <jh@jameshoward.us> wrote:
> This commit provides support for commit.status, --status, and
> --no-status, which control whether or not the git status information
> is included in the commit message template when using an editor to
> prepare the commit message. It does not affect the effects of a
> user's commit.template settings.
At the risk of sounding like a curmudgeon, I have to register an objection
to this _as a config option_, for the same reasons that I objected to the
'grep --full-tree' config option [1]. Both options fundamentally change
user visible behavior, both options IMHO will be used by a small minority
of git users, and in both cases, the desired behavior can be attained using
aliases in combination with a new command line option.
Sorry if I'm obstructing progress, but I think that, for the good of the
Git community as a whole, it's best to keep the number of config options
(and to a lesser extent, command line options) as low as possible.
Note that I do not object to adding --no-status as a command line option.
[1] http://article.gmane.org/gmane.comp.version-control.git/133681
James
^ permalink raw reply
* Re: Re: [RFC PATCH v2 0/2] git-gui: (un)stage a range of changes at once
From: Jeff Epler @ 2009-12-08 0:38 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091207125435.GA43609@book.hvoigt.net>
On Mon, Dec 07, 2009 at 01:54:35PM +0100, Heiko Voigt wrote:
> Jeff could you clarify or provide an example?
If I recall correctly, the problem with the v2 patch was when the change
was like
@@ -13,8 +13,8 @@ set appvers {@@GITGUI_VERSION@@}
set copyright [encoding convertfrom utf-8 {
Copyright © 2006, 2007 Shawn Pearce, et. al.
-This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
+blah blah
+blah blah
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
and the 'blah blah' lines were both staged in the same operation.
When doing this, the staged change is actually
+blah blah
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
+blah blah
but the change that should have been staged is:
+blah blah
+blah blah
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Since it requires staging multiple "+" lines in one go, this problem
doesn't exist in git-gui before my changes.
The v3 patch I posted just a few minutes ago fixes this problem.
Jeff
^ permalink raw reply
* [PATCH] Add commit.status, --status, and --no-status
From: James P. Howard, II @ 2009-12-07 22:45 UTC (permalink / raw)
To: git; +Cc: James P. Howard, II
In-Reply-To: <20091206131217.GA12851@sigill.intra.peff.net>
This commit provides support for commit.status, --status, and
--no-status, which control whether or not the git status information
is included in the commit message template when using an editor to
prepare the commit message. It does not affect the effects of a
user's commit.template settings.
Signed-off-by: James P. Howard, II <jh@jameshoward.us>
---
Documentation/config.txt | 5 +++++
Documentation/git-commit.txt | 14 +++++++++++++-
builtin-commit.c | 9 +++++++--
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a1e36d7..5561560 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -705,6 +705,11 @@ color.ui::
terminal. When more specific variables of color.* are set, they always
take precedence over this setting. Defaults to false.
+commit.status
+ A boolean to enable/disable inclusion of status information in the
+ commit message template when using an editor to prepare the commit
+ message. Defaults to true.
+
commit.template::
Specify a file to use as the template for new commit messages.
"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index d227cec..0e53518 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,8 @@ SYNOPSIS
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
[(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
- [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
+ [--cleanup=<mode>] [--status | --no-status] [--]
+ [[-i | -o ]<file>...]
DESCRIPTION
-----------
@@ -207,6 +208,17 @@ specified.
to be committed, paths with local changes that will be left
uncommitted and paths that are untracked.
+--status::
+ Include the output of linkgit:git-status[1] in the commit
+ message template when using an editor to prepare the commit
+ message. Defaults to on, but can be used to override
+ configuration variable commit.status.
+
+--no-status::
+ Do not include the output of linkgit:git-status[1] in the
+ commit message template when using an editor to prepare the
+ default commit message.
+
\--::
Do not interpret any more arguments as options.
diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..095c186 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -67,7 +67,7 @@ static enum {
} cleanup_mode;
static char *cleanup_arg;
-static int use_editor = 1, initial_commit, in_merge;
+static int use_editor = 1, initial_commit, in_merge, include_status = 1;
static const char *only_include_assumed;
static struct strbuf message;
@@ -97,6 +97,7 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
+ OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
OPT_GROUP("Commit contents options"),
OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
@@ -547,7 +548,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
/* This checks if committer ident is explicitly given */
git_committer_info(0);
- if (use_editor) {
+ if (use_editor && include_status) {
char *author_ident;
const char *committer_ident;
@@ -1006,6 +1007,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
if (!strcmp(k, "commit.template"))
return git_config_pathname(&template_file, k, v);
+ if (!strcmp(k, "commit.status")) {
+ include_status = git_config_bool(k, v);
+ return 0;
+ }
return git_status_config(k, v, s);
}
--
1.6.5.3
^ 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