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

Fix Release Pipeline for distutils #598

Merged
merged 15 commits into from
Oct 1, 2024
Merged

Fix Release Pipeline for distutils #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4ea58b8
fix ccompiler import
waahm7 Sep 26, 2024
e29951c
try importing twice
waahm7 Sep 26, 2024
2fcd519
try using it directly instead of import
waahm7 Sep 26, 2024
c2cbc42
Revert "try using it directly instead of import"
waahm7 Sep 26, 2024
b81bd19
try improting once
waahm7 Sep 26, 2024
800b2b3
nope
waahm7 Sep 26, 2024
ee7cb4c
Just keep using distutils.ccompiler
waahm7 Sep 26, 2024
8f31f34
drop 3.7
waahm7 Sep 26, 2024
20f816f
use proper name
waahm7 Sep 26, 2024
ec21fc3
lint
waahm7 Sep 26, 2024
bacfe15
more python 37 removal
waahm7 Sep 26, 2024
7d7ee40
fix issue link
waahm7 Sep 26, 2024
db16297
More fixes
waahm7 Sep 27, 2024
ecba0e2
fix comment
waahm7 Sep 27, 2024
54d4b9d
add comments in this function as well
waahm7 Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ jobs:
- x64
- x86
python:
- cp37-cp37m
- cp38-cp38
- cp39-cp39
permissions:
Expand All @@ -52,7 +51,6 @@ jobs:
- x86
- aarch64
python:
- cp37-cp37m
- cp38-cp38
- cp39-cp39
- cp310-cp310
Expand Down Expand Up @@ -85,7 +83,6 @@ jobs:
- x64
- aarch64
python:
- cp37-cp37m
- cp38-cp38
- cp39-cp39
- cp310-cp310
Expand Down
2 changes: 0 additions & 2 deletions codebuild/cd/manylinux-x64-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ phases:
build:
commands:
- echo Build started on `date`
- /opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
- auditwheel repair --plat manylinux1_x86_64 dist/awscrt-*cp37-cp37m-linux_x86_64.whl
- /opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
- auditwheel repair --plat manylinux1_x86_64 dist/awscrt-*cp38-cp38-linux_x86_64.whl
- /opt/python/cp39-cp39/bin/python setup.py sdist bdist_wheel
Expand Down
2 changes: 0 additions & 2 deletions codebuild/cd/manylinux-x86-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ phases:
build:
commands:
- echo Build started on `date`
- /opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
- auditwheel repair --plat manylinux1_i686 dist/awscrt-*cp37-cp37m-linux_i686.whl
- /opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
- auditwheel repair --plat manylinux1_i686 dist/awscrt-*cp38-cp38-linux_i686.whl
- /opt/python/cp39-cp39/bin/python setup.py sdist bdist_wheel
Expand Down
7 changes: 4 additions & 3 deletions continuous-delivery/build-wheels-manylinux2014-aarch64.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ set -ex

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

/opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_aarch64 dist/awscrt-*cp37*.whl

/opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_aarch64 dist/awscrt-*cp38*.whl

Expand All @@ -23,6 +20,10 @@ auditwheel repair --plat manylinux2014_aarch64 dist/awscrt-*cp311*.whl
# The 3.11 wheel uses the stable ABI, so it works with newer versions too.

# We are using the Python 3.13 stable ABI from Python 3.13 onwards because of deprecated functions.
# Manylinux images don't contain setuptools from Python 3.13, so we need to install it.
# Install in a custom location due to access issues.
/opt/python/cp313-cp313/bin/python -m pip install --target ./local -r requirements-dev.txt
export PYTHONPATH=./local:$PYTHONPATH
/opt/python/cp313-cp313/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_aarch64 dist/awscrt-*cp313*.whl

Expand Down
7 changes: 4 additions & 3 deletions continuous-delivery/build-wheels-manylinux2014-x86_64.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ set -ex

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

/opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_x86_64 dist/awscrt-*cp37*.whl

/opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_x86_64 dist/awscrt-*cp38*.whl

