Added support for armv7l #106

Lauszus · 2024-12-11T20:19:11Z

No description provided.

jsirois

This doesn't address ci or releases - do you plan on doing that?

science/providers/python_build_standalone.py

Lauszus · 2024-12-11T20:54:56Z

This doesn't address ci or releases - do you plan on doing that?

Yes. Will work on it now :)

jsirois · 2024-12-11T20:56:48Z

Thanks @Lauszus!

FYI: a-scie/ptex#255 & a-scie/jump#258

Not sure if you want to toss s390x into the mix or not. I'm happy to follow behind and add support to science and Pex if you want to stay focused on armv7l.

Lauszus · 2024-12-11T21:34:19Z

Thanks @Lauszus!

FYI: a-scie/ptex#255 & a-scie/jump#258

Not sure if you want to toss s390x into the mix or not. I'm happy to follow behind and add support to science and Pex if you want to stay focused on armv7l.

I think I'll just stick with armv7l for now. Can you allow the CI to run, so I can test my PR?

.github/workflows/ci.yml

jsirois

LGTM. You may have to dig in a cross container to figure out where the arm uname -m is coming from.

...aha. My bad reviewing your scie-jump change:
https://github.com/a-scie/jump/blob/60015e988144d8b4886ca17e5bb13c02c5fcd35a/jump/src/context.rs#L640-L650

So env::consts::ARCH is not only used for the scie-jump binary suffix, it's also used for the {scie.platform} and {scie.platform.arch} lift manifest placeholders: https://github.com/a-scie/jump/blob/main/docs/packaging.md#advanced-placeholders

