Git development
 help / color / mirror / Atom feed
* Re: How do you best store structured data in git repositories?
From: David Aguilar @ 2009-12-08  7:14 UTC (permalink / raw)
  To: Sebastian Setzer; +Cc: git
In-Reply-To: <1260220821.3545.12.camel@nord26-amd64>

On Mon, Dec 07, 2009 at 10:20:21PM +0100, Sebastian Setzer wrote:
> On Thursday, Dec 03 2009 at 16:14 -0800, David Aguilar wrote:
> > On Wed, Dec 02, 2009 at 04:17:10PM -0500, Avery Pennarun wrote:
> > > On Wed, Dec 2, 2009 at 4:08 PM, Sebastian Setzer
> > > <sebastianspublicaddress@googlemail.com> wrote:
> > > > Do you use XML for this purpose?
> > > 
> > > XML is terrible for most data storage purposes.
> > 
> > I agree 100%.
> > 
> > JSON's not too bad for data structures and is known to
> > be friendly to XML expats.
> > 
> Sorry, I didn't want to start a flamewar against XML. I'm no big friend
> of XML myself, but I don't know of an (open source) diff-/merge tool for
> any general purpose file format other than XML or plain text.
> When you mention other formats, I'd be interested in
>   - why this format is good for storage in git
>   - if there are merge tools available which ensure that, after a merge,
> the structure (and maybe additional contraints) is still valid.
> 
> Thanks for your comments,
> Sebastian

Sorry, didn't mean to sound xml-flaming.  The only reason for
mentioning json, yaml, etc. is that they're good data structure
formats.  They're all plain text formats, so you can use existing
diff/merge tools.

I guess none of this has much to do with git aside from being
able to write custom merge drivers to operate on them as data.

If there's a diff/merge tool for xml that works well then
hooking it up to git-{diff,merge}tool might be something
to try too.

-- 
		David

^ permalink raw reply

* Re: [PATCH] Add commit.status, --status, and --no-status
From: Junio C Hamano @ 2009-12-08  7:13 UTC (permalink / raw)
  To: Jeff King; +Cc: James P. Howard, II, git
In-Reply-To: <20091208060415.GC9951@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Dec 07, 2009 at 05:45:27PM -0500, James P. Howard, II 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.
>
> Thanks, this looks very cleanly done. The only complaint I would make is
> that it should probably include a simple test case.

Yes.  Also I am a _bit_ worried about the name "status", as the longer
term direction is to make "status" not "a preview of commit", may confuse
people who do read Release Notes.

^ permalink raw reply

* Re: Gui criticism
From: Marco Costalba @ 2009-12-08  7:12 UTC (permalink / raw)
  To: Ram Rachum; +Cc: git
In-Reply-To: <loom.20091205T194800-496@post.gmane.org>

On Sat, Dec 5, 2009 at 19:51, Ram Rachum <cool-rr@cool-rr.com> wrote:
> Hello!
>
> This is my first time on this list. I'm a Python developer who's been using git
> for about a year. I generally like it, but I have several gripes about the GUI.
> (My development machine is on Windows XP.) Would specifying these criticisms be
> helpful to you?
>

Wrong approach. First do then ask. Is not so polite but is how it
works in real word.  :-)

^ 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  7:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, James Vega, git
In-Reply-To: <20091208054724.GA21347@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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.

Thanks.

> 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.

I was wondering about the same thing while bisecting.  By the current
definition of "diff --git", removing the "deleted file" or "new file" line
makes the patch an invalid "git format diff".  See the beginning of
parse_git_header() where we say "we don't guess" and initialize both
is_new and is_delete to false (and we flip them upon seeing "deleted file"
and "new file", but never with "/dev/null").

> But maybe it is not worth worrying about too much. That patch format is
> not generated intentionally by any known software.

I think some recent other SCMs produce what they claim to be "diff --git",
but I don't know if they implement the format correctly enough.  I am not
worried about their implemention of binary patches (if they do not
implement it correctly they will most likely get garbage), but do they get
the abbreviated hash on the "index" line correctly?  You can put garbage
on the line and most of the time it would work but it will break "am -3"
by breaking "apply --build-fake-ancestor".

I just checked "hg diff --git"; at least it shows "deleted file".

> 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.

I agree it is weird.

> 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).

Interesting.  Does "add -p" (especially its [e]dit codepath) know enough
about what it is doing?  If so, it should be able to add "deleted file" on
its own (and remove it when the result of editing and picking hunks makes
the patch a non-deletion).  For example, if you have a two-liner in the
index and have deleted one line in the work tree, and run "add -p":

        diff --git a/foo b/foo
        index 3bd1f0e..257cc56 100644
        --- a/foo
        +++ b/foo
        @@ -1,2 +1 @@
         foo
        -bar

you *should* be able to edit it into a patch that removes all lines.

Perhaps the "add -i" at the end should offer, after noticing that the
chosen and edited hunks will make the postimage an empty file, a chance
for the user to say "I not only want to remove the contents from the path,
but want to remove the path itself" in such a case?

I dunno.

^ permalink raw reply

* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: James Vega @ 2009-12-08  6:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091208060109.GB9951@coredump.intra.peff.net>

On Tue, Dec 08, 2009 at 01:01:09AM -0500, Jeff King wrote:
> On Tue, Dec 08, 2009 at 12:47:24AM -0500, Jeff King wrote:
> 
> > 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).
> 
> BTW, the code for this is the much smaller change below. If you prefer
> that, I can squash in the test and write up an appropriate commit
> message.
> 
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 35f4ef1..02e97b9 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1217,7 +1217,11 @@ sub patch_update_file {
>  	if (@{$mode->{TEXT}}) {
>  		unshift @hunk, $mode;
>  	}
> -	if (@{$deletion->{TEXT}} && !@hunk) {
> +	if (@{$deletion->{TEXT}}) {
> +		foreach my $hunk (@hunk) {
> +			push @{$deletion->{TEXT}}, @{$hunk->{TEXT}};
> +			push @{$deletion->{DISPLAY}}, @{$hunk->{DISPLAY}};
> +		}
>  		@hunk = ($deletion);
>  	}
>  

Thanks for the quick patches.  This was similar to what I was working on, but
cleaner than what I had.  Works well for me.

-- 
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <vega.james@gmail.com>

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Jeff King @ 2009-12-08  6:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Ilari Liusvaara, git
In-Reply-To: <20091208055735.GA9951@coredump.intra.peff.net>

On Tue, Dec 08, 2009 at 12:57:35AM -0500, Jeff King wrote:

> On Mon, Dec 07, 2009 at 12:07:24PM -0800, Junio C Hamano wrote:
> 
> > I haven't asked people why they choose to write like this:
> > 
> > 	char* string;
> > 
> > beyond "that is how we were taught and what we are used to".
> 
> I have seen it in C++ code and recommended many years ago on
> comp.lang.c++. The argument was something along the lines of:

Perhaps I should have simply used google:

  http://www2.research.att.com/~bs/bs_faq2.html#whitespace

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] git-rm doc: Describe how to sync index & work tree
From: Björn Gustavsson @ 2009-12-08  6:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr6qze4i.fsf@alter.siamese.dyndns.org>

2009/12/7 Junio C Hamano <gitster@pobox.com>:
> I think mentioning "add -u" in the same section as "commit -a" would be
> helpful, as these two are more for user's own development (as opposed to
> vendor-code-drop).  I'd perhaps squash something like this in.  Please say
> "yes", "don't, it is horrible", or something in between ;-)

Yes, but...

> -automatically notice and record all removals.
> +automatically notice and record all removals.  `git add -u`
> +can be used for a similar effect without commiting.

s/commiting/committing/

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

^ permalink raw reply

* Re: [PATCH 2/2] git-rm doc: Describe how to sync index & work tree
From: Björn Gustavsson @ 2009-12-08  6:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr6qze4i.fsf@alter.siamese.dyndns.org>

2009/12/7 Junio C Hamano <gitster@pobox.com>:
> Looks sensible.
>
> I think mentioning "add -u" in the same section as "commit -a" would be
> helpful, as these two are more for user's own development (as opposed to
> vendor-code-drop).  I'd perhaps squash something like this in.  Please say
> "yes", "don't, it is horrible", or something in between ;-)

Yes. :-)

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

^ permalink raw reply

* Re: [PATCH] Add commit.status, --status, and --no-status
From: Jeff King @ 2009-12-08  6:04 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 07, 2009 at 05:45:27PM -0500, James P. Howard, II 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.