Expand All @@ -23,6 +20,10 @@ auditwheel repair --plat manylinux2014_x86_64 dist/awscrt-*cp311*.whl
# The 3.11 wheel uses the stable ABI, so it works with newer versions too.

# We are using the Python 3.13 stable ABI from Python 3.13 onwards because of deprecated functions.
# Manylinux images don't contain setuptools from Python 3.13, so we need to install it.
# Install in a custom location due to access issues.
/opt/python/cp313-cp313/bin/python -m pip install --target ./local -r requirements-dev.txt
export PYTHONPATH=./local:$PYTHONPATH
Comment on lines +23 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

(debatable) Move those to a prepare-manylinux.sh.
So that have one source instead of copy/paste to 4 places?
And we will remove manylinux1 sometime soon, so it's fine that not invoking it from manylinux1?
More future prove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any prepare-manylinux.sh script. Do you mean creating a new one? I think it's fine for now, since it's just two lines of code, and the rest of the code is different per platform. This is also ABI compatible wheel so we don't expect to do this often.

manylinux1 doesn't support Python 3.13, I believe, so we don't need to add it there.

/opt/python/cp313-cp313/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat manylinux2014_x86_64 dist/awscrt-*cp313*.whl

Expand Down
7 changes: 4 additions & 3 deletions continuous-delivery/build-wheels-musllinux-1-1-aarch64.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ set -ex

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

/opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_aarch64 dist/awscrt-*cp37*.whl

/opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_aarch64 dist/awscrt-*cp38*.whl

Expand All @@ -23,6 +20,10 @@ auditwheel repair --plat musllinux_1_1_aarch64 dist/awscrt-*cp311*.whl
# The 3.11 wheel uses the stable ABI, so it works with newer versions too.

# We are using the Python 3.13 stable ABI from Python 3.13 onwards because of deprecated functions.
# Manylinux images don't contain setuptools from Python 3.13, so we need to install it.
# Install in a custom location due to access issues.
/opt/python/cp313-cp313/bin/python -m pip install --target ./local -r requirements-dev.txt
export PYTHONPATH=./local:$PYTHONPATH
/opt/python/cp313-cp313/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_aarch64 dist/awscrt-*cp313*.whl

Expand Down
7 changes: 4 additions & 3 deletions continuous-delivery/build-wheels-musllinux-1-1-x86_64.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ set -ex

/opt/python/cp39-cp39/bin/python ./continuous-delivery/update-version.py

/opt/python/cp37-cp37m/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_x86_64 dist/awscrt-*cp37*.whl

/opt/python/cp38-cp38/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_x86_64 dist/awscrt-*cp38*.whl

Expand All @@ -23,6 +20,10 @@ auditwheel repair --plat musllinux_1_1_x86_64 dist/awscrt-*cp311*.whl
# The 3.11 wheel uses the stable ABI, so it works with newer versions too.

# We are using the Python 3.13 stable ABI from Python 3.13 onwards because of deprecated functions.
# Manylinux images don't contain setuptools from Python 3.13, so we need to install it.
# Install in a custom location due to access issues.
/opt/python/cp313-cp313/bin/python -m pip install --target ./local -r requirements-dev.txt
export PYTHONPATH=./local:$PYTHONPATH
/opt/python/cp313-cp313/bin/python setup.py sdist bdist_wheel
auditwheel repair --plat musllinux_1_1_x86_64 dist/awscrt-*cp313*.whl

Expand Down
1 change: 0 additions & 1 deletion continuous-delivery/build-wheels-osx.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ set -ex

/Library/Frameworks/Python.framework/Versions/3.9/bin/python3 ./continuous-delivery/update-version.py

/Library/Frameworks/Python.framework/Versions/3.7/bin/python3 setup.py sdist bdist_wheel
/Library/Frameworks/Python.framework/Versions/3.8/bin/python3 setup.py sdist bdist_wheel
/Library/Frameworks/Python.framework/Versions/3.9/bin/python3 setup.py sdist bdist_wheel
/Library/Frameworks/Python.framework/Versions/3.10/bin/python3 setup.py sdist bdist_wheel
Expand Down
1 change: 0 additions & 1 deletion continuous-delivery/build-wheels-win32.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

