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.

Sign up for GitHub

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

Jump to bottom

Fix some nondeterminism when generating binpkgs #1098

Closed
wants to merge 3 commits into from
Closed

Fix some nondeterminism when generating binpkgs #1098

wants to merge 3 commits into from

Conversation

ismell
Copy link

@ismell ismell commented Sep 19, 2023

  • config: Don't directly modify FEATURES
  • bin/phase-functions: Move du stats into subshell

vapier
vapier approved these changes Oct 3, 2023
View reviewed changes
lib/portage/package/ebuild/config.py Show resolved Hide resolved
floppym
floppym requested changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@floppym floppym left a comment

Choose a reason for hiding this comment

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

Please review our copyright policy and add Signed-off-by.

Raul E Rangel added 3 commits October 11, 2023 12:49
config: Don't directly modify FEATURES
06f1bad 
Iterating on self.features returns items in a non-deterministic order,
it also leaves self.features out of sync. If we instead use the `remove`
method, the values get sorted and synchronized correctly.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
bin/phase-functions: Move du stats into subshell
aa20b8c 
These variables are only used inside this subshell. This avoids
polluting the environment.

Apparently this calculation isn't hermetic. I'm not sure why:

@@ -268,10 +268,10 @@
 declare -x cros_setup_hooks_run="booya"
 declare -a exclude_hermetic=([0]="--exclude-non-hermetic")
 declare -- f
-declare -a isz=([0]="264" [1]="/var/tmp/portage/dev-libs/libffi-3.1-r8/image/")
-declare -a nsz=([0]="2803" [1]="/var/tmp/portage/dev-libs/libffi-3.1-r8/work")
+declare -a isz=([0]="16" [1]="/var/tmp/portage/dev-libs/libffi-3.1-r8/image/")
+declare -a nsz=([0]="2599" [1]="/var/tmp/portage/dev-libs/libffi-3.1-r8/work")
 declare -- phase_func
 __eapi6_src_install ()

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
config: Sort USE_EXPAND variables
05e7923 
Without the sort, we end up extending the USE_EXPAND variables with
a non deterministic ordering. This makes binary package creation non
hermetic.

i.e.,
```
-declare -x PYTHON_TARGETS="python3_8 python3_7 python3_6 python3_10 python3_9"
+declare -x PYTHON_TARGETS="python3_8 python3_9 python3_10 python3_7 python3_6"
```

Assuming `PYTHON_TARGETS=python3_8` in make.conf, after this change we
get:
```
declare -x PYTHON_TARGETS="python3_8 python3_10 python3_6 python3_7 python3_9"
```

Ideally we would completely sort the USE_EXPAND variables, but the
LINGUAS variable appears to need a very specific ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
@ismell
Copy link
Author

ismell commented Oct 11, 2023

Oops. I added the signed off line and also added an additional patch for some non-determinism when generating USE_EXPAND variables.

@thesamesam
Copy link
Member

Thank you!

@thesamesam
Copy link
Member

@ismell Please take a look at the issue mentioned in b150419.

@zmedico
Copy link
Member

zmedico commented Oct 13, 2023

@ismell Please take a look at the issue mentioned in b150419.

The issue is that self.features.remove("test") creates a persistent setting in self._features_overrides which is not reverted by self.reset(). Meanwhile, direct modification of the FEATURES variable just changes self.configdict["env"] which is reverted by each call to self.reset().

@ismell
Copy link
Author

ismell commented Oct 13, 2023 via email

palao pushed a commit to palao/portage that referenced this pull request Oct 16, 2023
@palao
config: Sort USE_EXPAND variables
3fa49b2 
Without the sort, we end up extending the USE_EXPAND variables with
a non deterministic ordering. This makes binary package creation non
hermetic.

i.e.,
```
-declare -x PYTHON_TARGETS="python3_8 python3_7 python3_6 python3_10 python3_9"
+declare -x PYTHON_TARGETS="python3_8 python3_9 python3_10 python3_7 python3_6"
```

Assuming `PYTHON_TARGETS=python3_8` in make.conf, after this change we
get:
```
declare -x PYTHON_TARGETS="python3_8 python3_10 python3_6 python3_7 python3_9"
```

Ideally we would completely sort the USE_EXPAND variables, but the
LINGUAS variable appears to need a very specific ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Closes: gentoo#1098
Signed-off-by: Sam James <sam@gentoo.org>
@thesamesam
Copy link
Member

thesamesam commented Oct 20, 2023 •
edited

Thanks! Please do.

palao pushed a commit to palao/portage that referenced this pull request Oct 22, 2023
@palao
config: Sort USE_EXPAND variables
4c58c26 
Without the sort, we end up extending the USE_EXPAND variables with
a non deterministic ordering. This makes binary package creation non
hermetic.

i.e.,
```
-declare -x PYTHON_TARGETS="python3_8 python3_7 python3_6 python3_10 python3_9"
+declare -x PYTHON_TARGETS="python3_8 python3_9 python3_10 python3_7 python3_6"
```

Assuming `PYTHON_TARGETS=python3_8` in make.conf, after this change we
get:
```
declare -x PYTHON_TARGETS="python3_8 python3_10 python3_6 python3_7 python3_9"
```

Ideally we would completely sort the USE_EXPAND variables, but the
LINGUAS variable appears to need a very specific ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Closes: gentoo#1098
Signed-off-by: Sam James <sam@gentoo.org>
@ismell
Copy link
Author

ismell commented Oct 23, 2023

Thanks for pushing the new commit!

@thesamesam
Copy link
Member

i think that's on one of @palao's branches but not sure where...

ismell pushed a commit to ismell/portage that referenced this pull request Mar 21, 2024
Reapply "config: Don't directly modify FEATURES"
2881bf8 
This reverts commit b150419.

It turns out we need to keep the open coded version to avoid creating
a persistent setting.
See gentoo#1098 (comment)

This change just adds a sorted() so we get deterministic ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
@ismell ismell mentioned this pull request Mar 21, 2024
Closed
ismell pushed a commit to ismell/portage that referenced this pull request Mar 21, 2024
Reapply "config: Don't directly modify FEATURES"
1d0a152 
This reverts commit b150419.

It turns out we need to keep the open coded version to avoid creating
a persistent setting.
See gentoo#1098 (comment)

This change just adds a sorted() so we get deterministic ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
gentoo-bot pushed a commit that referenced this pull request Mar 24, 2024
@zmedico
Reapply "config: Don't directly modify FEATURES"
72f41d0 
This reverts commit b150419.

It turns out we need to keep the open coded version to avoid creating
a persistent setting.
See #1098 (comment)

This change just adds a sorted() so we get deterministic ordering.

Bug: https://bugs.gentoo.org/914441
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Closes: #1312
Signed-off-by: Zac Medico <zmedico@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulm ulm ulm left review comments

@vapier vapier vapier approved these changes

@floppym floppym floppym approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@ismell @thesamesam @zmedico @vapier @ulm @floppym