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

Add python-as-wrapper package and use it in asciidoc build. #36891

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

Conversation

smoser
Copy link
Member

@smoser smoser commented Dec 13, 2024

Hopefully the use here in asciidoc shows the usefulness of this wrapper. Many programs or build systems really want to use 'python3', but we really want them to use 'python3.X'.

asciidoc's build system is hard coded to call 'python3'. I had submitted a patch upstream to allow us to use 'python3.13' here. But doing that is costly in time. Not doing it means the programs it installs in /usr/bin have shbang of 'python3', which we do not want. The python3-as-3.13 package makes that "just work" without modification.

This wrapper package provides a compromise that gets rid of the "sticky" results of calling 'python3'. The big change is that now a program that uses 'python3' to write its scripts will write a shbang as '#!/usr/bin/python3.XX'

@smoser smoser requested a review from murraybd December 13, 2024 14:11
@smoser
Copy link
Member Author

smoser commented Dec 13, 2024

I feel like this can actually replace a lot of the shell that I've written in pipelines/py/, but even without that, it allows more build systems to "just work" by providing a python3 or python that works we want it to.

@smoser smoser force-pushed the feat/python-as-wrapper branch from 70ac7d0 to 471cf64 Compare December 13, 2024 14:14
@octo-sts octo-sts bot added the bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. label Dec 13, 2024

This comment was marked as off-topic.

This comment was marked as resolved.

@smoser smoser added the ai/skip-comment Stop AI from commenting on PR label Dec 13, 2024
@smoser smoser force-pushed the feat/python-as-wrapper branch from 471cf64 to 26d4d62 Compare December 13, 2024 14:23
python-as-wrapper.yaml Outdated Show resolved Hide resolved
@xnox
Copy link
Member

xnox commented Dec 13, 2024

silly question, why shell and not python https://docs.python.org/3/library/os.html#os.execv ?

dannf
dannf previously approved these changes Dec 13, 2024
Copy link
Contributor

@dannf dannf left a comment

Choose a reason for hiding this comment

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

The concept makes sense to me, nice! Should we add some bumpers to try and avoid this getting shipped in packages/images? Maybe emit a warning when used?

python-as-wrapper.yaml Show resolved Hide resolved
python-as-wrapper.yaml Outdated Show resolved Hide resolved
python-as-wrapper.yaml Outdated Show resolved Hide resolved
@smoser
Copy link
Member Author

smoser commented Dec 13, 2024

It is probably difficult to tell exactly what python-as-wrapper does by reading python-as-wrapper.yaml. I encourange you to pull this branch, make package/python-as-wrapper and then make local-wolfi and apk add python-as-3.13 or python3-as-3.12.

What you'll see is:

  • python3-as-python3.13 package
    • depends on python-3.13-base (for /usr/bin/python3.13)
    • installs /usr/bin/python3 with:
      #!/bin/sh
      exec "python3.13" "$@"
  • python-as-python3.13 package
    • depends on python-3.13-base
    • installs /usr/bin/python with the same 2 line shell script.

The python3-as-env and python-as-env packages are a bit more complicated.
They install /usr/bin/python3 and /usr/bin/python with the content below.

  • python-as-env uses the variable PYTHON_AS
  • python3-as-env uses the variable PYTHON3_AS.
  • this is fully posix shell.
  • all of the code is really just to give good error messages so people will be
    able to diagnose what went wrong.
#!/bin/sh
fail() { echo "FATAL:" "$0" "$@"; exit 9; }
[ -n "${PYTHON3_AS}" ] || fail "invoked without PYTHON3_AS set"
[ "$PYTHON3_AS" -ef "$0" ] && fail "invoked with PYTHON3_AS = $0"
[ "${PYTHON3_AS}" != "${0##*/}" ] || fail "invoked with PYTHON3_AS = basename($0)"
command -v "$PYTHON3_AS" >/dev/null ||
  fail "invoked with PYTHON3_AS=$PYTHON3_AS, but $PYTHON3_AS not found in PATH"
