Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on no-revbump for PEP517 #498

Closed
thesamesam opened this issue Nov 27, 2022 · 4 comments
Closed

Warn on no-revbump for PEP517 #498

thesamesam opened this issue Nov 27, 2022 · 4 comments
Assignees
Labels

Comments

@thesamesam
Copy link
Member

Enabling PEP517 mode is a non-conservative change and may result in installed files changing. It should always be done in a new revision.

@thesamesam
Copy link
Member Author

We need to handle this before too much of the tree is converted to PEP517, as it'll stop being useful then.

@thesamesam thesamesam self-assigned this Mar 4, 2023
@arthurzam
Copy link
Member

Currently the issue is that in the git checks, we use the not-parsed ebuilds, which results in my inability to know the state of PEP517. This is the reason for it being postponed.

@thesamesam
Copy link
Member Author

thesamesam commented Mar 4, 2023

I think we can do it with reduced accuracy if we regex-match the text for ^DISTUTILS_USE_PEP517 either having a changed value on the match on the right-hand-side of = or just not existing/existing in the changed ebuild (like we do for the copyright check in feed()) but I'm not sure how to get the old ebuild text.

@arthurzam
Copy link
Member

but I'm not sure how to get the old ebuild text.

if pkg not in added and old_pkg.rdepend != new_pkg.rdepend:
yield RdependChange(pkg=new_pkg)
old_slot, new_slot = old_pkg.slot, new_pkg.slot
if old_slot != new_slot:
slotmoves = (
x[1:] for x in self.repo.config.updates.get(new_pkg.key, ()) if x[0] == "slotmove"
)
for atom, moved_slot in slotmoves:
if atom.match(old_pkg) and new_slot == moved_slot:
break
else:
yield MissingSlotmove(old_slot, new_slot, pkg=new_pkg)

thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 4, 2023
Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 4, 2023
Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 4, 2023
Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 4, 2023
Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 4, 2023
Warn when DISTUTILS_USE_PEP517 is added or removed from an
ebuild without a new revision.

This is important because PEP517 affects a package's installed
files anyway and it's important to ensure that dependencies
work correctly against it, but also because e.g. config files
or other data files might either now be installed to the wrong
location or not installed at all. Developers must inspect
the image before/after (e.g. using iwdevtools) to avoid
issues.

Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
thesamesam added a commit to thesamesam/pkgcheck that referenced this issue Mar 5, 2023
Warn when DISTUTILS_USE_PEP517 is added or removed from an
ebuild without a new revision.

This is important because PEP517 affects a package's installed
files anyway and it's important to ensure that dependencies
work correctly against it, but also because e.g. config files
or other data files might either now be installed to the wrong
location or not installed at all. Developers must inspect
the image before/after (e.g. using iwdevtools) to avoid
issues.

Fixes: pkgcore#498
Signed-off-by: Sam James <sam@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants