Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 567920 - sys-apps/portage: "ebuild ... manifest" doesn't update Manifest mtime when it should
Summary: sys-apps/portage: "ebuild ... manifest" doesn't update Manifest mtime when it...
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Ebuild Support (show other bugs)
Hardware: All Linux
: Normal major (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 549914 557962 567830
  Show dependency tree
 
Reported: 2015-12-10 09:38 UTC by Lars Wendler (Polynomial-C) (RETIRED)
Modified: 2016-03-14 02:20 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
demonstrate_manifest_timestamps.sh (demonstrate_manifest_timestamps.sh,1.29 KB, text/plain)
2015-12-10 09:38 UTC, Lars Wendler (Polynomial-C) (RETIRED)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Wendler (Polynomial-C) (RETIRED) gentoo-dev 2015-12-10 09:38:16 UTC
Created attachment 418920 [details]
demonstrate_manifest_timestamps.sh

For a couple of versions portage sometimes won't update Manifest mtime even if it should.

This breaks package distribution via rsync when size of the Manifest file(s) also doesn't change and thus rsync won't update the Manifest file(s) on the receiver side.
The problem is not visible in the master repository. Only the rsync "clones" are affected unless rsync is called with --checksum option which is not the default.

I have attached a shell script which can be used to reproduce the problem.

The problem comes from ebuilds being renamed and thus won't have their mtime being changed as well. When you now re-create the Manifest file and the Manifest file happens to stay the same in size like before the update, rsync fails to see the file as changed.
Comment 1 Zac Medico gentoo-dev 2015-12-10 16:23:26 UTC
We'll have to modify this code to bump the mtime when the previous manifest differs from the new manifest:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=6dacd0ed9f6dc206f4932d42bbb36300f56e71f7
Comment 2 Zac Medico gentoo-dev 2015-12-10 16:29:53 UTC
(In reply to Zac Medico from comment #1)
> We'll have to modify this code to bump the mtime when the previous manifest
> differs from the new manifest:
> 
> https://gitweb.gentoo.org/proj/portage.git/commit/
> ?id=6dacd0ed9f6dc206f4932d42bbb36300f56e71f7

Actually, it's not possible for this code to detect the sort of change that we're looking for, because the previous manifest is a "thin" manifest. Maybe we should use thick manifests in git.
Comment 3 Zac Medico gentoo-dev 2015-12-13 21:06:17 UTC
@infra: I'm not sure how to handle this, other that to switch the git tree to thick manifests so that all changes will bump the manifest mtime.
Comment 4 Zac Medico gentoo-dev 2015-12-13 21:48:27 UTC
Maybe if we include directory mtimes in the max mtime calculation, then that will suffice, since directory mtimes change when files are removed or renamed.
Comment 5 dwfreed 2015-12-14 13:38:05 UTC
imo, this code should just be reverted, and Manifest mtimes should just become the time they're actually changed.  This code was added for infra, to produce stable mtimes of thin->thickened manifests.  I'm 99% finished with a patch to infra's rsync-gen script which will solve their issue through other means (just testing it at this point).  It does have the slight flaw that if the only change is deleting an ebuild that has not DIST entries, the Manifest won't be updated at all, but afaik having extra entries in Manifest is harmless.
Comment 6 Zac Medico gentoo-dev 2015-12-14 17:01:44 UTC
(In reply to dwfreed from comment #5)
> imo, this code should just be reverted, and Manifest mtimes should just
> become the time they're actually changed.

Well, if we include the directory mtimes in the max mtime calculation, then that keeps people's options open. There may be other parties interested in doing thin -> thick manifest conversion, and stable mtimes could be useful to them.

> This code was added for infra, to
> produce stable mtimes of thin->thickened manifests.  I'm 99% finished with a
> patch to infra's rsync-gen script which will solve their issue through other
> means (just testing it at this point).  It does have the slight flaw that if
> the only change is deleting an ebuild that has not DIST entries, the
> Manifest won't be updated at all, but afaik having extra entries in Manifest
> is harmless.

Extra DIST entries are ignored. However, portage's digestcheck function will return an error for EBUILD entries (in thick manifests) if the corresponding ebuild file is missing. The relevant error message looks like this:

"\n!!! A file listed in the Manifest could not be found: %s\n"
Comment 7 dwfreed 2015-12-15 04:39:17 UTC
(In reply to Zac Medico from comment #6)
> (In reply to dwfreed from comment #5)
> > imo, this code should just be reverted, and Manifest mtimes should just
> > become the time they're actually changed.
> 
> Well, if we include the directory mtimes in the max mtime calculation, then
> that keeps people's options open. There may be other parties interested in
> doing thin -> thick manifest conversion, and stable mtimes could be useful
> to them.

My gut is telling me that something will go horribly wrong with directory mtimes somewhere, but my brain is having a hard time coming up with a situation that would cause that.  (Mostly I'm trying to think of a scenario where the directory mtime would get bumped but no changes have been made in the directory that would affect the Manifest generation.)  The implementation I'm doing checks every AUX/EBUILD/MISC entry in the existing Manifest to see if it still exists on the filesystem if the Manifest has the newest mtime of anything in the directory.  If something has been deleted, it will re-run the Manifest generation and bump the Manifest mtime by 1 second.

> > This code was added for infra, to
> > produce stable mtimes of thin->thickened manifests.  I'm 99% finished with a
> > patch to infra's rsync-gen script which will solve their issue through other
> > means (just testing it at this point).  It does have the slight flaw that if
> > the only change is deleting an ebuild that has not DIST entries, the
> > Manifest won't be updated at all, but afaik having extra entries in Manifest
> > is harmless.
> 
> Extra DIST entries are ignored. However, portage's digestcheck function will
> return an error for EBUILD entries (in thick manifests) if the corresponding
> ebuild file is missing. The relevant error message looks like this:
> 
> "\n!!! A file listed in the Manifest could not be found: %s\n"

Yep, I ran into this when a git update introduced 4 meta packages that had ebuilds deleted, and repoman manifest-check started complaining because their EBUILD entries still existed.
Comment 8 Zac Medico gentoo-dev 2015-12-15 08:16:56 UTC
There's a patch in the following branch:

https://github.com/zmedico/portage/tree/bug_567920

I've posted it for review here:

https://archives.gentoo.org/gentoo-portage-dev/message/22bce4b3e4e51b8f545c7945413f213f
Comment 9 Zac Medico gentoo-dev 2015-12-15 08:29:30 UTC
(In reply to dwfreed from comment #7)
> My gut is telling me that something will go horribly wrong with directory
> mtimes somewhere, but my brain is having a hard time coming up with a
> situation that would cause that.  (Mostly I'm trying to think of a scenario
> where the directory mtime would get bumped but no changes have been made in
> the directory that would affect the Manifest generation.)

Ultimately, all mtime modifications are controllable. If it turns out that egencache --update-changelogs introduces undesirable directory mtime changes, then we can fix it. Likewise for any other obstacle that might arise.
Comment 12 Zac Medico gentoo-dev 2015-12-17 06:27:25 UTC
For completeness, Manifest._apply_max_mtime needs to be fixed to include the mtimes of directories that contain only directories.

For example, consider directories files/x and files/y, where files/ only contains the two directories x and y. If x and y are swapped, then only the mtime of the files/ directory will change. Therefore, we must ensure that directories which only contain directories are included in the max mtime calculation (they are currently not included).
Comment 13 Zac Medico gentoo-dev 2015-12-17 18:00:17 UTC
(In reply to Zac Medico from comment #12)
> For completeness, Manifest._apply_max_mtime needs to be fixed to include the
> mtimes of directories that contain only directories.

There's a patch for this in my branch:

https://github.com/zmedico/portage/tree/bug_567920

https://archives.gentoo.org/gentoo-portage-dev/message/7703036d859095e18745a2af0fa43984
Comment 14 Zac Medico gentoo-dev 2015-12-21 17:57:30 UTC
(In reply to Zac Medico from comment #13)
> (In reply to Zac Medico from comment #12)
> > For completeness, Manifest._apply_max_mtime needs to be fixed to include the
> > mtimes of directories that contain only directories.
> 
> There's a patch for this in my branch:
> 
> https://github.com/zmedico/portage/tree/bug_567920

This is in the master branch now:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=7d7248470d42f034184249f96995762b3b27f227

Before I commit v3 of my --stable-mtime patch, I'm waiting to see if there's any more feedback.
Comment 15 Zac Medico gentoo-dev 2015-12-26 23:58:26 UTC
(In reply to Zac Medico from comment #9)
> Ultimately, all mtime modifications are controllable. If it turns out that
> egencache --update-changelogs introduces undesirable directory mtime
> changes, then we can fix it.

There are no changes needed for egencache --update-changelogs, since it only re-generates the ChangeLog if its mtime is lower than the time of the most recent commit in the relevant directory.
Comment 16 Zac Medico gentoo-dev 2016-03-14 02:20:53 UTC
Fixed in 2.2.27.