exec "${PYTHON3_AS}" "$@"

The overhead of both the -env and -3.XX versions is minimal.
The following is collected on a workstation by running time like below
with PPATH set to the wrapper /usr/bin/python3 and /usr/bin/python3.13 directly.

nums=$(seq 1 1000);
PPATH=/usr/bin/python3
time sh -c 'p=$1; shift; for n in "$@"; do $p -c pass; done' PPATH $nums
invocation PYTHON3_AS real user sys
python3-as-env /usr/bin/python3.13 14.55s 10.14s 3.53s
python3-as-env python3.13 14.56s 10.13s 3.55s
python3-as-3.13 n/a 15.16s 10.16s 3.86s
/usr/bin/python3.13 n/a 12.92s 9.69s 2.79s

The numbers aren't scientific. Prices and participation my vary, but they do
show that the overhead is small. Maybe < 3 seconds on 1000 invocations.
I don't know why, but in the two times I ran the python3-as-3.13 was slower
than python3-as-env. That probably indicates that there is more noise in the
tests on a virtual machine in GCE.

@smoser
Copy link
Member Author

smoser commented Dec 13, 2024

silly question, why shell and not python https://docs.python.org/3/library/os.html#os.execv ?

It would be longer text-wise and slower. It would drop a shell depends, but ... we're not really going to get rid of /bin/sh in our build environments.

Here is a 'time' run of 1000 invocations of the noops `python3.13 -c pass' and 'sh -c :'. Basically, it shows the overhead of bringup of those interpreters.

$ nums=$(seq 1 1000); time sh -c 'for n in "$@"; do python3.13 -c pass ; don
e' -- $nums
real    0m 13.26s
user    0m 9.80s
sys     0m 2.91s

$ nums=$(seq 1 1000); time sh -c 'for n in "$@"; do sh -c :; done' -- $nums
real    0m 1.84s
user    0m 0.45s
sys     0m 0.90s

@xnox
Copy link
Member

xnox commented Dec 13, 2024

silly question, why shell and not python https://docs.python.org/3/library/os.html#os.execv ?

It would be longer text-wise and slower. It would drop a shell depends, but ... we're not really going to get rid of /bin/sh in our build environments.

Here is a 'time' run of 1000 invocations of the noops `python3.13 -c pass' and 'sh -c :'. Basically, it shows the overhead of bringup of those interpreters.

$ nums=$(seq 1 1000); time sh -c 'for n in "$@"; do python3.13 -c pass ; don
e' -- $nums
real    0m 13.26s
user    0m 9.80s
sys     0m 2.91s

$ nums=$(seq 1 1000); time sh -c 'for n in "$@"; do sh -c :; done' -- $nums
real    0m 1.84s
user    0m 0.45s
sys     0m 0.90s

ok, rust binary? c binary? go binary? i do understand those are harder to write over shell script.

i guess future improvements. Really need to write shell -> rust transpiler.

@smoser
Copy link
Member Author

smoser commented Dec 13, 2024

@dannf

Should we add some bumpers to try and avoid this getting shipped in packages/images? Maybe emit a warning when used?

ideas on how?

Also, thanks for the push to split out the HERE docs, this is more readable now.

@smoser smoser marked this pull request as ready for review December 13, 2024 17:17
@xnox xnox added the approved-to-run A repo member has approved this external contribution label Dec 13, 2024
Hopefully the use here in asciidoc shows the usefulness of this
wrapper.  Many programs or build systems really want to use
'python3', but we really want them to use 'python3.X'.

This wrapper package provides a compromise that gets rid of the
"sticky" results of calling 'python3'.  The big change is that
now a program that uses 'python3' to write its scripts will
write a shbang as '#!/usr/bin/python3.XX'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai/skip-comment Stop AI from commenting on PR approved-to-run A repo member has approved this external contribution bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants