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

Update default Python app to 3.10.11 #16

Closed
wants to merge 2 commits into from

Conversation

andreittr
Copy link
Contributor

@andreittr andreittr commented Aug 10, 2023

Update the prebuilt Python rootfs to 3.10.11, containing a minimal standard library without extensions.

In addition, update the README and Makefile.

Part of the larger Python 3.10 work:

Update the prebuilt Python rootfs to 3.10.11, containing a minimal
standard library without extensions.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@StefanJum
Copy link
Member

@andreittr can you please also update the Makefile to add all the library dependencies?

@andreittr
Copy link
Contributor Author

@andreittr can you please also update the Makefile to add all the library dependencies?

From what I can tell this app intends to build a minimal Python 3 without any extensions (the configs and Makefile reflect this), and for that reason the Makefile is perfectly adequate. The long list of libs mentioned in the PR is for when you want to build py3 with all possible extensions; they are not dependencies and python builds perfectly well without them.

@StefanJum
Copy link
Member

From what I can tell this app intends to build a minimal Python 3 without any extensions (the configs and Makefile reflect this).

Yes, that should be it, but I get some errors if I only leave the current libraries:

workdir/apps/python3/build/libpython3/origin/Python-3.10.11/Include/internal/pycore_bitutils.h:101: undefined reference to `__popcountdi2'

@andreittr
Copy link
Contributor Author

andreittr commented Aug 10, 2023

From what I can tell this app intends to build a minimal Python 3 without any extensions (the configs and Makefile reflect this).

Yes, that should be it, but I get some errors if I only leave the current libraries:

workdir/apps/python3/build/libpython3/origin/Python-3.10.11/Include/internal/pycore_bitutils.h:101: undefined reference to `__popcountdi2'

Hmm, __popcountdi2 seems to be defined by libcompiler-rt; what compiler & version are you using? (if it's common enough we might need to pull in libcompiler-rt just to be safe).

Edit: nvm, it seems to fail w/ gcc 13 (but not clang), so best to pull in compiler-rt (lib-python3 already implies it). Commit incoming.

@StefanJum
Copy link
Member

Hmm, __popcountdi2 seems to be defined by libcompiler-rt; what compiler & version are you using? (if it's common enough we might need to pull in libcompiler-rt just to be safe).

gcc version 12.3.0 (Ubuntu 12.3.0-1ubuntu1~23.04) yes, we should pull compiler-rt

This change brings the README, Makefile and defconfig files in line
with requirements for Python 3.10. Specifically:
- remove libuuid as a dependency; libuuid is entirely optional
- add compiler-rt as dependency, as it is required by some compilers

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@andreittr
Copy link
Contributor Author

@andreittr please update libuuid to compiler-rt in the other places in the README:

* https://github.com/unikraft/app-python3/blob/staging/README.md?plain=1#L98

* https://github.com/unikraft/app-python3/blob/staging/README.md?plain=1#L163

* https://github.com/unikraft/app-python3/blob/staging/README.md?plain=1#L186

Whoops, my bad, fixed now.

@andreittr andreittr changed the title Update rootfs to Python 3.10.11 Update default Python app to 3.10.11 Aug 11, 2023
@razvand razvand self-assigned this Aug 11, 2023
@razvand razvand added the enhancement New feature or request label Aug 11, 2023
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

All good, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

Copy link

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

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

Thanks, all good!

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Approved-by: Razvan Deaconescu razvand@unikraft.io

unikraft-bot pushed a commit that referenced this pull request Aug 11, 2023
This change brings the README, Makefile and defconfig files in line
with requirements for Python 3.10. Specifically:
- remove libuuid as a dependency; libuuid is entirely optional
- add compiler-rt as dependency, as it is required by some compilers

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Maria Sfiraiala <maria.sfiraiala@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #16
@andreittr andreittr deleted the ttr/up-3.10.11 branch August 11, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/merged enhancement New feature or request
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

None yet

5 participants