As subversion does not recognize $Header$, it is unnecessary to split the commits into ebuild commit and Manifest commit due to its existence. Especially that subversion handles revisions repository-wide and this introduces a single broken rev.
It currently uses egrep for Header|Id, so it seems like we'll want to convert this code to python and split the Header and Id matches into separate groups. I wonder if cvs supports $Id$
Thanks, this is in git now: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=579d11b4def198d9ce48dd4e73d6f805e501dd81
hmmm, weird, I thought prefix repoman already didn't do this, and that we pushed all of that to master at some point.
(In reply to comment #0) > As subversion does not recognize $Header$ Subversion >=1.6 supports Header keyword. Please note that given keyword needs to be in svn:keywords property to enable keyword substitution. Maybe Portage should check if given keywords are in svn:keywords property before using egrep. Subversion supports the following keywords: LastChangedRevision, Rev, Revision LastChangedDate, Date LastChangedBy, Author HeadURL, URL Id Header
By the way, I start to think that our svn:keywords check didn't work anyway, as I don't see any props in Sunrise and repoman did dual-commit anyway.
it works in Prefix for sure. repoman there only commits dual if the ebuild has one of the props set, and something is expanded
This is fixed in 2.2_rc93 and 2.1.9.15.
I meant that unconditional ignoring of $Header$ is a regression.
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commitdiff;h=e9ef0bcf6231efa4d04342bf4e75086d2d7f79b9 tries to properly fix this bug.
(In reply to comment #8) > I meant that unconditional ignoring of $Header$ is a regression. Yeah, and repoman did split the commits anyway...
Ok, tested it. Repoman says: * 2 files being committed... 0 have headers that will change. but the commit is still being broken into ebuild commit and Manifest commit.
Ok, the actual issue is something different. It is signed Manifest commits which (for some reason) are split independently of keywords existence.
Created attachment 289035 [details, diff] portage-repoman-split-manifest.patch The way signed manifest commits is handled for DVCSes (and now subversion/CVS with this patch) is that the normal commit stage is skipped, the Manifest is signed, and then a commit including the signed Manifest and normal files is made. With this patch, the normal commit stage is now skipped for CVS/subversion if no header changes are detected in files being committed. Tested to successfully fix unnecessary split Manifest commits in sunrise overlay and to properly detect the need for a split Manifest commit on gentoo-x86.
(In reply to comment #13) > Created attachment 289035 [details, diff] > portage-repoman-split-manifest.patch I'm pretty sure that this patch doesn't correctly interact with this part that comes later: if vcs in ['git', 'bzr', 'hg'] or manifest_commit_required or signed: myfiles = mymanifests[:] if vcs in ['git', 'bzr', 'hg']: myfiles += myupdates myfiles += myremoved myfiles.sort() I think you want it to do the myfiles += myupdates and myfiles += myremoved part for cvs and svn if the myupdates and myremoved commits get skipped earlier.
(In reply to comment #14) > (In reply to comment #13) > > Created attachment 289035 [details, diff] > > portage-repoman-split-manifest.patch > > I'm pretty sure that this patch doesn't correctly interact with this part that > comes later: ... > I think you want it to do the myfiles += myupdates and myfiles += myremoved > part for cvs and svn if the myupdates and myremoved commits get skipped > earlier. You're right. I feel stupid for committing a signed Manifest without the relevant files in myupdates and myremoved to sunrise, at least it didn't affect sunrise's reviewed branch ;-).
Created attachment 289039 [details, diff] portage-repoman-split-manifest-r1.patch This which seems to work at least for SVN with and without keyword changes. I think it should work with CVS too. I also poked at the code for category-wide commits, I think that my first patch would allow a category-wide commit to commit unsigned Manifests while not committing the updated/removed ebuilds.
(In reply to comment #16) > Created attachment 289039 [details, diff] > portage-repoman-split-manifest-r1.patch > > @@ -2410,7 +2410,7 @@ else: > print("* aborting commit.") > sys.exit(1) > > - if vcs in ('cvs', 'svn') and (myupdates or myremoved): > + if vcs in ('cvs', 'svn') and (myupdates or myremoved) and (not sign_manifests or myheaders): > myfiles = myupdates + myremoved > if not myheaders and not sign_manifests: > myfiles += mymanifests > @@ -2557,8 +2557,10 @@ else: I'm not sure if the "not sign_manifests" condition belongs there. We have to review that condition carefully. > I also poked at the code for category-wide commits, I think that my first patch > would allow a category-wide commit to commit unsigned Manifests while not > committing the updated/removed ebuilds. That category-wide code no longer exists in the master branch. I completely removed it a week ago: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a24bcfcfad95fcf4b71064e386fb6272f41ff49a
Created attachment 289041 [details, diff] portage-repoman-split-manifest-r2.patch (In reply to comment #17) > I'm not sure if the "not sign_manifests" condition belongs there. We have to > review that condition carefully. I don't remember my rationale for having that there, but removing it implies that the Manifest should never ever be committed in this first commit pass. And it also implies that the second commit pass should be unconditional. Thus the restructuring of this patch, but I don't reindent everything... > That category-wide code no longer exists in the master branch. I completely > removed it a week ago: > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a24bcfcfad95fcf4b71064e386fb6272f41ff49a I seem to forget something different every time I try to use git ;-). Now there is no reason for that manifest_commit_required variable to exist at all... Thanks for the patience, reviews, and insights!
(In reply to comment #18) > Created attachment 289041 [details, diff] > portage-repoman-split-manifest-r2.patch Thanks, this is in git: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=1cdf41d1b5c17ac593f0d61a90b8d2a7db6b494d > Thanks for the patience, reviews, and insights! You're welcome. :)
Here's a somewhat related tweak for commit logic when using thin-manifests: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=ae260804da9689c69fad3775d8a4be31b34a087d
Here's a tweak for the case where files are only being removed: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=d2249e5f44ee4ae22304b4d120c4be43c6ab45d4
This is fixed in 2.1.10.23 and 2.2.0_alpha63.
