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

PoC: Do not export A #1357

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

Conversation

Flowdalic
Copy link
Member

Instead of passing A as part of the process environment, we pass it via a file. Since A is usually the greatest contributor to the process environment, removing it from the process environment significantly helps running into MAX_ARG_STRLEN when spawning a new child process.

This means that A is now unexported in the ebuild. However, A is mostly used as part of the default_src_unpack function. And there A does not need to be exported.

Proof-of-concept for https://bugs.gentoo.org/721088

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

@mgorny
Copy link
Member

mgorny commented Jul 4, 2024

Why do you need to pass it via a file? It shouldn't be used by any external process, so it should be sufficient to leave it as a non-exported bash variable.

@Flowdalic
Copy link
Member Author

It shouldn't be used by any external process, so it should be sufficient to leave it as a non-exported bash variable.

I assumed that this is not possible. The A variable can't be part of the env dict that is passed to spawn(), as otherwise it would be part of the process environment and therefore subject to MAX_ARG_STRLEN.

I'd love to pass the variable directly, without a file. But I don't see how this is possible. What am I missing?

ebuildExtraSource = os.path.join(env["T"], "portage-ebuild-extra-source")
with open(ebuildExtraSource, mode="w") as f:
for name, value in orig_env.items():
if name != "A":
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 realize that this is not optimal, but that's a leftover from a previous implementation where everything was passed via file and not via env. This should be changed once this PR leaves the PoC stage.

@mgorny
Copy link
Member

mgorny commented Jul 4, 2024

Perhaps we could write it straight into environment that gets sourced by bash-side of Portage anyway.

@Flowdalic Flowdalic force-pushed the pass-env-via-file branch 2 times, most recently from d9ec4a2 to ac7f204 Compare July 4, 2024 16:50
@Flowdalic
Copy link
Member Author

Perhaps we could write it straight into environment that gets sourced by bash-side of Portage anyway.

I've looked into this and I believe the resulting code would be more convoluted that we we have right now.

bin/ebuild.sh Show resolved Hide resolved
Instead of passing A as part of the process environment, we pass it
via a file. Since A is usually the greatest contributor to the process
environment, removing it from the process environment significantly
helps running into MAX_ARG_STRLEN when spawning a new child process.

This means that A is now unexported in the ebuild. However, A is
mostly used as part of the default_src_unpack function. And there A
does not need to be exported.

Proof-of-concept for https://bugs.gentoo.org/721088

Bug: https://bugs.gentoo.org/721088
Signed-off-by: Florian Schmaus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants