Git development
 help / color / mirror / Atom feed
* Re: Where did Documentation/perf_counter disappear from linux-2.6-tip.git ?
From: Junio C Hamano @ 2009-12-22 18:08 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: Git Mailing List
In-Reply-To: <4B3099A5.6040808@gmail.com>

Tomas Carnecky <tomas.carnecky@gmail.com> writes:

>  $ git --version
> git version 1.6.6.rc4
>
> # Documentation/perf_counter is missing from the master branch, so
> first let's find
> # out what the last commit was that touched that subdirectory:
> $ git log --all -1 -- Documentation/perf_counter
> commit 436224a6d8bb3e29fe0cc18122f8d1f593da67b8
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Tue Jun 2 21:02:36 2009 +0200
> ...
> M       Documentation/perf_counter/builtin-report.c
>
> # Great, let's look in which branch that commit is
> $ git branch --contains 436224a6d8bb3e29fe0cc18122f8d1f593da67b8
> * master
>
> # So, let's look at the log of master and limit it to that subdirectory:
> $ git log master -- Documentation/perf_counter
> $

Add --full-history so that you would get _all_ possible explanation of the
history, perhaps?

In a history with this shape:

    ---A---B---C---D---E
            \     /
             F---G

suppose that

 - commit F introduces a path;
 - commit G removes the path;
 - no other commit has the path in question.

Without --full-history, "log E -- path" is asked to give "_one_ possible
way to explain the current state of path in E" (iow, "why there is nothing
there right now at E?").

Two explanations are possible even in this vastly simplified toy history.

 - It didn't exist in A, and none of the subsequent commits B, C, D that
   lead to E did anything to change that.

 - F added it, but G changed mind and removed it.

When "log" encounters a merge commit while traversing the history
backwards (in this case D) with paths limiter, if there is a commit among
its parent whose tree matches its tree with respect to the paths, side
branches leading to all the other parents are culled and only that one
history is followed to explain the history.  In this case, neither C or D
has the path, so their trees with respect to the paths limiter match, and
git doesn't follow the side branch that has F and G without
--full-history.

^ permalink raw reply

* Re: Huge pack file from small unpacked objects
From: Junio C Hamano @ 2009-12-22 18:11 UTC (permalink / raw)
  To: Nick Triantos; +Cc: B Smith-Mannschott, git@vger.kernel.org
In-Reply-To: <75B8C0BEE0AE2A44AA971D218D9FE99E6B06F111@VMBX125.ihostexchange.net>

Nick Triantos <nick@perceptivepixel.com> writes:

> Is there an easy way to unpack the pack file and see what's inside (including sizes)?

Unpack?

    (mkdir /var/tmp/junk &&
     cd /var/tmp/junk &&
     git init && git unpack-objects) <.git/objects/pack/pack-$that_one.pack

It might also be interesting to see

    git verify-pack -v .git/objects/pack/pack-$that_one.pack

^ permalink raw reply

* Re: Where did Documentation/perf_counter disappear from linux-2.6-tip.git ?
From: Tomas Carnecky @ 2009-12-22 18:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vmy1a3mrh.fsf@alter.siamese.dyndns.org>

On 12/22/09 7:08 PM, Junio C Hamano wrote:
> Tomas Carnecky<tomas.carnecky@gmail.com>  writes:
>
>>   $ git --version
>> git version 1.6.6.rc4
>>
>> # Documentation/perf_counter is missing from the master branch, so
>> first let's find
>> # out what the last commit was that touched that subdirectory:
>> $ git log --all -1 -- Documentation/perf_counter
>> commit 436224a6d8bb3e29fe0cc18122f8d1f593da67b8
>> Author: Peter Zijlstra<a.p.zijlstra@chello.nl>
>> Date:   Tue Jun 2 21:02:36 2009 +0200
>> ...
>> M       Documentation/perf_counter/builtin-report.c
>>
>> # Great, let's look in which branch that commit is
>> $ git branch --contains 436224a6d8bb3e29fe0cc18122f8d1f593da67b8
>> * master
>>
>> # So, let's look at the log of master and limit it to that subdirectory:
>> $ git log master -- Documentation/perf_counter
>> $
>
> Add --full-history so that you would get _all_ possible explanation of the
> history, perhaps?
>
> In a history with this shape:
>
>      ---A---B---C---D---E
>              \     /
>               F---G
>
> suppose that
>
>   - commit F introduces a path;
>   - commit G removes the path;
>   - no other commit has the path in question.
>
> Without --full-history, "log E -- path" is asked to give "_one_ possible
> way to explain the current state of path in E" (iow, "why there is nothing
> there right now at E?").
>
> Two explanations are possible even in this vastly simplified toy history.
>
>   - It didn't exist in A, and none of the subsequent commits B, C, D that
>     lead to E did anything to change that.
>
>   - F added it, but G changed mind and removed it.
>
> When "log" encounters a merge commit while traversing the history
> backwards (in this case D) with paths limiter, if there is a commit among
> its parent whose tree matches its tree with respect to the paths, side
> branches leading to all the other parents are culled and only that one
> history is followed to explain the history.  In this case, neither C or D
> has the path, so their trees with respect to the paths limiter match, and
> git doesn't follow the side branch that has F and G without
> --full-history.

I've never used nor seen --full-history before, but it did help in this 
case, git log now correctly sees the commits touching that subdirectory. 
Thanks for the explanation.

tom

^ permalink raw reply

* Re: [RFC PATCH] Record a single transaction for conflicting push  operations
From: Catalin Marinas @ 2009-12-22 18:33 UTC (permalink / raw)
  To: Karl Wiberg; +Cc: git, Gustav Hållberg
In-Reply-To: <b8197bcb0912210548q67c1da4bhe023bed2811394d4@mail.gmail.com>

Updated patch below:


Record a single transaction for conflicting push operations

From: Catalin Marinas <catalin.marinas@gmail.com>

StGit commands resulting in a conflicting patch pushing record two
transactions in the log (with one of them being inconsistent with HEAD
!= top). Undoing such operations requires two "stg undo" (possibly with
--hard) commands which is unintuitive. This patch changes such
operations to only record one log entry and "stg undo" reverts the stack
to the state prior to the operation.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
Cc: Gustav Hållberg <gustav@virtutech.com>
Cc: Karl Wiberg <kha@treskal.com>
---
 stgit/lib/transaction.py |   35 ++++++++++++++++-------------------
 t/t3101-reset-hard.sh    |    2 +-
 t/t3103-undo-hard.sh     |    4 ++--
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 30a153b..d82e724 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -90,7 +90,6 @@ class StackTransaction(object):
         self.__applied = list(self.__stack.patchorder.applied)
         self.__unapplied = list(self.__stack.patchorder.unapplied)
         self.__hidden = list(self.__stack.patchorder.hidden)
