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

emerge: Make --root-deps install build deps to ROOT plus / for all EAPIs #1319

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chewi
Copy link
Member

@chewi chewi commented May 10, 2024

Rather than instead of / for EAPI 6 and below. This changes the behaviour, but it is arguably not a breaking change, as no installations are being dropped. The previous behaviour was highly likely to break anyway.

I'm not 100% sure about the changes in _complete_graph and _validate_blockers, but I think the idea here was to skip processing when you were discarding dependencies for /. We're not doing that for bare --root-deps any more, only rdeps specifically.

Rather than instead of / for EAPI 6 and below. This changes the
behaviour, but it is arguably not a breaking change, as no installations
are being dropped. The previous behaviour was highly likely to break
anyway.

Bug: https://bugs.gentoo.org/435066
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
@@ -8390,7 +8398,7 @@ def _complete_graph(self, required_sets=None):
for root in self._frozen_config.roots:
if root != self._frozen_config.target_root and (
"remove" in self._dynamic_config.myparams
or self._frozen_config.myopts.get("--root-deps") is not None
or self._frozen_config.myopts.get("--root-deps") != "rdeps"
Copy link
Member

Choose a reason for hiding this comment

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

Looking back at f241cdb, I'm not sure what I fixed. Maybe we could try removing --root-deps from this expression entirely (basically revert f241cdb).

@@ -8625,7 +8633,7 @@ def _validate_blockers(self):
dep_keys = Package._runtime_keys
for myroot in self._frozen_config.trees:
if (
self._frozen_config.myopts.get("--root-deps") is not None
self._frozen_config.myopts.get("--root-deps") != "rdeps"
and myroot != self._frozen_config.target_root
Copy link
Member

Choose a reason for hiding this comment

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

Looking back at e9740bc, I'm not sure about the optimization logic. Once again, maybe we could try removing --root-deps from this expression entirely (basically revert e9740bc).

It seems a9de7a2 which was committed around the same time was also reverted at some point.

Copy link
Member Author

@chewi chewi May 13, 2024

Choose a reason for hiding this comment

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

a9de7a2 was reverted in 814e82f with the introduction of EAPI 5-hdepend. No idea why though.

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 don't think keeping this does any harm, and I'd rather not break anything. Same for the above. I do intend to remove the rdeps option when we drop EAPI 6, although I was discussing the need for such an option that works with EAPI 7+ with someone today. rdeps is a terrible name, so I'd probably call it something else. I'm also considering other ways to solve their problem.

Copy link
Member

Choose a reason for hiding this comment

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

Okay that seems fair to preserve backward compat. Should these conditions use == rather than !=, in order to affect the --root-deps=rdeps case, like this?

-               or self._frozen_config.myopts.get("--root-deps") is not None
+               or self._frozen_config.myopts.get("--root-deps") == "rdeps"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants