subversion-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Sperling <s...@elego.de>
Subject Re: FreeBSD project and subversion.
Date Sat, 02 Feb 2013 17:07:42 GMT
On Fri, Feb 01, 2013 at 11:11:05AM -0500, Alfred Perlstein wrote:
> So I have two answers here:
> 
> 1) about mergeinfo
> It seems as if doing it all at the top can make merges over long
> haul internet very painful.

I acknowledge that a merge that runs across the entire FreeBSD src
tree can be slow.

For this and other reasons, it can make sense to create mergeinfo at
particular levels of the tree.

For example, it can make sense to run kernel code merges within src/sys,
and userland changes elsewhere. Subversion won't care. Keeping a consistent
pattern by documenting the expected roots used for merges and then using
those consistently will help create a clean log history.

I see that FreeBSD has already documented this, which is good!

The major hassle created by lots of subtree mergeinfo is property
changes cluttering out of commands such as 'svn log -v' and 'svn diff'.
This was particularly bad with Subversion 1.6, which updated any subtree
mergeinfo with a merge target, regardless of whether the merge actually
touched content of the files and directories which had their svn:mergeinfo
property updated. This has been addressed in Subversion 1.7, see:
http://subversion.apache.org/docs/release-notes/1.7.html#subtree-mergeinfo-recording
Also, there is no way to filter excessive property changes from the
output of 'svn diff'. This will be partly addressed in Subversion 1.8
by a new --ignore-properties option, see
http://subversion.apache.org/docs/release-notes/1.8.html#svn-diff
While this suppresses all property diffs and doesn't target
svn:mergeinfo in particular, it can help with producing diff output
that more usable than the default output in some cases.

> 2) about reintegrate
> I've attached the two messages that show what makes FreeBSD gun shy
> about merges to HEAD.
> Maybe these issues are resolved, or maybe the developer did
> something incorrect, or maybe something else entirely different
> happened.

See my response below.
It turns out that this wasn't a problem with --reintegrate at all,
but a user error made during a sync merge.

> Date: Fri, 1 Feb 2013 09:49:23 -0500
> From: John Baldwin <jhb@freebsd.org>
> To: Alfred Perlstein <alfred@freebsd.org>
> Subject: Fwd: Re: SVN merge question.
> X-Spam-Level: 
> User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64;
>  ; )
> Content-Type: Multipart/Mixed;  boundary="Boundary-00=_zX9CRbNRbX0geE6"
> Message-Id: <201302010949.23897.jhb@freebsd.org>
> 
> 
> -- 
> John Baldwin

> Date: Fri, 1 Jun 2012 13:56:03 -0400
> From: John Baldwin <jhb@freebsd.org>
> To: obrien@freebsd.org
> Cc: Grzegorz Bernacki <gber@freebsd.org>, Eitan Adler <eadler@freebsd.org>,
>  developers@freebsd.org
> Subject: Re: SVN merge question.
> User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p13; KDE/4.5.5; amd64;
>  ; )
> Content-Type: Text/Plain;  charset="iso-8859-1"
> Message-Id: <201206011356.03933.jhb@freebsd.org>
> 
> On Friday, June 01, 2012 1:40:29 pm David O'Brien wrote:
> > On Wed, May 16, 2012 at 11:00:48AM -0400, John Baldwin wrote:
> > > I more or less don't trust svn merge to DTRT here.  This was done with
> > > the cpuset branch merge up to HEAD and it broke the log history of many
> > > files in HEAD.
> > 
> > Specifically how did it break log history?
> 
> http://svnweb.freebsd.org/base/head/share/man/man4/geom_map.4?view=log
> 
> You have to walk up to the previous directory in svnweb and go back to
> change 222812 and then click on geom_map.4 to find its original log.
> 
> sys/dev/iicbus/ad7417.c was also busted this way.

Adrian Chadd originally added the man page to the head branch in this
revision:

[[[
------------------------------------------------------------------------
r221501 | adrian | 2011-05-05 16:43:35 +0200 (Thu, 05 May 2011) | 4 lines
Changed paths:
   M /head/share/man/man4/Makefile
   A /head/share/man/man4/geom_map.4

Add a manpage for geom_map(4).

Submitted by:   ray@dlink.ua
]]]