-        self.__conflicting_push = None
         self.__error = None
         self.__current_tree = self.__stack.head.data.tree
         self.__base = self.__stack.base
@@ -232,10 +231,9 @@ class StackTransaction(object):
             self.__stack.patchorder.hidden = self.__hidden
             log.log_entry(self.__stack, msg)
         old_applied = self.__stack.patchorder.applied
-        write(self.__msg)
-        if self.__conflicting_push != None:
-            self.__patches = _TransPatchMap(self.__stack)
-            self.__conflicting_push()
+        if not self.__conflicts:
+            write(self.__msg)
+        else:
             write(self.__msg + ' (CONFLICT)')
         if print_current_patch:
             _print_current_patch(old_applied, self.__applied)
@@ -358,26 +356,25 @@ class StackTransaction(object):
         elif not merge_conflict and cd.is_nochange():
             s = 'empty'
         out.done(s)
-        def update():
-            if comm:
-                self.patches[pn] = comm
-            if pn in self.hidden:
-                x = self.hidden
-            else:
-                x = self.unapplied
-            del x[x.index(pn)]
-            self.applied.append(pn)
+
         if merge_conflict:
             # We've just caused conflicts, so we must allow them in
             # the final checkout.
             self.__allow_conflicts = lambda trans: True
+            self.__patches = _TransPatchMap(self.__stack)

-            # Save this update so that we can run it a little later.
-            self.__conflicting_push = update
-            self.__halt("%d merge conflict(s)" % len(self.__conflicts))
+        # Update the stack state
+        if comm:
+            self.patches[pn] = comm
+        if pn in self.hidden:
+            x = self.hidden
         else:
-            # Update immediately.
-            update()
+            x = self.unapplied
+        del x[x.index(pn)]
+        self.applied.append(pn)
+
+        if merge_conflict:
+            self.__halt("%d merge conflict(s)" % len(self.__conflicts))

     def push_tree(self, pn):
         """Push the named patch without updating its tree."""
diff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh
index bd97b3a..45e86dc 100755
--- a/t/t3101-reset-hard.sh
+++ b/t/t3101-reset-hard.sh
@@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '
     stg reset --hard master.stgit^~1 &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p2 - p3"
+    test "$(echo $(stg series))" = "+ p1 + p2 > p3"
 '

 test_done
diff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh
index 2d0f382..df14b1f 100755
--- a/t/t3103-undo-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -46,11 +46,11 @@ test_expect_success 'Try to undo without --hard' '

 cat > expected.txt <<EOF
 EOF
-test_expect_failure 'Try to undo with --hard' '
+test_expect_success 'Try to undo with --hard' '
     stg undo --hard &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p2 - p3" &&
+    test "$(echo $(stg series))" = "+ p1 + p2 > p3" &&
     test "$(stg id)" = "$(stg id $(stg top))"
 '

^ permalink raw reply related

* Re: following untracked parents in git-svn
From: Eric Wong @ 2009-12-22 18:38 UTC (permalink / raw)
  To: Robert Schiele; +Cc: git
In-Reply-To: <20091222102815.GA12259@sigfpe.ibm.com>

Robert Schiele <rschiele@gmail.com> wrote:
> Hi Eric et al.,
> 
> While using git-svn to work with a repository with a very complex history I
> discovered a very unfortunate behavior:
> 
> In general when a branch was derived (copied) from somewhere else git-svn
> follows this parent branch and imports it.  If multiple branches do that
> git-svn detects that the corresponding parrent branch already had been
> imported and reuses the imported data.  Unfortunately when the parent
> directory in the svn repository is not tracked as a branch in the svn-remote
> section of the config file (for instance when it is just a subdirectory of a
> tracked branch) this situation is no longer detected and this parent branch is
> imported multiple times with the same result.  In a large repository this can
> increase importing time drastically.
> 
> My analysis (as far as I understand the code) is that this is because the map
> files in .git/svn are indexed by their ref name in the git repository.
> Untracked branches are indexed by the name of their following branch ref name
> followed by @XX where XX is the revision number of the branch point.
> Obviously with that scheme the index name for two branches following a common
> parent tree is different and thus an already imported tree is not correctly
> detected.

Hi Robert, I'm aware of this problem.  It's not hit too often, but
occassional repositories I follow tend to hit this.

> My thoughts where now that this could potentially be fixed by not indexing
> those map files by their ref name in the git repository but by their location
> in the original svn repository.  Given that my understanding of the git-svn
> code is not good enough to decide about all the consequences of such a design
> change I'd like to ask you whether you think this change would be a good idea
> or whether I might have overlooked a fundamental problem that makes it
> impossible (or at least hard) to implement this idea.

Your idea sounds like it should work.  Unfortunately the code is a mess
and I've been lazy and lacking time/sufficient motivation to clean it
up, but I'd be glad to accept patches since the test coverage is pretty
good.

-- 
Eric Wong

^ permalink raw reply

* Regression: git-svn clone failure
From: Andrew Myrick @ 2009-12-22 18:43 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, sam

[Resending because I forgot to make the message plain text]

I was testing the latest changes to git-svn pushed to Eric's repo (git://git.bogomips.org/git-svn) by cloning a few other projects that I work on, and one of those clones failed where it had succeeded with git 1.6.5.  The error message I received is:

W:svn cherry-pick ignored (/branches/BranchA:3933-3950) - missing 1 commit(s) (eg 3fc50d3a7e0f555547ab34bb570db47ce71e1abb)
W:svn cherry-pick ignored (/branches/BranchB:3951-3970) - missing 1 commit(s) (eg 3beb9f2fde0a91aa0e8097e05f9054b23b221daf)
W:svn cherry-pick ignored (/branches/BranchC:3971-3985) - missing 1 commit(s) (eg a7ae202254604f8a78cca391be36c58efc79eb20)
Found merge parent (svn:mergeinfo prop): 8b2cf9e9250b5ff1fe47c68215d0a178cfe35a3b
Found merge parent (svn:mergeinfo prop): 59f8c571ae77885469bb31f007b0048ee7812e07
fatal: ambiguous argument '0..1': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
rev-list -1 0..1: command returned error: 128

At this point, the clone got stuck in a loop and I had to kill it.

Note that all of the projects I cloned had "svn cherry-pick ignored" warnings sprinkled throughout the fetch logs; I'm not sure how much they matter.  It comes from find_extra_svn_parents(), which I would guess is a best-effort algorithm, and any failures to detect extra parents aren't anything to worry about.

Does anyone have suggestions on how I can debug this?  If you want to poke around, I can't provide access to the repository, but I can run commands and relay (sanitized) output if it will aid in debugging.

-Andrew

^ permalink raw reply

* [PATCH] Prevent git blame from segfaulting on a missing author name
From: David Reiss @ 2009-12-22 18:51 UTC (permalink / raw)
  To: git

The author name should never be missing in a valid commit, but
git shouldn't segfault no matter what is in the object database.
(Most of the C code was written by Junio.)

Signed-off-by: David Reiss <dreiss@facebook.com>
---
 builtin-blame.c  |   13 ++++++++++---
 t/t8003-blame.sh |   13 +++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index d4e25a5..14830a3 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1305,6 +1305,7 @@ static void get_ac_line(const char *inbuf, const char *what,
 	error_out:
 		/* Ugh */
 		*tz = "(unknown)";
+		strcpy(person, *tz);
 		strcpy(mail, *tz);
 		*time = 0;
 		return;
@@ -1314,20 +1315,26 @@ static void get_ac_line(const char *inbuf, const char *what,
 	tmp = person;
 	tmp += len;
 	*tmp = 0;
-	while (*tmp != ' ')
+	while (person < tmp && *tmp != ' ')
 		tmp--;
+	if (tmp == person)
+		goto error_out;
 	*tz = tmp+1;
 	tzlen = (person+len)-(tmp+1);
 
 	*tmp = 0;
-	while (*tmp != ' ')
+	while (person < tmp && *tmp != ' ')
 		tmp--;
+	if (tmp == person)
+		goto error_out;
 	*time = strtoul(tmp, NULL, 10);
 	timepos = tmp;
 
 	*tmp = 0;
-	while (*tmp != ' ')
+	while (person < tmp && *tmp != ' ')
 		tmp--;
+	if (tmp <= person)
+		return;
 	mailpos = tmp + 1;
 	*tmp = 0;
 	maillen = timepos - tmp;
diff --git a/t/t8003-blame.sh b/t/t8003-blame.sh
index 13c25f1..ad834f2 100755
--- a/t/t8003-blame.sh
+++ b/t/t8003-blame.sh
@@ -144,4 +144,17 @@ test_expect_success 'blame path that used to be a directory' '
 	git blame HEAD^.. -- path
 '
 
+test_expect_success 'blame to a commit with no author name' '
+  TREE=`git rev-parse HEAD:`
+  cat >badcommit <<EOF
+tree $TREE
+author <noname> 1234567890 +0000
+committer David Reiss <dreiss@facebook.com> 1234567890 +0000
+
+some message
+EOF
+  COMMIT=`git hash-object -t commit -w badcommit`
+  git --no-pager blame $COMMIT -- uno >/dev/null
+'
+
 test_done
-- 
1.6.3.3

^ permalink raw reply related

* Re: Regression: git-svn clone failure
From: Eric Wong @ 2009-12-22 19:21 UTC (permalink / raw)
  To: Andrew Myrick; +Cc: git, sam
In-Reply-To: <8BD646EB-3F47-41F8-918C-19133CCCA89C@apple.com>

Andrew Myrick <amyrick@apple.com> wrote:
> [Resending because I forgot to make the message plain text]
> 
> I was testing the latest changes to git-svn pushed to Eric's repo
> (git://git.bogomips.org/git-svn) by cloning a few other projects that
> I work on, and one of those clones failed where it had succeeded with
> git 1.6.5.  The error message I received is:
> 
> W:svn cherry-pick ignored (/branches/BranchA:3933-3950) - missing 1 commit(s) (eg 3fc50d3a7e0f555547ab34bb570db47ce71e1abb)
> W:svn cherry-pick ignored (/branches/BranchB:3951-3970) - missing 1 commit(s) (eg 3beb9f2fde0a91aa0e8097e05f9054b23b221daf)
> W:svn cherry-pick ignored (/branches/BranchC:3971-3985) - missing 1 commit(s) (eg a7ae202254604f8a78cca391be36c58efc79eb20)
> Found merge parent (svn:mergeinfo prop): 8b2cf9e9250b5ff1fe47c68215d0a178cfe35a3b
> Found merge parent (svn:mergeinfo prop): 59f8c571ae77885469bb31f007b0048ee7812e07
> fatal: ambiguous argument '0..1': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions
> rev-list -1 0..1: command returned error: 128

Hi Andrew,

That looks like a simple error, does the following patch help?

diff --git a/git-svn.perl b/git-svn.perl
index 3670960..dba0d12 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3163,7 +3163,8 @@ sub find_extra_svn_parents {
 				next unless $new_parents[$i];
 				next unless $new_parents[$j];
 				my $revs = command_oneline(
-					"rev-list", "-1", "$i..$j",
+					"rev-list", "-1",
+					"$new_parents[$i]..$new_parents[$j]",
 				       );
 				if ( !$revs ) {
 					undef($new_parents[$i]);


Unfortunately I don't know my way around the rest of this code well
so I shall defer to Sam if it's something else...
-- 
Eric Wong

^ permalink raw reply related

* Re: Regression: git-svn clone failure
From: Andrew Myrick @ 2009-12-22 19:38 UTC (permalink / raw)
  To: Eric Wong; +Cc: git, sam
In-Reply-To: <20091222192115.GA10313@dcvr.yhbt.net>


On Dec 22, 2009, at 11:21 AM, Eric Wong wrote:
> That looks like a simple error, does the following patch help?
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 3670960..dba0d12 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3163,7 +3163,8 @@ sub find_extra_svn_parents {
> 				next unless $new_parents[$i];
> 				next unless $new_parents[$j];
> 				my $revs = command_oneline(
> -					"rev-list", "-1", "$i..$j",
> +					"rev-list", "-1",
> +					"$new_parents[$i]..$new_parents[$j]",
> 				       );
> 				if ( !$revs ) {
> 					undef($new_parents[$i]);
> 
> 

Worked like a charm; the fetch is proceeding now.  Thanks, Eric!

Do you know what the "svn cherry-pick ignored" warnings mean, and if it's something I should be concerned about?  This particular project is missing up to 65 commits at some revisions.

-Andrew

^ permalink raw reply

* Re: Regression: git-svn clone failure
From: Eric Wong @ 2009-12-22 20:26 UTC (permalink / raw)
  To: Junio C Hamano, Andrew Myrick; +Cc: git, sam
In-Reply-To: <B82A784D-C8D7-4DDF-AE63-390C7AE1CC2D@apple.com>

Andrew Myrick <amyrick@apple.com> wrote:
> On Dec 22, 2009, at 11:21 AM, Eric Wong wrote:
> > That looks like a simple error, does the following patch help?

<snip>

> Worked like a charm; the fetch is proceeding now.  Thanks, Eric!

Awesome, tanks for the feedback, Andrew.

I've pushed out a proper commit to git://git.bogomips.org/git-svn
for Junio (which also contains the previous pull request).

Andrew Myrick (1):
      git-svn: Remove obsolete MAXPARENT check

Eric Wong (3):
      git svn: fix --revision when fetching deleted paths
      update release notes for git svn in 1.6.6
      git svn: lookup new parents correctly from svn:mergeinfo

Sam Vilain (5):
      git-svn: expand the svn mergeinfo test suite, highlighting some failures
      git-svn: memoize conversion of SVN merge ticket info to git commit ranges
      git-svn: fix some mistakes with interpreting SVN mergeinfo commit ranges
      git-svn: exclude already merged tips using one rev-list call
      git-svn: detect cherry-picks correctly.

> Do you know what the "svn cherry-pick ignored" warnings mean, and if
> it's something I should be concerned about?  This particular project
> is missing up to 65 commits at some revisions.

Definitely a question for Sam :)

-- 
Eric Wong

^ permalink raw reply

* Re: [spf:guess] Re: Regression: git-svn clone failure
From: Sam Vilain @ 2009-12-22 20:35 UTC (permalink / raw)
  To: Eric Wong; +Cc: Andrew Myrick, git
In-Reply-To: <20091222192115.GA10313@dcvr.yhbt.net>

On Tue, 2009-12-22 at 11:21 -0800, Eric Wong wrote:
> That looks like a simple error, does the following patch help?
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 3670960..dba0d12 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3163,7 +3163,8 @@ sub find_extra_svn_parents {
>  				next unless $new_parents[$i];
>  				next unless $new_parents[$j];
>  				my $revs = command_oneline(
> -					"rev-list", "-1", "$i..$j",
> +					"rev-list", "-1",
> +					"$new_parents[$i]..$new_parents[$j]",
>  				       );

Yes, that is the intent.

Hrm, I'd have thought my test would have stepped over that code when it
merged in a branch which merged two an svn branch which included a merge
of another svn branch.  Obviously not!  I'll cook something up to cover
that..

Sam.

^ permalink raw reply

* Re: Regression: git-svn clone failure
From: Sam Vilain @ 2009-12-22 21:13 UTC (permalink / raw)
  To: Andrew Myrick; +Cc: Eric Wong, git
In-Reply-To: <B82A784D-C8D7-4DDF-AE63-390C7AE1CC2D@apple.com>

On Tue, 2009-12-22 at 11:38 -0800, Andrew Myrick wrote:
> Worked like a charm; the fetch is proceeding now.  Thanks, Eric!
> 
> Do you know what the "svn cherry-pick ignored" warnings mean, and if it's
>  something I should be concerned about?  This particular project is missing
>  up to 65 commits at some revisions.

With git, merge parent relationships imply (conceptually, anyway) that
all of the changes reachable from that branch are included in the
commit.  If someone is doing cherry-picking, then they are specifically
excluding some commits, so adding a merge parent to that branch isn't
right.  This is what the warning is saying.  It's happening every commit
because that section of code doesn't know whether a mergeinfo record is
new or not.

This wasn't happening with the old code, because it was simply not
detecting them correctly and adding merge parents anyway.

However in the case that someone is merging from another branch, merging
most commits, and only skipping a few, then it may make sense to record
it as a real merge.  Here we start getting into non-deterministic
conversion; I had to do this for perl.git, because the merge records
weren't reliable.  Basically I had the script, when the amount of merged
records was within a certain window, prompt me to ask me whether - based
on the change comment and outstanding files to merge - whether it should
be recorded as a real merge or not.  Then, depending on which option I
picked, it would write out to the commit message a note of which files
were not *actually* merged in that commit.

Something like the below change might be the right thing for you, it
might not - before using it, make sure you keep a complete copy of your
git-svn clone so you can restart if required.  Run it for a bit and
inspect the results.  Basically considers a 90% merge "good enough" and
records the differences in the log.  It could be possible to record the
cherry-pick information in the commit message, too - but we'd need to
also know which merge records were *added* in the current commit.
Actually, knowing that would make the whole thing much faster anyway, so
perhaps we need to bite the bullet and record it somewhere in the
metadata.

Anyway, this change may work - it doesn't break the test suite so that's
a good sign.  But hopefully it should give you an idea of the direction
things could have to take.  Perhaps you can see why I built a
high-performance fastimport importer for perl.git...

Subject: [PATCH] git-svn: consider 90% of a branch cherry picked to be a merge

Be slightly fuzzy when deciding if a branch is a merge or a cherry pick; in
some instances this might indicate intentionally skipping changes as not
required, as if they had performed a real merge and then skipped those
files.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
 git-svn.perl |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index f06e535..3064504 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2562,6 +2562,10 @@ sub do_git_commit {
 	unless ($self->no_metadata) {
 		print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
 		              or croak $!;
+		if ($log_entry->{merge_notes}) {
+			print $msg_fh "\ngit-svn-merge: $log_entry->{merge_notes}\n"
+		              or croak $!;
+		}
 	}
 	$msg_fh->flush == 0 or croak $!;
 	close $msg_fh or croak $!;
@@ -3027,10 +3031,11 @@ sub check_cherry_pick {
 	my @ranges = @_;
 	my %commits = map { $_ => 1 }
 		_rev_list("--no-merges", $tip, "--not", $base);
+	my $before = keys %commits;
 	for my $range ( @ranges ) {
 		delete @commits{_rev_list($range)};
 	}
-	return (keys %commits);
+	return ($before, keys %commits);
 }
 
 BEGIN {
@@ -3103,6 +3108,8 @@ sub find_extra_svn_parents {
 	my %excluded = map { $_ => 1 }
 		parents_exclude($parents, grep { defined } @merge_tips);
 
+	my @merge_warnings;
+
 	# check merge tips for new parents
 	my @new_parents;
 	for my $merge_tip ( @merge_tips ) {
@@ -3118,14 +3125,25 @@ sub find_extra_svn_parents {
 		       );
 
 		# double check that there are no missing non-merge commits
-		my (@incomplete) = check_cherry_pick(
+		my ($total, @incomplete) = check_cherry_pick(
 			$merge_base, $merge_tip,
 			@$ranges,
 		       );
 
-		if ( @incomplete ) {
+		if ( @incomplete and @incomplete > ($total*0.10) ) {
 			warn "W:svn cherry-pick ignored ($spec) - missing "
-				.@incomplete." commit(s) (eg $incomplete[0])\n";
+				.@incomplete."/$total commit(s) (eg $incomplete[0])\n";
+			# XXX - can't do this, it will appear every time;
+			# we need to know this record was added this commit
+			#push @merge_warnings, "picked: ". join(" ",
+			#     map { my $x=$_; $x=~
+			#	s{([a-f0-9]{12})[a-f0-9]+}{$1}g } @$ranges)
+		} elsif ( @incomplete ) {
+			warn "W:treating svn cherry-pick as merge "
+				.@incomplete."/$total commit(s) included\n";
+			push @merge_warnings, "skipped: ".
+				join(" ", map { substr $_, 0, 12 } @incomplete)
+				.")";
 		} else {
 			warn
 				"Found merge parent (svn:mergeinfo prop): ",
@@ -3151,6 +3169,7 @@ sub find_extra_svn_parents {
 		}
 	}
 	push @$parents, grep { defined } @new_parents;
+	return ( @merge_warnings ? join("; ", @merge_warnings) : undef );
 }
 
 sub make_log_entry {
@@ -3159,6 +3178,7 @@ sub make_log_entry {
 
 	my @parents = @$parents;
 	my $ps = $ed->{path_strip} || "";
+	my $merge_notes;
 	for my $path ( grep { m/$ps/ } %{$ed->{dir_prop}} ) {
 		my $props = $ed->{dir_prop}{$path};
 		if ( $props->{"svk:merge"} ) {
@@ -3166,7 +3186,7 @@ sub make_log_entry {
 				($ed, $props->{"svk:merge"}, \@parents);
 		}
 		if ( $props->{"svn:mergeinfo"} ) {
-			$self->find_extra_svn_parents
+			$merge_notes = $self->find_extra_svn_parents
 				($ed,
 				 $props->{"svn:mergeinfo"},
 				 \@parents);
@@ -3269,6 +3289,7 @@ sub make_log_entry {
 	$log_entry{email} = $email;
 	$log_entry{commit_name} = $commit_name;
 	$log_entry{commit_email} = $commit_email;
+	$log_entry{merge_notes} = $merge_notes;
 	\%log_entry;
 }
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: Regression: git-svn clone failure
From: Junio C Hamano @ 2009-12-22 21:38 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Andrew Myrick, Eric Wong, git
In-Reply-To: <1261516416.23944.44.camel@denix>

Sam Vilain <sam@vilain.net> writes:

> With git, merge parent relationships imply (conceptually, anyway) that
> all of the changes reachable from that branch are included in the
> commit.  If someone is doing cherry-picking, then they are specifically
> excluding some commits, so adding a merge parent to that branch isn't
> right.  This is what the warning is saying.  It's happening every commit
> because that section of code doesn't know whether a mergeinfo record is
> new or not.
> ...
> Subject: [PATCH] git-svn: consider 90% of a branch cherry picked to be a merge
>
> Be slightly fuzzy when deciding if a branch is a merge or a cherry pick; in
> some instances this might indicate intentionally skipping changes as not
> required, as if they had performed a real merge and then skipped those
> files.
>
> Signed-off-by: Sam Vilain <sam@vilain.net>

If I were _using_ git-svn (or any other tool), I would rather be forced to
see overlapping changes from both branches to sort out the conflict myself
when I merge such a cherry-picked history, rather than an automated but
unreliable operation that drops changes randomly, still records that
everything from the branch is now merged, and reports "everything is
peachy".

That sounds horrible, as you cannot trust your merges anymore.  I hope I
am mis-interpreting what you wrote above.

^ permalink raw reply

* [PATCH RFC 0/4] rebase -i: Add --refs option to rewrite heads within branch
From: Greg Price @ 2009-12-22 22:20 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano

This RFC series adds a "ref" command to rebase -i, and an option
--refs to generate "ref" commands in the original TODO file.  This
makes it easy to rebase a branch together with intermediate markers,
or a series of branches.

For example, in this situation:

      part1 part2 topic
        |     |     |
        v     v     v
  A--*--*--*--*--*--*
   \
    B <--master

we may want to rebase 'topic' onto the new master, rewriting the
intermediate branches 'part1', 'part2' to the corresponding new
commits.  This can be done with a sequence of "git rebase --onto"
commands, but it's tricky to get right.

With this series, the command
  $ git rebase -i --refs master topic
suffices to produce this result:

        part1 part2 topic
  A       |     |     |
   \      v     v     v
    B--*--*--*--*--*--*
    ^
    |
    master


These patches work for me.  Before I recommend the last two patches
for merge, they'll need the following additional work:
 - interoperate with rebase --abort
 - add documentation
 - probably add tests


The series begins with two patches that are independently useful, and
that I believe are ready for merge:
  pretty: Add %D for script-friendly decoration
  log --decorate=full: drop the "tag: " prefix
They make the functionality of 'git log --decorate' available in an
output format optimized for machine parsing.  I expect other people
will find this useful for other scripts.

Greg

^ permalink raw reply

* [PATCH 1/4] pretty: Add %D for script-friendly decoration
From: Greg Price @ 2009-12-22 22:22 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <20091222222032.GU30538@dr-wily.mit.edu>

When in a script or porcelain one wants to identify what refs point to
which commits in a series, the functionality of 'git log --decorate'
is extremely useful.  This is available with the %d format code in a
form optimized for humans, but for scripts a more raw format is better.
Make such a format available through a new format code %D.

Signed-off-by: Greg Price <price@ksplice.com>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   33 +++++++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 53a9168..b6b840e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -119,6 +119,7 @@ The placeholders are:
 - '%ct': committer date, UNIX timestamp
 - '%ci': committer date, ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
+- '%D': full ref names, like the --decorate=full option of linkgit:git-log[1]
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/pretty.c b/pretty.c
index 8f5bd1a..18ce2ff 100644
--- a/pretty.c
+++ b/pretty.c
@@ -582,21 +582,35 @@ static void parse_commit_message(struct format_commit_context *c)
 	c->commit_message_parsed = 1;
 }
 
-static void format_decoration(struct strbuf *sb, const struct commit *commit)
+
+static void format_decoration(struct strbuf *sb, const struct commit *commit,
+			      int decoration_style, const char *affixes[3])
 {
 	struct name_decoration *d;
-	const char *prefix = " (";
+	const char *affix = affixes[0];
 
-	load_ref_decorations(DECORATE_SHORT_REFS);
+	load_ref_decorations(decoration_style);
 	d = lookup_decoration(&name_decoration, &commit->object);
 	while (d) {
-		strbuf_addstr(sb, prefix);
-		prefix = ", ";
+		strbuf_addstr(sb, affix);
+		affix = affixes[1];
 		strbuf_addstr(sb, d->name);
 		d = d->next;
 	}
-	if (prefix[0] == ',')
-		strbuf_addch(sb, ')');
+	if (affix == affixes[1])
+		strbuf_addstr(sb, affixes[2]);
+}
+
+static void format_decoration_short(struct strbuf *sb, const struct commit *commit)
+{
+	const char *affixes[3] = {" (", ", ", ")"};
+	format_decoration(sb, commit, DECORATE_SHORT_REFS, affixes);
+}
+
+static void format_decoration_full(struct strbuf *sb, const struct commit *commit)
+{
+	const char *affixes[3] = {"", " ", ""};
+	format_decoration(sb, commit, DECORATE_FULL_REFS, affixes);
 }
 
 static void strbuf_wrap(struct strbuf *sb, size_t pos,
@@ -756,7 +770,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		                 : '>');
 		return 1;
 	case 'd':
-		format_decoration(sb, commit);
+		format_decoration_short(sb, commit);
+		return 1;
+	case 'D':
+		format_decoration_full(sb, commit);
 		return 1;
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
-- 
1.6.6.rc1.9.g2ad41.dirty

^ permalink raw reply related

* [PATCH 2/4] log --decorate=full: drop the "tag: " prefix
From: Greg Price @ 2009-12-22 22:22 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <20091222222032.GU30538@dr-wily.mit.edu>

The "tag: " prefix complicates machine parsing of decorations, so we
drop it from the output formats intended to be parsed by machine,
namely --decorate=full and the %D format code.

The prefix is helpful for a human reader to see that the ref is an
(annotated) tag, especially since we omit the "refs/tags/" prefix in
the default output of "git log --decorate".  In a script, however, it
is easy to use "git cat-file -t" to distinguish annotated tags from
commits when the distinction is relevant.

Signed-off-by: Greg Price <price@ksplice.com>
---
 log-tree.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 0fdf159..5eb6b00 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -22,17 +22,18 @@ static void add_name_decoration(const char *prefix, const char *name, struct obj
 
 static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
+	int short_refs = !!(!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS);
 	struct object *obj = parse_object(sha1);
 	if (!obj)
 		return 0;
-	if (!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS)
+	if (short_refs)
 		refname = prettify_refname(refname);
 	add_name_decoration("", refname, obj);
 	while (obj->type == OBJ_TAG) {
 		obj = ((struct tag *)obj)->tagged;
 		if (!obj)
 			break;
-		add_name_decoration("tag: ", refname, obj);
+		add_name_decoration(short_refs ? "tag: " : "", refname, obj);
 	}
 	return 0;
 }
-- 
1.6.6.rc1.9.g2ad41.dirty

^ permalink raw reply related

* [PATCH RFC 3/4] rebase -i: Add the "ref" command
From: Greg Price @ 2009-12-22 22:22 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20091222222032.GU30538@dr-wily.mit.edu>

This is useful for, e.g., rewriting a branch that has ancestor
branches along the way.

Signed-off-by: Greg Price <price@ksplice.com>
---
 git-rebase--interactive.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..25ac3e3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -367,6 +367,10 @@ do_next () {
 		warn
 		exit 0
 		;;
+	ref)
+		mark_action_done
+		git update-ref $sha1 HEAD # $sha1 is actually a refname
+		;;
 	squash|s)
 		comment_for_reflog squash
 
-- 
1.6.6.rc1.9.g2ad41.dirty

^ permalink raw reply related

* [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch
From: Greg Price @ 2009-12-22 22:23 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20091222222032.GU30538@dr-wily.mit.edu>

The new option --refs causes the TODO file to contain a "ref" command
for each head pointing to a selected commit, other than the one we are
already rebasing.  The effect of this is that when a branch contains
intermediate branches, like so:

      part1 part2 topic
        |     |     |
        v     v     v
  A--*--*--*--*--*--*
   \
    B <--master

a single command like "git rebase -i --refs master topic" suffices to
rewrite all the heads that are part of the topic, like so:

        part1 part2 topic
  A       |     |     |
   \      v     v     v
    B--*--*--*--*--*--*
    ^
    |
    master

Signed-off-by: Greg Price <price@ksplice.com>
---
 git-rebase--interactive.sh |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 25ac3e3..cccb031 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -18,6 +18,7 @@ git-rebase [-i] (--continue | --abort | --skip)
  Available options are
 v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
+refs               rewrite intermediate heads on branch
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
 m,merge            always used (no-op)
@@ -42,6 +43,7 @@ REWRITTEN="$DOTEST"/rewritten
 DROPPED="$DOTEST"/dropped
 PRESERVE_MERGES=
 STRATEGY=
+REWRITE_REFS=
 ONTO=
 VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
@@ -578,6 +580,9 @@ first and then run 'git rebase --continue' again."
 
 		output git reset --hard && do_rest
 		;;
+	--refs)
+		REWRITE_REFS=t
+		;;
 	-s)
 		case "$#,$1" in
 		*,*=*)
@@ -705,11 +710,14 @@ first and then run 'git rebase --continue' again."
 			REVISIONS=$ONTO...$HEAD
 			SHORTREVISIONS=$SHORTHEAD
 		fi
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --topo-order \
+		git rev-list $MERGES_OPTION --abbrev-commit --abbrev=7 \
+			--reverse --left-right --topo-order \
+			--pretty=format:"$(printf '%%m%%h %%s\n%%m%%D')" \
 			$REVISIONS | \
 			sed -n "s/^>//p" | while read shortsha1 rest
 		do
+			read refs
+
 			if test t != "$PRESERVE_MERGES"
 			then
 				echo "pick $shortsha1 $rest" >> "$TODO"
@@ -734,6 +742,16 @@ first and then run 'git rebase --continue' again."
 					echo "pick $shortsha1 $rest" >> "$TODO"
 				fi
 			fi
+
+			if test t = "$REWRITE_REFS"
+			then
+				for ref in $refs
+				do
+					test ${ref#refs/heads/} != $ref &&
+					test $ref != $(cat "$DOTEST"/head-name) &&
+					echo "ref $ref" >> "$TODO"
+				done
+			fi
 		done
 
 		# Watch for commits that been dropped by --cherry-pick
-- 
1.6.6.rc1.9.g2ad41.dirty

^ permalink raw reply related

* Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch
From: Junio C Hamano @ 2009-12-22 23:37 UTC (permalink / raw)
  To: Greg Price; +Cc: git, Johannes Schindelin
In-Reply-To: <20091222222316.GY30538@dr-wily.mit.edu>

Greg Price <price@ksplice.com> writes:

> The new option --refs causes the TODO file to contain a "ref" command
> for each head pointing to a selected commit, other than the one we are
> already rebasing.  The effect of this is that when a branch contains
> intermediate branches, like so:
>
>       part1 part2 topic
>         |     |     |
>         v     v     v
>   A--*--*--*--*--*--*
>    \
>     B <--master
>
> a single command like "git rebase -i --refs master topic" suffices to
> rewrite all the heads that are part of the topic, like so:
>
>         part1 part2 topic
>   A       |     |     |
>    \      v     v     v
>     B--*--*--*--*--*--*
>     ^
>     |
>     master
>
> Signed-off-by: Greg Price <price@ksplice.com>
> ---

Two comments and a half.

 - As decoration is a fairly expensive operation (which is the reason why
   loading_ref_decorations() is called lazily by format_decoration() in
   the first place, especially in repositories with tons of refs), you
   shouldn't give --format=%D to rev-list when the new feature is not
   asked for.

 - This seems to rewrite only branch heads; don't you want to allow users
   to rewrite lightweight tags and possibly annotated ones as well, by
   perhaps giving "--rewrite-refs=refs/heads/" or "--rewrite-refs=refs/"
   to limit what parts of the ref namespace to consider rewriting?

 - Otherwise the option should not be called "refs" but be named using the
   word "branch" to clarify that it affects _only_ branches.

Obviously the series also needs tests.

I also have to wonder if this feature should also handle a case like this:

                  side
                  |
                  V
                  *
                 /
        part1   *    topic
          |    /      |
          v   /       v
    A--*--*--*--*--*--*
     \
      B <--master

===>

                     side
                     |
                     V
                     *
                    /
           part1   *    topic
     A       |    /      |
      \      v   /       v
       B--*--*--*--*--*--*
       ^
       |
       master

especially if it were to be specific to branch management.

On the other hand, if the "partN" markers in your example workflow are
primarily meant to be used to mark places on a branch (as opposed to
arbitrary branch tips that independent development starting from them can
further continue), it would make a lot more sense to use lightweight or
annotated tags for them, and instead of "--refs" that rewrites only other
branch tips, it might make a lot more sense to have "--rewrite-tags" that
rewrites tags that point at the commits that are rewritten, without
touching any branch tip.

^ permalink raw reply

* Re: Regression: git-svn clone failure
From: Andrew Myrick @ 2009-12-22 23:50 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Eric Wong, git
In-Reply-To: <1261516416.23944.44.camel@denix>


On Dec 22, 2009, at 1:13 PM, Sam Vilain wrote:

> On Tue, 2009-12-22 at 11:38 -0800, Andrew Myrick wrote:
>> Worked like a charm; the fetch is proceeding now.  Thanks, Eric!
>> 
>> Do you know what the "svn cherry-pick ignored" warnings mean, and if it's
>> something I should be concerned about?  This particular project is missing
>> up to 65 commits at some revisions.
> 
> With git, merge parent relationships imply (conceptually, anyway) that
> all of the changes reachable from that branch are included in the
> commit.  If someone is doing cherry-picking, then they are specifically
> excluding some commits, so adding a merge parent to that branch isn't
> right.  This is what the warning is saying.  It's happening every commit
> because that section of code doesn't know whether a mergeinfo record is
> new or not.
> 
> This wasn't happening with the old code, because it was simply not
> detecting them correctly and adding merge parents anyway.

This makes perfect sense now.  Thank you for clarifying.  Unfortunately, I don't think the patch you provided will help my particular problem.  Allow me to elaborate.

As I mentioned before, my project's integration model is to create a separate branch for every change.  Specifically, we create a branch from a recent internal tag.  So, the model for a simple bug fix looks something like this:

            F---G  branch1
          /           \
        D  tag1  \    E  tag2
       /                 \  /
A---B                 C  trunk

Revision B on trunk was tagged with tag1.  A bug was found in that version, so a branch was created from tag1, a fix was committed to the branch, and then the branch was merged back to trunk.  Finally, trunk is tagged with tag2.

The "missing commit" messages show up when git svn fetch is fetching revision C.  It warns the there is a cherry-pick from branch1, and states that commits D and F are missing.  These commits are just copies, however; there is no code change.  The svn:mergeinfo property on trunk also only points at commit G.  Should git-svn be ignoring commits D and F, which are copy operations, not code changes?

Also of note is that we very, very rarely cherry-pick commits, and never directly from a branch to trunk.  Branches are always integrated back to trunk in their entirety.

-Andrew

^ permalink raw reply

* Re: Regression: git-svn clone failure
From: Sam Vilain @ 2009-12-23  0:09 UTC (permalink / raw)
  To: Andrew Myrick; +Cc: Eric Wong, git
In-Reply-To: <B39991E2-D632-4FD5-B5DA-0B6B502BBFC8@apple.com>

Andrew Myrick wrote:
> This makes perfect sense now.  Thank you for clarifying.  Unfortunately, I don't think the patch you provided will help my particular problem.  Allow me to elaborate.
>
> As I mentioned before, my project's integration model is to create a separate branch for every change.  Specifically, we create a branch from a recent internal tag.  So, the model for a simple bug fix looks something like this:
>
>             F---G  branch1
>           /           \
>         D  tag1  \    E  tag2
>        /                 \  /
> A---B                 C  trunk
>
> Revision B on trunk was tagged with tag1.  A bug was found in that version, so a branch was created from tag1, a fix was committed to the branch, and then the branch was merged back to trunk.  Finally, trunk is tagged with tag2.
>
> The "missing commit" messages show up when git svn fetch is fetching revision C.  It warns the there is a cherry-pick from branch1, and states that commits D and F are missing.  These commits are just copies, however; there is no code change.  The svn:mergeinfo property on trunk also only points at commit G.  Should git-svn be ignoring commits D and F, which are copy operations, not code changes?
>
> Also of note is that we very, very rarely cherry-pick commits, and never directly from a branch to trunk.  Branches are always integrated back to trunk in their entirety.
>   

Ok. Yes, I can see that. I guess what the code needs to do then is
figure out if the missing changes didn't touch the tree, and exclude
them if that happens.

in the check_cherry_pick function, try putting at the end something like;

for my $commit (keys %commits) {
if (has_no_changes($commit)) {
delete $commits{$commit};
}
}

Before the return. has_no_changes should be:

sub has_no_changes {
my $commit = shift;
# merges should always have no changes, but more
# importantly $commit~1 won't be defined for them, so
# don't proceed if that is the case.
my $num_parents = split / /, command_oneline(
qw(rev-list --parents -1 -m), $commit,
);
return 0 if $num_parents > 1;
return (command_oneline("rev-parse", "$commit^{tree}")
eq command_oneline("rev-parse", "$commit~1^{tree}"));
}

has_no_changes should also be memoized. Cherry picking a single commit
from a large unrelated branch will be slow (using cat-file --batch could
help here, but that's not something I can hack out off the cuff like
this), the first time, then it will remember whether particular commits
have changes or not.

Good luck,
Sam

^ permalink raw reply

* Re: Regression: git-svn clone failure
From: Sam Vilain @ 2009-12-23  0:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andrew Myrick, Eric Wong, git
In-Reply-To: <7vbphqzo2y.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Sam Vilain <sam@vilain.net> writes:
>   
>> With git, merge parent relationships imply (conceptually, anyway) that all of the changes reachable from that branch are included in the commit.  If someone is doing cherry-picking, then they are specifically excluding some commits, so adding a merge parent to that branch isn't right.
>> ...
>> Subject: [PATCH] git-svn: consider 90% of a branch cherry picked to be a merge
>>
>> Be slightly fuzzy when deciding if a branch is a merge or a cherry pick; ... might ... if ...
>>
>> Signed-off-by: Sam Vilain <sam@vilain.net>
>>     
>
> If I were _using_ git-svn (or any other tool), I would rather be forced to ... sort out the conflict myself ... rather than an automated but unreliable operation that drops changes randomly, ... and reports "everything is peachy".
>
> That sounds horrible, as you cannot trust your merges anymore.  I hope I
> am mis-interpreting what you wrote above.
>   

Welcome to the world of SVN, Junio. It's a world of sunshine and
happiness, pain and despair.

Sam.

^ permalink raw reply

* [PATCH] git svn: add (failing) test for a git svn gc followed by a  git svn mkdirs
From: Robert Zeh @ 2009-12-23  4:08 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

git svn gc will compress the unhandled.log files that git svn mkdirs reads,
causing git svn mkdirs to skip directory creation.
---
 t/t9152-svn-empty-dirs-after-gc.sh |   41 ++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100755 t/t9152-svn-empty-dirs-after-gc.sh

diff --git a/t/t9152-svn-empty-dirs-after-gc.sh
b/t/t9152-svn-empty-dirs-after-gc.sh
new file mode 100755
index 0000000..9ac6ea9
--- /dev/null
+++ b/t/t9152-svn-empty-dirs-after-gc.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Robert Zeh
+
+test_description='git svn creates empty directories, calls git gc,
makes sure they are still empty'
+. ./lib-git-svn.sh
+
+test_expect_success 'initialize repo' '
+	for i in a b c d d/e d/e/f "weird file name"
+	do
+		svn_cmd mkdir -m "mkdir $i" "$svnrepo"/"$i"
+	done
+'
+
+test_expect_success 'clone' 'git svn clone "$svnrepo" cloned'
+
+test_expect_success 'git svn gc runs' '
+	(
+		cd cloned &&
+		git svn gc
+	)
+'
+
+test_expect_success 'git svn mkdirs recreates empty directories after
git svn gc' '
+	(
+		cd cloned &&
+		rm -r * &&
+		git svn mkdirs &&
+		for i in a b c d d/e d/e/f "weird file name"
+		do
+			if ! test -d "$i"
+			then
+				echo >&2 "$i does not exist"
+				exit 1
+			fi
+		done
+	)
+'
+
+
+test_done
-- 
1.6.6.rc3.dirty

^ permalink raw reply related

* Re: git svn mkdirs ignores compressed unhandled.log files
From: Robert Zeh @ 2009-12-23  4:12 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20091217200852.GA5797@dcvr.yhbt.net>

On Dec 17, 2009, at 2:08 PM, Eric Wong wrote:

> Robert Zeh <robert.a.zeh@gmail.com> wrote:
>> It looks like there is a conflict between git svn gc and git svn
>> mkdirs.  The git svn mkdirs command only looks at unhandled.log files.
>> Shouldn't it also look at any compressed unhandled.log files too?
> 
> Hi Robert,
> 
> Yes, an oversight. Does this patch work for you? (Highly untested)
> 
> Would you mind writing a test case, been a bit busy with other stuff.
> Thanks.

Eric,

Your patch works for the existing t9146-git-svn-empty-dirs.sh test, and the test
I've sent as a patch in another email.

Robert

^ permalink raw reply

* [PATCH 1/2] t7800-difftool: Set a bogus tool for use by tests
From: David Aguilar @ 2009-12-23  5:27 UTC (permalink / raw)
  To: git; +Cc: gitster

If a difftool test has an error then running the git test suite
may end up invoking a GUI diff tool.  We now guard against this
by setting a difftool.bogus-tool.cmd variable.

The tests already used --tool=bogus-tool in various places so
this is simply ensuring that nothing ever falls back and
finds a real diff tool.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 t/t7800-difftool.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index fff6a6d..707a0f5 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -36,6 +36,7 @@ restore_test_defaults()
 	unset GIT_DIFFTOOL_NO_PROMPT
 	git config diff.tool test-tool &&
 	git config difftool.test-tool.cmd 'cat $LOCAL'
+	git config difftool.bogus-tool.cmd false
 }
 
 prompt_given()
@@ -71,7 +72,7 @@ test_expect_success 'custom commands' '
 
 # Ensures that git-difftool ignores bogus --tool values
 test_expect_success 'difftool ignores bad --tool values' '
-	diff=$(git difftool --no-prompt --tool=bogus-tool branch)
+	diff=$(git difftool --no-prompt --tool=bad-tool branch)
 	test "$?" = 1 &&
 	test "$diff" = ""
 '
-- 
1.6.2.5

^ permalink raw reply related


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