So using a custom ARCH for armv7l fights a bit of an uphill battle. That should be enough info though to help you find all the spots (the one I pointed out above may be the only one, but I'm not positive (from my phone)).

Lauszus · 2024-12-12T09:14:05Z

LGTM. You may have to dig in a cross container to figure out where the arm uname -m is coming from.

...aha. My bad reviewing your scie-jump change: https://github.com/a-scie/jump/blob/60015e988144d8b4886ca17e5bb13c02c5fcd35a/jump/src/context.rs#L640-L650

So env::consts::ARCH is not only used for the scie-jump binary suffix, it's also used for the {scie.platform} and {scie.platform.arch} lift manifest placeholders: https://github.com/a-scie/jump/blob/main/docs/packaging.md#advanced-placeholders

So using a custom ARCH for armv7l fights a bit of an uphill battle. That should be enough info though to help you find all the spots (the one I pointed out above may be the only one, but I'm not positive (from my phone)).

Argh I should have grepped for ARCH. I'll fix it right away and send a PR.

Lauszus · 2024-12-12T09:28:41Z

LGTM. You may have to dig in a cross container to figure out where the arm uname -m is coming from.
...aha. My bad reviewing your scie-jump change: https://github.com/a-scie/jump/blob/60015e988144d8b4886ca17e5bb13c02c5fcd35a/jump/src/context.rs#L640-L650
So env::consts::ARCH is not only used for the scie-jump binary suffix, it's also used for the {scie.platform} and {scie.platform.arch} lift manifest placeholders: https://github.com/a-scie/jump/blob/main/docs/packaging.md#advanced-placeholders
So using a custom ARCH for armv7l fights a bit of an uphill battle. That should be enough info though to help you find all the spots (the one I pointed out above may be the only one, but I'm not positive (from my phone)).

Argh I should have grepped for ARCH. I'll fix it right away and send a PR.

Here it is: a-scie/jump#260

jsirois · 2024-12-12T10:43:44Z

Alrighty, thanks again @Lauszus. Released: https://github.com/a-scie/jump/releases/tag/v1.4.1

Lauszus · 2024-12-12T10:44:35Z

Alrighty, thanks again @Lauszus. Released: https://github.com/a-scie/jump/releases/tag/v1.4.1

Thanks. I've already updated my pex PR: pex-tool/pex#2620

Can you retrigger the CI for this PR?

jsirois · 2024-12-12T10:57:03Z

CHANGES.md

@@ -1,5 +1,9 @@
 # Release Notes

+## 0.9.0
+
+This release adds support for Linux ARM (armv7l and armv8l 32 bit mode).


Similar to scie jump, if you check all the RELEASE.md prep boxes, I can get a release out. You're missing a corresponding version bump for the package.

Super. I'll fix.

Aha, not so easy. There will need to be some test mods for armv7l. I'll check back in on your progress in the morning Nevada, US time.

I get the following warning:

nox > mypy --python-version 3.12 science tests test-support noxfile.py docs science/commands/complete.py:19: error: Incompatible types in assignment (expression has type "Process | None", variable has type "Process") [assignment] Found 1 error in 1 file (checked 45 source files) nox > Command mypy --python-version 3.12 science tests test-support noxfile.py docs failed with exit code 1 nox > Session check-/home/lauszus/Github/lift/.nox/PBS/20241206/3.12.8/install_only_stripped/python/bin/python3.12 failed. nox > Ran multiple sessions: nox > * fmt-/home/lauszus/Github/lift/.nox/PBS/20241206/3.12.8/install_only_stripped/python/bin/python3.12: success nox > * lint-/home/lauszus/Github/lift/.nox/PBS/20241206/3.12.8/install_only_stripped/python/bin/python3.12: success nox > * check-/home/lauszus/Github/lift/.nox/PBS/20241206/3.12.8/install_only_stripped/python/bin/python3.12: failed

But I ignored it, as I haven't touched that file or made any changes to the psutil dependency.

Aha, not so easy. There will need to be some test mods for armv7l. I'll check back in on your progress in the morning Nevada, US time.

Super. I'll try to fix the pytest tests. Seems like it is running it for armv7l even though it is not available.

Lauszus · 2024-12-12T14:10:16Z

tests/test_installer.py

@@ -30,6 +31,10 @@ def test_installer_help(installer: list):
        assert long_help in result.stdout, f"Expected '{long_help}' in tool output"


+@pytest.mark.skipif(
+    Platform.current() == Platform.Linux_armv7l,
+    reason="TODO: Remove once 0.9.0 is released",


I had to skip these two tests for now until 0.9.0 is released, as it tried to fetch the armv7l, but that is obviously not possible since I am adding it in this PR.

jsirois

Thanks for working through this one @Lauszus.

Lauszus · 2024-12-12T17:17:58Z

Thanks for working through this one @Lauszus.

You're welcome :)

jsirois · 2024-12-12T18:51:09Z

All right, the doc site was a little bumpy (an issue I caused with #105), but that's fixed up:

Lauszus mentioned this pull request Dec 11, 2024

Add support for armv7l. a-scie/ptex#253

Merged

Lauszus force-pushed the feature/armv7l branch from 50dccd3 to ccc5caf Compare December 11, 2024 20:23

jsirois reviewed Dec 11, 2024

View reviewed changes

science/providers/python_build_standalone.py Show resolved Hide resolved

Lauszus force-pushed the feature/armv7l branch from ccc5caf to 1793d3a Compare December 11, 2024 20:55

Lauszus marked this pull request as ready for review December 11, 2024 21:29

Lauszus marked this pull request as draft December 11, 2024 21:29

Lauszus mentioned this pull request Dec 11, 2024

Added support for armv7l pex-tool/pex#2620

Merged

Lauszus force-pushed the feature/armv7l branch 2 times, most recently from 349b216 to 2b68f13 Compare December 11, 2024 22:48

Lauszus commented Dec 11, 2024

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

Lauszus force-pushed the feature/armv7l branch 2 times, most recently from 109eafc to 9a019dc Compare December 11, 2024 23:15

Lauszus marked this pull request as ready for review December 11, 2024 23:30

Lauszus requested a review from jsirois December 11, 2024 23:30

jsirois reviewed Dec 12, 2024

View reviewed changes

Lauszus force-pushed the feature/armv7l branch 5 times, most recently from 52e4cf2 to f8aa7b1 Compare December 12, 2024 14:07

Lauszus commented Dec 12, 2024

View reviewed changes

Added support for armv7l

37a7ff7

Lauszus force-pushed the feature/armv7l branch from f8aa7b1 to 37a7ff7 Compare December 12, 2024 16:20

jsirois approved these changes Dec 12, 2024

View reviewed changes

jsirois merged commit b160893 into a-scie:main Dec 12, 2024
8 checks passed

Lauszus deleted the feature/armv7l branch December 12, 2024 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for armv7l #106

Added support for armv7l #106

Lauszus commented Dec 11, 2024

jsirois left a comment

Lauszus commented Dec 11, 2024

jsirois commented Dec 11, 2024

Lauszus commented Dec 11, 2024

jsirois left a comment •

edited

Loading

Lauszus commented Dec 12, 2024

Lauszus commented Dec 12, 2024

jsirois commented Dec 12, 2024

Lauszus commented Dec 12, 2024

jsirois Dec 12, 2024

Lauszus Dec 12, 2024

Lauszus Dec 12, 2024

jsirois Dec 12, 2024

Lauszus Dec 12, 2024

Lauszus Dec 12, 2024

Lauszus Dec 12, 2024

jsirois left a comment

Lauszus commented Dec 12, 2024

jsirois commented Dec 12, 2024 •

edited

Loading

Added support for armv7l #106

Added support for armv7l #106

Conversation

Lauszus commented Dec 11, 2024

jsirois left a comment

Choose a reason for hiding this comment

Lauszus commented Dec 11, 2024

jsirois commented Dec 11, 2024

Lauszus commented Dec 11, 2024

jsirois left a comment • edited Loading

Choose a reason for hiding this comment

Lauszus commented Dec 12, 2024

Lauszus commented Dec 12, 2024

jsirois commented Dec 12, 2024

Lauszus commented Dec 12, 2024

jsirois Dec 12, 2024

Choose a reason for hiding this comment

Lauszus Dec 12, 2024

Choose a reason for hiding this comment

Lauszus Dec 12, 2024

Choose a reason for hiding this comment

jsirois Dec 12, 2024

Choose a reason for hiding this comment

Lauszus Dec 12, 2024

Choose a reason for hiding this comment

Lauszus Dec 12, 2024

Choose a reason for hiding this comment

Lauszus Dec 12, 2024

Choose a reason for hiding this comment

jsirois left a comment

Choose a reason for hiding this comment

Lauszus commented Dec 12, 2024

jsirois commented Dec 12, 2024 • edited Loading

jsirois left a comment •

edited

Loading

jsirois commented Dec 12, 2024 •

edited

Loading