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

_validate_deps: Discard configdict["pkg"]["USE"] #1297

Conversation

zmedico
Copy link
Member

@zmedico zmedico commented Mar 2, 2024

Since configdict["pkg"]["USE"] may contain package.use settings from config.setcpv, it is inappropriate to use here (bug 675748), so discard it. This is only an issue because configdict["pkg"] is a sub-optimal place to extract metadata from. This issue does not necessarily indicate a flaw in the Package constructor, since passing in precalculated USE can be valid for things like autounmask USE changes.

Bug: https://bugs.gentoo.org/675748

@zmedico zmedico requested a review from thesamesam March 2, 2024 20:25
@zmedico zmedico force-pushed the bug_675748_fix_doeebuild_validate_deps_invalid_USE branch from 980445e to 1232fde Compare March 3, 2024 07:13
Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance of a test case, or is it a pain because of ResolverPlayground? Does it work if we do it with global USE? I guess not? If not, it's fine obviously and I wouldn't worry about it.

@zmedico zmedico force-pushed the bug_675748_fix_doeebuild_validate_deps_invalid_USE branch from 1232fde to 3ea4c3e Compare March 3, 2024 19:27
# Demonstrate that settings.configdict["pkg"]["USE"] contains our arbitrary
# package.use setting in order to trigger bug 675748.
self.assertEqual(settings.configdict["pkg"]["USE"], arbitrary_package_use)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance of a test case, or is it a pain because of ResolverPlayground?

I was able to add some package.env and package.use settings in testDoebuild that create a state like the one which triggered bug 925625.

Does it work if we do it with global USE? I guess not? If not, it's fine obviously and I wouldn't worry about it.

By global USE do you mean like configdict["env"]["USE"] which would be like the setting from emerge --info? That value should never have any effect on _validate_deps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also realized that we could do this to avoid a redundant setcpv call in _validate_deps:

    # A USE calculation from setcpv should always be available here because
    # mysettings.mycpv is not None, so use it to prevent redundant setcpv calls.
    metadata["USE"] = mysettings["PORTAGE_USE"]

Since configdict["pkg"]["USE"] may contain package.use settings
from config.setcpv, it is inappropriate to use here (bug 675748),
so discard it. This is only an issue because configdict["pkg"] is
a sub-optimal place to extract metadata from. This issue does not
necessarily indicate a flaw in the Package constructor, since
passing in precalculated USE can be valid for things like
autounmask USE changes.

Bug: https://bugs.gentoo.org/675748
Signed-off-by: Zac Medico <zmedico@gentoo.org>
@zmedico zmedico force-pushed the bug_675748_fix_doeebuild_validate_deps_invalid_USE branch from 3ea4c3e to 6ce2be8 Compare March 3, 2024 19:53
@gentoo-bot gentoo-bot merged commit 6ce2be8 into gentoo:master Mar 3, 2024
12 checks passed
@zmedico zmedico deleted the bug_675748_fix_doeebuild_validate_deps_invalid_USE branch March 3, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants