On Wed, Nov 27, 2013 at 6:55 PM, Julian Foad <julianf...@btopenworld.com>wrote:
> Stefan Fuhrmann wrote: > >>>>> As of r1516665, work on this branch has been completed. > >>>>> Please review. See the BRANCH-README for the list of > >>>>> major changes. > > I have not reviewed, but looked at how to review it. > > First I am looking for an overall description of the changes. The > BRANCH-README says: > > [[[ > This is an integration / development branch to copy / implement and > review the logical addressing feature originally developed on the > fsfs-format7 branch. After review and after the fsfs-improvements > branch got merged into /trunk, this whole branch will get merged > into /trunk as well. > > Features to implement here: > > * support for logical addressing > * switch to format 7; use log addressing after for new shards created > after svnadmin upgrade > * block read / data prefetch for f7 rev / pack file data > * reorder data upon pack > * add checksums to index data > * improve verification to cover all f7 rev / pack file data > > More on this here: > http://svn.haxx.se/dev/archive-2013-06/0755.shtml > http://svn.haxx.se/dev/archive-2013-07/0059.shtml > ]]] > > I don't know how up to date that is, but anyhow it doesn't precisely > describe the changes. I only used it as a TODO list. > If you could perhaps, starting from this text, turn it into the standard > log message format that you would eventually include in the merge log > message, that would be much better. I am in favour of merges having a log > message that really describes what's being merged, since trying to > understand it from scanning the original series of branch commits can be > very inefficient. > r1546928 on the fsfs-improvements branch now contains a summary of all f7 changes. It does not give function-level detail but describes the intended changes on a per-file level. > Then, to see the actual changes, we can run a diff if we first discover > the trunk revision to diff against: > $ svn mergeinfo ^/subversion/trunk \ > ^/subversion/branches/fsfs-improvements > youngest common ancestor > | last full merge > | | tip of branch > | | | repository path > > 1499980 1545953 1546120 > | | | > -------| |------------ subversion/trunk > \ \ > \ \ > --| |------------ subversion/branches/fsfs-improvements > | > 1546120 > > $ svn mergeinfo ^/subversion/branches/fsfs-improvements \ > ^/subversion/branches/log-addressing > youngest common ancestor > | last full merge > | | tip of branch > | | | repository path > > 1509278 1545955 1546118 > | | | > -------| |------------ subversion/branches/fsfs-improvements > \ \ > \ \ > --| |------------ subversion/branches/log-addressing > | > 1546118 > > $ svn diff --old=^/subversion/trunk@1545953 \ > --new=^/subversion/branches/log-addressing > d > > I find it easier to have the changes in a trunk WC and use diff-aware > editors than to browse the patch file directly. I can apply this patch: > > $ svn patch d > D subversion/libsvn_subr/md5.h > D subversion/libsvn_subr/sha1.c > D subversion/libsvn_subr/sha1.h > U build.conf > A BRANCH-README > U subversion/libsvn_fs_base/fs.c > [...] > > OK, fine. (I got a minor text conflict in a test, presumably due to a > recent trunk change that's not yet sync'd to the branch.) > > Alternatively, I usually just run the proposed merge. At first I tried to > merge it directly to a trunk WC, using automatic merge: "svn merge > ^/subversion/branches/log-addressing". No go. Major conflicts. > > The branching structure is like this (showing only the most recent merges): > > ------------------------ trunk > \ ... \ > +-----------+------- branches/fsfs-improvements > \ ... \ > +-----------+--- branches/log-addressing > > Subversion's automatic merge doesn't support tracking merges going round a > cycle (trunk -> fsfs-imp. -> log-addr. -> trunk). That's the basic reason > it produces conflicts when we try. The "proper" way to merge this back to > trunk, in terms of what Subversion's automatic merge supports, is first to > merge it to fsfs-improvements. That works fine in my trial. (I only ran the > merge, didn't try to build the resulting code.) Then I suppose a merge from > fsfs-improvements to trunk would work fine. > Yes, that is a limitation in the way we currently use merge tracking: Catch-up merges must follow the branch hierarchy. Cherry-picks are slightly less critical since one would expect them to cause (trivial) conflicts anyways when resync'ing the source branch with the target. It looks like fsfs-improvements itself currently has no changes on it that > have not been merged to trunk (apart from BRANCH-README and a mergeinfo > property). > > Stefan, does all this match your understanding? Would it make sense for > you to merge this to fsfs-improvements right away, to make it easier for > other people to run a trial merge to trunk? And then of course the proposal > would be to merge fsfs-improvements to trunk, rather than log-addressing to > trunk. (I'm assuming this is the only active sub-branch off the > fsfs-improvements branch at the moment.) > That is correct. I just merged everything into fsfs-improvements and dropped the sub-branch. -- Stefan^2.