In a later revision, Attilio Rao re-added the same file to the largeSMP
branch, apparently while being immersed in a cloud of rage against svn :)

[[[
------------------------------------------------------------------------
r221716 | attilio | 2011-05-10 00:13:07 +0200 (Tue, 10 May 2011) | 4 lines
Changed paths:
   A /projects/largeSMP/share/man/man4/geom_map.4
   A /projects/largeSMP/sys/nfs/nfs_kdtrace.h
   A /projects/largeSMP/sys/sys/_stdint.h
   A /projects/largeSMP/tools/build/options/WITHOUT_GPIO
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote1.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote2.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote3.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote4.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote5.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote6.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote7.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote8.0
   A /projects/largeSMP/tools/regression/bin/sh/parser/dollar-quote9.0

Fix by hand files that aren't added automatically by svn.

BIG SUCKAGE!
]]]

It's not clear what happened here and why the files were missing.

The largeSMP branch was first created in r221273 as an unmodified
copy of ^/head@221272. That's good.

Just before the "big suckage" commit, Attilio merged r221699-221709
from ^/head in r221716. This merged range does not include r221501,
so the files had been missing from his branch much earlier.

In r221562, an earlier sync merge from ^/head to ^/projects/largeSMP
was committed, which includes r221501 in its svn:mergeinfo changes
(the range merged was r221494-221557).

Let's try this sync merge:

$ svn co svn://svn.freebsd.org/base/projects/largeSMP@221561
$ cd largeSMP
$ svn merge -r221494:221558 ^/head

The result is:

--- Merging r221495 through r221558 into '.':
A    tools/build/options/WITHOUT_GPIO
U    tools/build/options/WITHOUT_FLOPPY
U    tools/build/options/WITHOUT_FDT
U    tools/build/options/WITHOUT_ACCT
D    tools/build/options/WITH_GPIO
A    tools/regression/bin/sh/parser/dollar-quote1.0
A    tools/regression/bin/sh/parser/dollar-quote2.0
A    tools/regression/bin/sh/parser/dollar-quote3.0
A    tools/regression/bin/sh/parser/dollar-quote4.0
A    tools/regression/bin/sh/parser/dollar-quote5.0
A    tools/regression/bin/sh/parser/dollar-quote6.0
A    tools/regression/bin/sh/parser/dollar-quote7.0
A    tools/regression/bin/sh/parser/dollar-quote8.0
A    tools/regression/bin/sh/parser/dollar-quote9.0
[...]
--- Merging r221495 through r221558 into '.':
A    share/man/man4/geom_map.4
[...]

$ svn status  tools/regression/bin/sh/parser/           
A  +    tools/regression/bin/sh/parser/dollar-quote1.0
A  +    tools/regression/bin/sh/parser/dollar-quote2.0
A  +    tools/regression/bin/sh/parser/dollar-quote3.0
A  +    tools/regression/bin/sh/parser/dollar-quote4.0
A  +    tools/regression/bin/sh/parser/dollar-quote5.0
A  +    tools/regression/bin/sh/parser/dollar-quote6.0
A  +    tools/regression/bin/sh/parser/dollar-quote7.0
A  +    tools/regression/bin/sh/parser/dollar-quote8.0
A  +    tools/regression/bin/sh/parser/dollar-quote9.0
$ svn status share/man/man4
M       share/man/man4/Makefile
A  +    share/man/man4/geom_map.4
$ svn info share/man/man4/geom_map.4 | grep ^Copied\ From
Copied From URL: svn://svn.freebsd.org/base/head/share/man/man4/geom_map.4
Copied From Rev: 221558

So r221562 commit should have included these files, and we should be
seeing lines in 'svn log -v -r221562' such as:

   A /projects/largeSMP/share/man/man4/geom_map.4 (from /head/share/man/man4/geom_map:221558)

For some reason these changes weren't committed by Attilio.

I guess that Attilio had some local state in his working copy that
prevented some files from being added (unversioned files of the same
name, e.g. due to build artifacts?), and he realised the problem only
after committing his botched merge.

When he realised that files were missing from his branch, Attilio ran
'svn add' on unversioned files in his working copy, creating a separate
line of history for these files.

Subversion makes additions and deletions of objects explicit (unlike many
other version control systems), and trusts users to tell it the truth about
whether or not an objected added to a path is genuinely new (and not a copy
of something else).

While I cannot explain why this mistake was made in the first place,
I can show you how to fix it.

First, I'll show you what Attilio should have done when he realised
his mistake.

Merging these additions to largeSMP would have required first reverse-merging
revisions which added the missing files on ^/head, and then merging
these revisions from ^/head again.

For instance, to re-merge the missing addition of the geom_map man page,
Attilio could have done this:

$ svn merge -c-r221501 ^/head
### resolve tree conflict
$ svn merge -cr221501 ^/head
$ svn commit

We can try this on a working copy of ^/projects/largeSMP@221716 (the
revision where the new files had just been added to the branch):

$ svn merge -c-r221501 ^/head            
--- Reverse-merging r221501 into '.':
U    share/man/man4/Makefile
   C share/man/man4/geom_map.4
--- Recording mergeinfo for reverse merge of r221501 into '.':
 U   .
Summary of conflicts:
  Tree conflicts: 1
$ svn status
 M      .
M       share/man/man4/Makefile
      C share/man/man4/geom_map.4
      >   local edit, incoming delete upon merge
Summary of conflicts:
  Tree conflicts: 1

Resolving tree conflicts can be hard for novice users because the svn ui
is rather unforgiving when it comes to tree conflicts. Work is underway
to fix this but it will still take a while.

With the current UI, I would resolve this conflict as follows:

Subversion is warning us that the merge is trying to delete geom_map.4,
which we know is a file that was added to locally on this branch. Adding
this file to the branch was a mistake! We want it to be deleted. So we
can resolve the conflict by deleting the file:

$ svn rm share/man/man4/geom_map.4
D         share/man/man4/geom_map.4
$ svn resolved share/man/man4/geom_map.4
$ svn status
 M      .
M       share/man/man4/Makefile
D       share/man/man4/geom_map.4

Now we can merge r221501 again, this time in the forward direction of
history, to bring in the file we actually want:

$ svn merge -cr221501 ^/head             
--- Merging r221501 into '.':
G    share/man/man4/Makefile
A    share/man/man4/geom_map.4
--- Recording mergeinfo for merge of r221501 into '.':
 G   .
$ svn st
R  +    share/man/man4/geom_map.4
$ 

So the changeset scheduled for commit now now is a replacement of the
geom_map.4 file with its cousin which has the proper line of history:

$ svn info share/man/man4/geom_map.4 | grep ^Copied\ From
Copied From URL: svn://svn.freebsd.org/base/head/share/man/man4/geom_map.4
Copied From Rev: 221501

Since the largeSMP branch has already been merged back to ^/head, the
mistake now needs to be fixed up on ^/head. This could again be done
via appropriate reverse-merges and re-merges, but there's an easier way.
We can reconnect the file to its proper line of history by deleting the
file with the bad line of history and replacing it with the file with
the good line of history.

First, let's check for differences between the latest version of
the file in head and the version from before the largeSMP branch
was reintegrated:

$ svn diff ^/head/share/man/man4/geom_map.4 ^/head/share/man/man4/geom_map.4@r222012
Index: geom_map.4
===================================================================
--- geom_map.4  (revision 246254)
+++ geom_map.4  (revision 222012)

Property changes on: geom_map.4
___________________________________________________________________
Deleted: svn:eol-style
## -1 +0,0 ##
-native
\ No newline at end of property
Deleted: svn:mime-type
## -1 +0,0 ##
-text/plain
\ No newline at end of property

The only information we need to manually preserve are these two properties.
That's easy:

$ svn rm share/man/man4/geom_map.4
D         share/man/man4/geom_map.4
$ svn copy ^/head/share/man/man4/com_map.4@r222012 share/man/man4/geom_map.4
A         share/man/man4/geom_map.4
1.7.x-power $ svn status
RM +    share/man/man4/geom_map.4
$ svn propset svn:eol-style native share/man/man4/geom_map.4
property 'svn:eol-style' set on 'share/man/man4/geom_map.4'
$ svn propset svn:mime-type text/plain share/man/man4/geom_map.4
property 'svn:mime-type' set on 'share/man/man4/geom_map.4'
$ svn status
RM +    share/man/man4/geom_map.4
$ svn diff
Index: share/man/man4/geom_map.4
===================================================================
--- share/man/man4/geom_map.4   (revision 246254)
+++ share/man/man4/geom_map.4   (working copy)

Property changes on: share/man/man4/geom_map.4
___________________________________________________________________
Added: svn:mergeinfo
   Merged /projects/quota64/share/man/man4/geom_map.4:r184125-207707
   Merged /vendor/resolver/dist/share/man/man4/geom_map.4:r1540-186085

Now the only property changes are svn:mergeinfo changes. We don't really
care about these, but always commit them!!!

Notice how log history has been repaired:

$ svn log share/man/man4/geom_map.4
------------------------------------------------------------------------
r222012 | uqs | 2011-05-17 11:51:02 +0200 (Tue, 17 May 2011) | 4 lines

More thorough mdoc and language fixes.

Submitted by:   ru

------------------------------------------------------------------------
r222008 | uqs | 2011-05-17 10:12:59 +0200 (Tue, 17 May 2011) | 2 lines

Typos, wording and mdoc fixes.

------------------------------------------------------------------------
r221501 | adrian | 2011-05-05 16:43:35 +0200 (Thu, 05 May 2011) | 4 lines

Add a manpage for geom_map(4).

Submitted by:   ray@dlink.ua

------------------------------------------------------------------------


> > > I would just generate a diff and manually apply that to
> > > a HEAD checkout and commit that.  You could perhaps svn cp over new files
> > > from the nand branch to HEAD to preserve their history, but I worry that
> > > anything other than diff + patch for existing files risks hosing history.
> > 
> > WOAH!!  Please lets gain some new experience with 'svn merge' using
> > version 1.7.  We do 100's of merges a year at $WORK (with svn 1.6)
> > on a code base 10x that of FreeBSD -- it works.
> 
> I've never had problems with merging downstream into work branches.  I've
> only seen upstream merges blow up.
> 
> -- 
> John Baldwin
> -- 
> This mail is for the internal use of the FreeBSD project committers,
> and as such is private. This mail may not be published or forwarded
> outside the FreeBSD committers' group or disclosed to other unauthorised
> parties without the explicit permission of the author(s).
> 


> Date: Fri, 1 Feb 2013 09:50:37 -0500
> From: John Baldwin <jhb@freebsd.org>
> To: Alfred Perlstein <bright@mu.org>
> Cc: Peter Wemm <peter@wemm.org>
> Subject: Re: Fwd: Re: FreeBSD project and subversion.
> X-Spam-Level: 
> User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64;
>  ; )
> Content-Type: Text/Plain;  charset="iso-8859-15"
> Message-Id: <201302010950.37457.jhb@freebsd.org>
> 
> On Friday, February 01, 2013 7:53:57 am Alfred Perlstein wrote:
> > John and Peter, both of you are +inf more knowledgeable about svn than I am.
> > 
> > I see we still try to minimize svn mergeinfo from the FAQ ("Selecting 
> > the Source and Target")
> > http://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-
> guide/subversion-primer.html#AEN771
> > 
> > I know I've seen some emails explaining the reasoning behind this but I 
> > can't find them.  What would the effect of bringing mergeinfo to the top be?
> > 
> > Possible problems:
> > 1) it would get very large
> > 2) if we ever were to split up the repo it would be a problem.
> > 3) ... ?
> 
> It makes merges from across the continental US take a long time.
> 
> > Additionally, what are our concerns about the --reintegrate option 
> > (which was added (or at least improved) after we switched)
> 
> It trashes history if SVN mucks it up (and it can).  This has already happened 
> at least once, and I've noted that in a thread on developers.

I would guess that these were similar user errors. Use of the --reintegrate
option is very important. Failure to use it can trigger spurious conflicts
during the merge back to ^/head, and can even exacerbate problems during
future merges from/to any branch.

Subversion 1.8 will finally deprecate this option, which is a _horrible_ UI
quirk because it requires users to reason about the directions their merges
are taking. Subversion 1.8 will automatically figure whether the option is
needed: http://subversion.apache.org/docs/release-notes/1.8.html#auto-merge

But while you're still using Subversion 1.7, please use --reintegrate
when merging branches back into their parent branch.

If this doesn't work in the expected way, mail our users@ list, 
describe the problem, and we'll help you. In the long run you will
have a much better Subversion experience this way.

Mime
View raw message