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 manylinux_2_24 images #822

Closed
wants to merge 107 commits into from
Closed

Add manylinux_2_24 images #822

wants to merge 107 commits into from

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Nov 8, 2020

This adds manylinux_2_24 images and rework the master branch to build manylinux2010, manylinux2014 and manylinux_2_24 from a single branch.

The build scripts were reworked for maintainers to be able to rebuild images taking advantage of docker cache.
This obviously has pros and cons compared to the previous one step build scripts. Here are my thoughts:
pros:

  • docker cache (i.e. faster build time for local builds)
  • one build script file per tool, I think this makes things clearer and allows to customize tool depending on the image being built without having a single script with many if
  • homogeneous tools available across images
  • image size is kept to a minimum (at the moment - and to my knowledge - yum or apt are making our life difficult when it comes to removing packages to restore a previous state, i.e. something like alpine's apk add --virtual build-deps .../apk del build-deps)

cons:

  • dependency versions are scattered across different files
  • docker handling of intermediate build stage images could be better

Please do provide feedback about rewriting as proposed. If you have other proposals on how you think it should be done, please do tell.

Some of the scripts might not be considered docker cache friendly. This is especially the case for install-entrypoint.sh, install-runtime-packages.sh and install-build-packages.sh. My motivation for doing things like this is that it helps getting consistency between images and those files are likely to change only when adding a new set of images which will not happen that often I guess. For install-build-packages.sh, it might also change when adding a new tool. This will have an impact on all images so should not be an issue.

This removes CPython 2.7 on manylinux2010 image.

README is not updated yet, waiting for feedback on other parts before doing this.

PS: This PR is also merging manylinux2014 in master not to lose history. This accounts for the high number of commits. I can open another PR targeting manylinux2014 branch first if this helps review.

mayeut and others added 30 commits November 10, 2019 23:15
latest autofconf was released in 2012, use the system provided one which is the latest on CentOS 7
* Update certifi from 2019.9.11 to 2019.11.28 (pypa#400)

* Update setuptools from 41.6.0 to 42.0.1 (pypa#398)

* Update setuptools from 42.0.1 to 42.0.2 (pypa#401)
- Update CPython 3.6 from 3.6.9 to 3.6.10
- Update CPython 3.7 from 3.7.5 to 3.7.6
- Update CPython 3.8 from 3.8.0 to 3.8.1
)

Bumps [setuptools](https://github.com/pypa/setuptools) from 46.0.0 to 46.1.1.
- [Release notes](https://github.com/pypa/setuptools/releases)
- [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst)
- [Commits](pypa/setuptools@v46.0.0...v46.1.1)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
)

Bumps [setuptools](https://github.com/pypa/setuptools) from 46.1.1 to 46.1.3.
- [Release notes](https://github.com/pypa/setuptools/releases)
- [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst)
- [Commits](pypa/setuptools@v46.1.1...v46.1.3)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
* Bump auditwheel from 3.2.0 to 3.3.0 in /docker/build_scripts

Bumps [auditwheel](https://github.com/pypa/auditwheel) from 3.2.0 to 3.3.0.
- [Release notes](https://github.com/pypa/auditwheel/releases)
- [Changelog](https://github.com/pypa/auditwheel/blob/master/CHANGELOG.md)
- [Commits](pypa/auditwheel@3.2.0...3.3.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>

* Bump wheel from 0.34.2 to 0.36.1 in /docker/build_scripts

Bumps [wheel](https://github.com/pypa/wheel) from 0.34.2 to 0.36.1.
- [Release notes](https://github.com/pypa/wheel/releases)
- [Changelog](https://github.com/pypa/wheel/blob/master/docs/news.rst)
- [Commits](pypa/wheel@0.34.2...0.36.1)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@mayeut mayeut marked this pull request as ready for review December 6, 2020 21:51
@mattip
Copy link
Contributor

mattip commented Dec 11, 2020

How would you suggest we proceed with getting this reviewed and merged? It is a massive PR. Is there a way to break it into pieces either as separate PRs or at least some kind of "guide for reviewers"?

@mattip
Copy link
Contributor

mattip commented Dec 11, 2020

Or maybe set up a synchronous call with some PyPA maintainers to walk them through it?

@mayeut mayeut mentioned this pull request Dec 11, 2020
@mayeut
Copy link
Member Author

mayeut commented Dec 11, 2020

@mattip,

As mentioned in "PS":

This PR is also merging manylinux2014 in master not to lose history. This accounts for the high number of commits. I can open another PR targeting manylinux2014 branch first if this helps review.

I created #873 to at least allow a review commit by commit. Nevertheless, commit 76dee71 will be a big one (breaks the "almost monolithic" build.sh/build_utils.sh in 1 script per tool meaning everything that was removed from those are to be found in the new files). I'm not sure how I can break this in a more "reviewable" way but I'll try to think of something over the week-end.

pinging @pypa/manylinux if they want to chime in on the way to proceed forward.

@mayeut
Copy link
Member Author

mayeut commented Dec 12, 2020

I will open one PR per tool migration for 76dee71 meaning I'll also do a few PR before that that build one on top of the others. So be it for the docker cache on my machine. I'll let travis-ci do the check and only build locally on errors if any in the meantime.

I can't get this PR back in draft, do not merge this.

@trishankatdatadog
Copy link
Member

trishankatdatadog commented Dec 12, 2020

I can't get this PR back in draft, do not merge this.

I can apparently mark it as draft. Would you like me to?

@mayeut
Copy link
Member Author

mayeut commented Dec 12, 2020

I propose the following strategy in order to get "this" merged:
Open PRs that will build one on top of the other requiring them to be reviewed sequentially which might take some time overall but will really ease the reviewing process with smaller changes. I won't open them all at once as they will depend on one another and reviews in a previous PR might lead to too much rework in the following PRs but I will open a couple at a time and rebase as PR gets merged in.
The first steps will be to merge manylinux2010 and manylinux2014 builds in the master branch. Once this is done, I can propose an extension for manylinux_2_24 images which will only have small changes compared to the new master.

I will create an issue to track the progress (but I'll keep this PR opened for now as a reference to where I'd like things to converge).

Edit: issue #877 created to discuss this further

@trishankatdatadog
Copy link
Member

You might find Projects helpful to track this work.

@mayeut mayeut marked this pull request as draft December 17, 2020 19:25
@h-vetinari
Copy link

h-vetinari commented Feb 10, 2021

Seems this needs a rebase after #993 and all the work from #877 (GH still shows the commits from the manylinux2014 branch).

@mayeut mayeut mentioned this pull request Feb 13, 2021
@mayeut
Copy link
Member Author

mayeut commented Feb 13, 2021

Superseded by #999

@mayeut mayeut closed this Feb 13, 2021
@mayeut mayeut deleted the pep600-master branch February 28, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.