On Fri, Jul 21, 2017 at 11:45:05AM -0700, Xin LI wrote:
> Hi, Stefan,
>
> Thanks for your feedback! Please see my response inline.
>
> On Fri, Jul 21, 2017 at 2:16 AM, Stefan Sperling <stsp_at_apache.org> wrote:
> > On Thu, Jul 20, 2017 at 08:01:55PM -0700, Xin LI wrote:
> > The FreeBSD repository is one of the best real world data sets available
> > to Subversion developers. I have referred to it several times while
> > working on the new conflict resolver planned for Subversion 1.10 and
> > have spent days running test merges against this repository.
> > The repository shows many difficult and interesting problems we would
> > normally only see in repositories that are locked up inside companies.
> >
> > While some of the problems I have seen are due to limitations of SVN,
> > I believe most problems FreeBSD developers have with SVN are self-made.
> > FreeBSD developers in general do not really understand the tool and
> > don't know how to work arounds its weaknesses. The FreeBSD handbook has
> > a section on Subversion which is full of bad advice and half-truths.
>
> I guess you mean this article?
> https://www.freebsd.org/doc/en/articles/committers-guide/subversion-primer.html
Yes.
> Could you please be more specific with the bad advices? (It would be
> very helpful if we could have fixed these documents and/or practices,
> if possible; I'll take a look at the premier to get the issues raised
> in EuroBSDCon 2014 tutorial addressed).
Sure. I'll go through it and point out things that stand out very positively
or negatively. Some information contained in this guide is simply outdated.
"5.3.2. Checkout"
"Note: Decreasing the depth of a working copy is not possible."
Long outdated advice. See the --depth=exclude option introduced in 1.6 (2009).
http://subversion.apache.org/docs/release-notes/1.6.html#sparse-directory-exclusion
"5.3.7. Adding and Removing Files"
"Before adding files, get a copy of auto-props.txt"
No longer necessary since Subversion 1.8 (2013). Use svn:auto-props instead.
http://subversion.apache.org/docs/release-notes/1.8.html#repos-dictated-config
"5.3.10. Diffs" should probably mention the --old and --new
syntax which can be used to compare arbitrary files and directories,
local and/or in the repository, including unversioned items.
This syntax is described in 'svn help diff', but is worth pointing out
anyway. http://subversion.apache.org/docs/release-notes/1.8.html#svn-diff
"5.3.12. Conflicts"
The list of possible values for the --accept option is incomplete.
Should mention that 'svn help' always has an up-to-date list.
The prominently mentioned 'mine-full' and 'theirs-full' options are
rarely needed. Most of the time, using the 'edit' command (e) at
the conflict prompt is the best path forward. In some cases (e.g.
files which use a line-by-line syntax) the 'merge' (m) command
provides a useful merge tool inspired by OpenBSD's version of sdiff(1).
http://subversion.apache.org/docs/release-notes/1.8.html#file-merge-tool
"5.4.1. Sparse Checkouts"
List of --depth option arguments is incomplete (lacks "exclude").
"5.4.2. Direct Operation"
"Certain operations can be performed directly on the repository without touching the working copy."
"merge" <-- wrong! Merge modifies the working copy unless --dry-run is used.
"5.4.3. Merging with SVN"
"5.4.3.1. About Merge Tracking"
" When set on a file, [mergeinfo] applies only to that file. When set on
a directory, it applies to that directory and its descendants (files and
directories) except for those that have their own svn:mergeinfo."
The above is the only part of this entire section which is 100% correct.
"It is not inherited. For instance, stable/6/contrib/openpam/ does not
implicitly inherit mergeinfo from stable/6/, or stable/6/contrib/."
This claim is true only if stable/6/contrib/openpam/ has an svn:mergeinfo
of its own. Otherwise it's just wrong. The remaining explanations provided
(which I won't quote here) about how mergeinfo inheritance works are vastly
oversimplified and can lead to misunderstandings and false conclusions.
I would delete them and instead refer interested readers to these articles
by Paul Burba which cover the subject in depth:
https://www.open.collab.net/community/subversion/articles/merge-info.html
http://blogs.collab.net/subversion/where-did-that-mergeinfo-come-from
(As an aside, it would be great to have some content from these articles
copied into the svnbook, provided Paul grants permission).
"5.4.3.3. Selecting the Source and Target for stable/10 and Newer"
"all merges are merged to and committed from the root of the branch"
Good!
However, this guide should also encourage (local) experimentation with
various merge tricks between paths when it comes to resolving tree conflicts.
While it often helps to think of directories in the repository as entire
branches when starting a merge, it can also help to think of a Subversion
repository as what it really is: a collection of revisions and paths with
ancestral relationships modelled via copies. When resolving tree conflicts,
this is right level of abstraction to use.
"5.4.3.4. Selecting the Source and Target for stable/9 and Older"
"For stable/9 and earlier, a different strategy was used, distributing mergeinfo around the tree so that merges could be performed without a complete checkout. This procedure proved extremely error-prone, with the convenience of partial checkouts for merges significantly outweighed by the complexity of picking mergeinfo targets. The procedure below describes this now-obsoleted process, "
No kidding! I'll just say that I'm happy this process was already changed.
"Never merge directly to a file.
Never, ever merge directly to a file.
Never, ever, ever merge directly to a file."
This was apparently written by someone who got burned by merge-tracking bugs
and lack of good tree conflict handling, didn't ask experts for help, and
resorted to just warning everyone to stay away from ever trying.
I have to admit that I have found myself having that same attitude towards
functions of tools I have had to use and probably didn't understand well :)
The correct answer is: It depends. As shown in my EuroBSDcon 2014 tutorial,
it may make sense to merge directly between files if resolving tree conflicts.
"5.4.3.5. Preparing the Merge Target"
"it is very important to never merge changes into a sparse working copy"
Yes! Read Paul Burba's articles to see why.
"5.4.3.9. Committing"
"Make sure to commit a top level directory to have the mergeinfo included
as well"
Good and essential advice.
"5.4.4.1.1. Flattening"
"The propdel bit is necessary because starting with 1.5, Subversion
will automatically add svn:mergeinfo to any directory that is copied or moved."
No longer true. Working-copy -> working-copy copies do not create mergeinfo.
"5.4.4.2.2. Importing into the Vendor Tree"
"the svn add and svn rm commands are used as needed:"
It makes sense to use 'svn move' instead of 'svn rm' followed by 'svn add'
where the vendor has moved files. This will become useful with Subversion 1.10.
"5.4.4.3. Merging to Head"
"^ is an alias for the repository path."
It's ^/, not just ^.
Why is the ^/ URL shortcut not mentioned more prominently much earlier,
e.g. in sections 5.4.2 or 5.4.2?
"5.4.6. Fixing Mistakes"
There is not such thing as the "repository journal" mentioned here.
I would suggest to replace "repository journal" with "repository".
"5.5. Some Tips"
"It is possible to automatically fill the "Sponsored by" and "MFC after" commit
log fields by setting "freebsd-sponsored-by" and "freebsd-mfc-after" fields in
the "[miscellany]" section of the ~/.subversion/config configuration file. For
example:
freebsd-sponsored-by = The FreeBSD Foundation
freebsd-mfc-after = 2 weeks"
I wish FreeBSD was putting some effort into getting their custom
features integrated in stock Subversion.
We managed to get FreeBSD's custom keywords patch integrated in 1.8.
http://subversion.apache.org/docs/release-notes/1.8.html#custom-keywords
Where is the rest? See http://subversion.apache.org/contributing.html#code
> > At EuroBSDcon 2014 I offered a special SVN tutorial for FreeBSD developers.
> > There were many participants but *nobody* of them were FreeBSD developers :(
> > There was a clear lack of interest for this kind of communication.
> > So the tutorial didn't go as planned and no interesting conversation happened.
> > You can still find the tutorial here (one of your own bad merges was discussed
> > in there, too, I believe): https://stsp.name/eurobsdcon2014-freebsd/
>
> I didn't knew this tutorial before, and thanks for taking time to do
> the analysis and sharing.
>
> So if I understood correctly, when doing a vendor import with file
> renames, in addition of doing 'svn mv's in the vendor tree [quick
> question: is there some semi-automated way to do this?], the developer
> should also handle the merge conflicts differently in the development
> trunk, by replaying each merges with:
>
> svn revert <old file> # Restore the file to before merge state
> svn rm <new file> # Remove new file in preparation of actual merge
> svn mv <old file> <new file> # Move old file to new location
> svn merge --accept postpone ^/vendor/.../<old file>@<vendor commit -1>
> ^/vendor/.../<new file>@<vendor commit> <new file>
>
> Then do a 'svn propdel svn:mergeinfo' on these new files?
>
> [Do we need to do similar changes when merging to stable/?]
Perhaps. I have looked at specific examples. I don't know how widely
any of them can be generalized. A competent developer should be able
to decide that for themselves.
My goal when writing the tutorial was to encourage experimentation with
solutions that go beyond just running "svn merge" and hoping for the best.
This requires some understanding of Subversion beyond what the FreeBSD
handbook can provide. But given the high standards FreeBSD sets for its
developers in other matters, I don't see why that should be a problem.
> And am I understanding correctly that the benefit of doing this is
> that the new files now get the same change history line from
> development trunk, instead of as a continue of the vendor history, and
> also avoided missing changes that happen in development trunk but not
> in vendor area (or development trunk)?
Yes, it seems more natural to me to have linear history on trunk.
Only people doing vendor imports should have to deal with their vendor branch.
Other people should never see their 'svn log' output go there.
One problem with my solution is that a merge like this indeed needs to
be scripted. Because Subversion is unable to match up the paths by itself
a lot of commands need to be run. This is unfortunate, but will improve
over time, with Subversion 1.10 being another huge step forward.
However, I do think that using some scripting to perform complex merges
is a healthy attitude and a nice tool developers should be using. If a merge
doesn't work automatically, try to script the merge so it runs automatically
starting from a clean working copy. Some trial and error should find the best
path forward in the given situation. Once you're happy, run the final script
once more with an "svn commit" command appended at the end.
A 1.10.0-alpha3 release will be out soon. I'd encourage you to try my
tutorial with this version, and let us know what you think.
Cheers,
Stefan
Received on 2017-07-22 13:07:38 CEST