"C:\Program Files (x86)\Python39-32\python.exe" .\continuous-delivery\update-version.py || goto error

"C:\Program Files (x86)\Python37-32\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files (x86)\Python38-32\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files (x86)\Python39-32\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files (x86)\Python310-32\python.exe" setup.py sdist bdist_wheel || goto error
Expand Down
1 change: 0 additions & 1 deletion continuous-delivery/build-wheels-win64.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"C:\Program Files\Python39\python.exe" continuous-delivery\update-version.py || goto error

"C:\Program Files\Python37\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files\Python38\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files\Python39\python.exe" setup.py sdist bdist_wheel || goto error
"C:\Program Files\Python310\python.exe" setup.py sdist bdist_wheel || goto error
Expand Down
18 changes: 13 additions & 5 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0.

import codecs
import distutils.ccompiler
import glob
import os
import os.path
Expand Down Expand Up @@ -78,6 +76,7 @@ def determine_generator_args():
if sys.platform == 'win32':
try:
# See which compiler python picks
import distutils.ccompiler
compiler = distutils.ccompiler.new_compiler()
compiler.initialize()

Expand Down Expand Up @@ -305,8 +304,12 @@ def run(self):
class bdist_wheel_abi3(bdist_wheel):
def get_tag(self):
python, abi, plat = super().get_tag()
if python.startswith("cp") and sys.version_info >= (3, 11):
# on CPython, our wheels are abi3 and compatible back to 3.11
# on CPython, our wheels are abi3 and compatible back to 3.11
if python.startswith("cp") and sys.version_info >= (3, 13):
# 3.13 deprecates PyWeakref_GetObject(), adds alternative
return "cp313", "abi3", plat
Copy link
Contributor

Choose a reason for hiding this comment

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

worth to mention that cp313 has a different abi3 (Not sure if it's right wording.)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, we already mention it at https://github.com/awslabs/aws-crt-python/blob/main/setup.py#L408. I will add a comment here as well.

elif python.startswith("cp") and sys.version_info >= (3, 11):
# 3.11 is the first stable ABI that has everything we need
return "cp311", "abi3", plat

return python, abi, plat
Expand All @@ -326,6 +329,11 @@ def awscrt_ext():
libraries.reverse()

if sys.platform == 'win32':
# distutils is deprecated in Python 3.10 and removed in 3.12. However, it still works because Python defines a compatibility interface as long as setuptools is installed.
# We don't have an official alternative for distutils.ccompiler as of September 2024. See: https://github.com/pypa/setuptools/issues/2806
# Once that issue is resolved, we can migrate to the official solution.
# For now, restrict distutils to Windows only, where it's needed.
import distutils.ccompiler
Comment on lines 331 to +336
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the import to the beginning of the file instead? So that we don't need to have two place to import the package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we move the import to the beginning of the file, it will be imported everywhere. Because this is only required on Windows and is deprecated, I have restricted it to Windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean move the

if sys.platform == 'win32':
   import distutils.ccompiler

to the top of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, missed it in this PR. I have created #599 to fix this.

# the windows apis being used under the hood. Since we're static linking we have to follow the entire chain down
libraries += ['Secur32', 'Crypt32', 'Advapi32', 'NCrypt', 'BCrypt', 'Kernel32', 'Ws2_32', 'Shlwapi']
# Ensure that debug info is in the obj files, and that it is linked into the .pyd so that
Expand Down Expand Up @@ -374,7 +382,7 @@ def awscrt_ext():
# rare cases where that didn't happen, so let's be explicit.
extra_link_args += ['-pthread']

if distutils.ccompiler.get_default_compiler() != 'msvc':
if sys.platform != 'win32' or distutils.ccompiler.get_default_compiler() != 'msvc':
extra_compile_args += ['-Wno-strict-aliasing', '-std=gnu99']

# treat warnings as errors in development mode
Expand Down
Loading