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

fix: support legacy arm platforms #478

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions aws_lambda_builders/workflows/python_pip/packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ class DependencyBuilder(object):
"manylinux1_x86_64": "manylinux_2_5_x86_64",
"manylinux2010_x86_64": "manylinux_2_12_x86_64",
"manylinux2014_x86_64": "manylinux_2_17_x86_64",
"manylinux1_aarch64": "manylinux_2_5_aarch64",
"manylinux2010_aarch64": "manylinux_2_12_aarch64",
"manylinux2014_aarch64": "manylinux_2_17_aarch64",
Comment on lines +195 to +197
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did you get these mapping from?

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 was using this PR to test if my assumption is correct or not, and I marked it as draft, but seems I made it ready by mistake .. sorry for that .. I still need to work on it, it is not finalized

}

_COMPATIBLE_PACKAGE_ALLOWLIST = {"sqlalchemy"}
Expand Down Expand Up @@ -295,6 +298,7 @@ def _download_dependencies(self, directory, requirements_filename):
incompatible_wheels = set()
sdists = set()
for package in deps:
print(f">>>>> package.filename {package.filename}")
mndeveci marked this conversation as resolved.
Show resolved Hide resolved
if package.dist_type == "sdist":
sdists.add(package)
elif self._is_compatible_wheel_filename(package.filename):
Expand Down Expand Up @@ -429,6 +433,7 @@ def _is_compatible_platform_tag(self, expected_abi, platform):
# Verify the tag pattern
# Try to get the matching value for legacy values or keep the current
perennial_tag = self._MANYLINUX_LEGACY_MAP.get(platform, platform)
print(f">>>>>> {platform}, {perennial_tag}")

match = re.match("manylinux_([0-9]+)_([0-9]+)_" + arch, perennial_tag)
if match is None:
Expand Down
69 changes: 69 additions & 0 deletions tests/integration/workflows/python_pip/test_python_pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ def setUp(self):
self.dependencies_dir = tempfile.mkdtemp()

self.manifest_path_valid = os.path.join(self.TEST_DATA_FOLDER, "requirements-numpy.txt")
self.manifest_path_legacy = os.path.join(self.TEST_DATA_FOLDER, "requirements-legacy.txt")
self.manifest_path_invalid = os.path.join(self.TEST_DATA_FOLDER, "requirements-invalid.txt")
self.manifest_path_sdist = os.path.join(self.TEST_DATA_FOLDER, "requirements-wrapt.txt")

self.test_data_files = {
"__init__.py",
"main.py",
"requirements-invalid.txt",
"requirements-legacy.txt",
"requirements-numpy.txt",
"requirements-wrapt.txt",
"local-dependencies",
Expand Down Expand Up @@ -103,6 +105,42 @@ def test_must_build_python_project(self):
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

@skipIf(NOT_ARM, "Skip in windows tests")
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't run arm tests (not sure why), but how are we verifying this works now and will work in the future?

Also the reason states "windows". I assume that is a copy paste error.

def test_must_build_python_project_for_legacy_platforms(self):
self.builder.build(
self.source_dir,
self.artifacts_dir,
self.scratch_dir,
self.manifest_path_legacy,
runtime=self.runtime,
experimental_flags=self.experimental_flags,
)

if self.runtime == "python3.7":
expected_files = self.test_data_files.union(
{
"inflate64",
"inflate64.libs",
"inflate64-0.1.4.dist-info",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear to me how this works with the mappings you added. Was inflate64 the issue or was it some other package? Looking at the downloadable whls form this version, I don't see any of the aarch64 whls there.

"typing_extensions.py",
"importlib_metadata-6.1.0.dist-info",
"typing_extensions-4.5.0.dist-info",
"zipp-3.15.0.dist-info",
"importlib_metadata",
"zipp",
}
)
else:
expected_files = self.test_data_files.union(
{
"inflate64",
"inflate64.libs",
"inflate64-0.1.4.dist-info",
}
)
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

def test_must_build_python_project_python3_binary(self):
python_paths = which("python")
with tempfile.TemporaryDirectory() as executable_dir_str:
Expand Down Expand Up @@ -175,6 +213,37 @@ def test_must_build_python_project_with_arm_architecture(self):
else:
self.check_architecture_in("numpy-1.20.3.dist-info", ["manylinux2014_aarch64"])

@skipIf(NOT_ARM, "Skip in windows tests")
def test_must_build_python_project_with_arm_architecture_for_legacy_platforms(self):
if self.runtime not in ARM_RUNTIMES:
self.skipTest("{} is not supported on ARM architecture".format(self.runtime))
### Check the wheels
self.builder.build(
self.source_dir,
self.artifacts_dir,
self.scratch_dir,
self.manifest_path_legacy,
runtime=self.runtime,
architecture="arm64",
experimental_flags=self.experimental_flags,
)
expected_files = self.test_data_files.union(
{
"inflate64",
"inflate64-0.1.4.dist-info",
"typing_extensions.py",
"importlib_metadata-6.1.0.dist-info",
"typing_extensions-4.5.0.dist-info",
"zipp-3.15.0.dist-info",
"importlib_metadata",
"zipp",
}
)
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

self.check_architecture_in("inflate64-0.1.4.dist-info", ["manylinux2014_aarch64"])

def test_mismatch_runtime_python_project(self):
# NOTE : Build still works if other versions of python are accessible on the path. eg: /usr/bin/python3.7
# is still accessible within a python 3.8 virtualenv.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
inflate64==0.1.4