Thanks, this looks very cleanly done. The only complaint I would make is
that it should probably include a simple test case.

-Peff

^ permalink raw reply

* Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"
From: Nanako Shiraishi @ 2009-12-08  6:01 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Matthieu Moy, Michael J Gruber,
	Michael Haggerty, git
In-Reply-To: <7vtyw2p2ju.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.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:"?

Sorry. Yes I did. Please amend it to -

 Subject: rebase -i --autosquash: auto-squash commits

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: git-apply fails on creating a new file, with both -p and --directory specified
From: Jeff King @ 2009-12-08  6:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: James Vega, git
In-Reply-To: <20091208054724.GA21347@coredump.intra.peff.net>

On Tue, Dec 08, 2009 at 12:47:24AM -0500, Jeff King wrote:

> 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).

BTW, the code for this is the much smaller change below. If you prefer
that, I can squash in the test and write up an appropriate commit
message.

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 35f4ef1..02e97b9 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1217,7 +1217,11 @@ sub patch_update_file {
 	if (@{$mode->{TEXT}}) {
 		unshift @hunk, $mode;
 	}
-	if (@{$deletion->{TEXT}} && !@hunk) {
+	if (@{$deletion->{TEXT}}) {
+		foreach my $hunk (@hunk) {
+			push @{$deletion->{TEXT}}, @{$hunk->{TEXT}};
+			push @{$deletion->{DISPLAY}}, @{$hunk->{DISPLAY}};
+		}
 		@hunk = ($deletion);
 	}
 

^ permalink raw reply related

* Re: What's cooking in git.git (Dec 2009, #02; Sat, 05)
From: Tay Ray Chuan @ 2009-12-08  5:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Martin Storsj?, Junio C Hamano, git
In-Reply-To: <20091207153736.GC17173@spearce.org>

Hi,

On Mon, Dec 7, 2009 at 11:37 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Martin Storsj? <martin@martin.st> wrote:
>> On Sun, 6 Dec 2009, Junio C Hamano wrote:
>> >
>> > * tr/http-updates (2009-12-01) 3 commits
>> >  - Allow curl to rewind the RPC read buffer
>> >  - Add an option for using any HTTP authentication scheme, not only basic
>> >  - http: maintain curl sessions
>> >
>> > There was a discussion on a better structure not to require rewinding in
>> > the first place?  I didn't follow it closely...
>>
>> I think the conclusion is: Rewinding support isn't strictly necessary,
>> there's a number of mechanisms in both git and curl that should make sure
>> that those cases shouldn't surface. A few of them in curl have an
>> unfortunate conincidence of bugs up until the latest version, though,
>> leaving much fewer mechanisms in place to avoid this.
>>
>> Since that patch is quite non-intrusive I think it's a good safeguard,
>> though. What do you think, Tay, keep it or leave it?
>
> I think the conclusion of the thread was that what you have queued
> in tr/http-updates is OK as-is.  The patch to grow the postbuffer
> to store the entire request wasn't a good idea and got dropped.

Martin, sorry the late reply. I agree with Shawn. Perhaps we could
relook at rewinding-to-any-position when restarting the rpc client
(eg. git-send-pack) has been implemented.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Jeff King @ 2009-12-08  5:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Ilari Liusvaara, git
In-Reply-To: <7vein635vn.fsf@alter.siamese.dyndns.org>

On Mon, Dec 07, 2009 at 12:07:24PM -0800, Junio C Hamano wrote:

> I haven't asked people why they choose to write like this:
> 
> 	char* string;
> 
> beyond "that is how we were taught and what we are used to".

I have seen it in C++ code and recommended many years ago on
comp.lang.c++. The argument was something along the lines of:

  1. It's good to keep type information together, especially in C++
     where you are often doing things like using types as template
     parameters.

  2. The fact that "char* foo, bar" doesn't do what you want isn't
     relevant if you have a style guideline not to declare two variables
     on the same line (because it's easier to notice both if they each
     get their own line, and because in C++ you can declare closer to
     the point of use).

But that is me paraphrasing an argument I read on usenet almost 10 years
ago, so I may be entirely misremembering (and please don't flame me; I
am presenting it for anthropological curiosity, not because I believe we
should use that style).

-Peff

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox