Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 26 Mar 2018 14:26:53 -0700
From:      John Baldwin <jhb@freebsd.org>
To:        rgrimes@freebsd.org
Cc:        Ian Lepore <ian@freebsd.org>, Mark Johnston <markj@freebsd.org>, Andriy Gapon <avg@freebsd.org>, src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   Re: svn commit: r331461 - in user/markj/netdump/sys: kern netinet/netdump sys vm
Message-ID:  <2474043.hPEXl2xGZH@ralph.baldwin.cx>
In-Reply-To: <201803261842.w2QIgXFJ048428@pdx.rh.CN85.dnsmgr.net>
References:  <201803261842.w2QIgXFJ048428@pdx.rh.CN85.dnsmgr.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, March 26, 2018 11:42:33 AM Rodney W. Grimes wrote:
> > On Mon, 2018-03-26 at 10:12 -0700, John Baldwin wrote:
> > > On Saturday, March 24, 2018 08:40:24 AM Rodney W. Grimes wrote:
> > > > 
> > > > > 
> > > > > On Sat, Mar 24, 2018 at 02:17:02PM +0200, Andriy Gapon wrote:
> > > > > > 
> > > > > > On 24/03/2018 04:46, Rodney W. Grimes wrote:
> > > > > > > 
> > > > > > > I know this is on a private branch, but when/if it
> > > > > > > is merged this becomes part of the main line.
> > > > > > Not with svn, I think.
> > > > > > At least, the way we use it.
> > > > > Indeed, I have no intention to merge the branch directly. I'm using an
> > > > > svn branch so that it's marginally easier for others to test.
> > > > None the less as stated in:
> > > > 	https://svnweb.freebsd.org/base/projects/GUIDELINES.txt?view=markup
> > > > 
> > > > 12?	General guidelines:
> > > > 13?	
> > > > 14?	* Should be relevant to FreeBSD.
> > > > 15?	* Should be at least conceivably of interest to somebody else.
> > > > 16?	* Should be in a format that is suitable to merge into the base tree.
> > > > 17?	* Should be something that is worth people's time to read commit mail for.
> > > > 18?	* Write decent commit messages!
> > > > 
> > > > Thanks,
> > > We generally don't do that for user, etc. branches.??Merging from a
> > > projects/user branch into head in svn is often a disaster due to svn's
> > > limitations, so normally a projects/user branch is treated as a work area
> > > and the resulting diff is then hand-applied to head with a suitable commit
> > > message that describes the entire change.??This is similar to using something
> > > like 'git rebase' to rewrite history and compress a long tail of changes
> > > down to a small number of commits prior to merging to head.
> > > 
> > > You generally don't see these work branches in svn as most developers do them
> > > outside of svn in git, p4, hg, etc. due to svn's limitations.
> > > 
> > > For things that live permanently in user/projects (e.g. the code for core
> > > elections or the patches for freebsd-update), we do want standard commit
> > > messages.??However, I don't think we want to impose that on WIP branches
> > > that are later compressed down before merging.
> > > 
> > 
> > The support for armv6 happened this way... lots of out-of-tree and
> > project-branch work, with nothing much useful in the way of commit
> > messages, followed by a single massive import with a commit summary of
> > something like "import armv6 support".
> > 
> > Now when you try to search svn history for how something came to be,
> > all you can find out is that it was part of the incomprehensibly-huge
> > initial commit. ?You can't even figure out who to ask about something,
> > let alone why something was done.
> 
> Similiarly I have tried to find out who and why certain former
> hard link files in /usr/bin that are extremly security related got
> changed from hard links to symlinks breaking a security
> assumption.  A symlink can not contain the immutable attribute,
> 
> -r-sr-xr-x  1 root  wheel  schg,uarch 25488 Jul 20  2017 /usr/bin/chpass
> 
> lrwxr-xr-x   1 root  wheel          6 Jul 20  2017 chfn -> chpass
> lrwxr-xr-x   1 root  wheel          6 Jul 20  2017 chsh -> chpass
> lrwxr-xr-x   1 root  wheel          6 Jul 20  2017 ypchfn -> chpass
> lrwxr-xr-x   1 root  wheel          6 Jul 20  2017 ypchpass -> chpass
> lrwxr-xr-x   1 root  wheel          6 Jul 20  2017 ypchsh -> chpass
> 
> This removes the immutable attribe that use to exist on these files,
> making it easier for them to be altered.
> 
> This occurred on a branch that was merged by gjb, with out even
> a clue as to where that branch lives.
> 
> I suppose our only method to research these types of things is
> through the mail archives?  And if those commit messages are crap,
> that wont do us much good either, other than maybe yielding a
> name to contact.

I suspect it is due to pkgbase and hardlinks not being "portable" (though
tar can store them).  I would agree that wholesale merges of branches isn't
always the best strategy as I said in my reply to Ian.  An example of this
for me currently is my branch to add a debug server to bhyve.  I've split
out each of the new ioctls into its own commit / review with the initial
debug server being a separate commit (I still need to create an initial
review for that) rather than committing the entire thing as one blob.

If you look at the commits on that branch though on github, some of them
do have more details, some are skimpy (compile fixes, etc.) and it includes
some false starts that had to be redone (e.g. I first implemented vCPU
suspension using one method and then had to throw it away to implement the
current one in review).  That would be a messy history to wade through in
comparison to N reasonably-sized commits on head each with a suitable
commit log.

This is not just a FreeBSD thing either.  For my commits to GDB I end up
compressing my work branches down to a smaller number of well-formed commits
via 'git rebase' before pushing them into GDB's master.  There are no merge
commits that pull in branches in GDB, only pushes of "polished" commits.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2474043.hPEXl2xGZH>