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

Add support to build AppImages on ARM #1564

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Add support to build AppImages on ARM #1564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b5e6a8
Add support to build AppImages on ARM
rmartin16 Dec 6, 2023
f50efcc
Revise manylinux arch warning and add removal note for manylinux imag…
rmartin16 Dec 8, 2023
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
1 change: 1 addition & 0 deletions changes/1564.feature.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
AppImages can now be built for the ARM architecture.
1 change: 1 addition & 0 deletions changes/1564.removal.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
New projects will now use ``manylinux_2_28`` instead of ``manylinux2014`` to create AppImages in Docker.
2 changes: 1 addition & 1 deletion docs/reference/platforms/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Platform support
+---------+-----------------+--------+-------+-----+--------+-------+-----+--------+-----+-------+
| iOS | |iOS|_ | |f| | |y| | | | | | | | |
+---------+-----------------+--------+-------+-----+--------+-------+-----+--------+-----+-------+
| Linux | |AppImage|_ | |v| | | | | | |v| | |v| | | |
| Linux | |AppImage|_ | |v| | |v| | | | | |v| | |v| | |v| | |v| |
+ +-----------------+--------+-------+-----+--------+-------+-----+--------+-----+-------+
| | |Flatpak|_ | | | | | | |v| | |f| | |v| | |v| |
+ +-----------------+--------+-------+-----+--------+-------+-----+--------+-----+-------+
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/platforms/linux/appimage.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ AppImage
+--------+-------+-----+--------+-------+-----+--------+-----+-------+
| x86‑64 | arm64 | x86 | x86‑64 | arm64 | x86 | x86‑64 | arm | arm64 |
+========+=======+=====+========+=======+=====+========+=====+=======+
| |v| | | | | | |v| | |v| | | |
| |v| | |v| | | | | |v| | |v| | |v| | |v| |
+--------+-------+-----+--------+-------+-----+--------+-----+-------+

.. admonition:: Best effort support
Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/bootstraps/pursuedpybear.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def pyproject_table_linux_system_arch(self):

def pyproject_table_linux_appimage(self):
return """
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"
Copy link
Member

Choose a reason for hiding this comment

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

We should flag this change with a removal release note (in addition to the feature), as it's a notable change to defaults.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was on the fence about whether this should be a removal note since it will only affect new projects. Although, if someone familiar with the old behavior created a wholly new project, they very well could be surprised then.


system_requires = [
# ?? FIXME
Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/bootstraps/pygame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def pyproject_table_linux_system_arch(self):

def pyproject_table_linux_appimage(self):
return """
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
]
Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/bootstraps/toga.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def pyproject_table_linux_system_arch(self):

def pyproject_table_linux_appimage(self):
return """
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
# Needed to compile pycairo wheel
Expand Down
26 changes: 21 additions & 5 deletions src/briefcase/integrations/linuxdeploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,32 @@ def file_path(self) -> Path:
"""The folder on the local filesystem that contains the file_name."""

@classmethod
def arch(cls, host_arch: str) -> str:
def arch(cls, tools: ToolCache) -> str:
"""The architecture defined (and supported) by linuxdeploy for AppImages."""
system_arch = tools.host_arch

# If Python is 32 bit, then use 32 bit linuxdeploy regardless of hardware.
# It is non-trivial to determine if Linux is 32 bit or 64 bit; so, this uses
# Python's bitness as a proxy for Linux's bitness. Furthermore, though, pip
# will install 32 bit packages if Python is 32 bit. So, using 32 bit
# linuxdeploy in this case ensures the entire resulting AppImage is consistent.
if tools.is_32bit_python:
system_arch = {
"aarch64": "armv8l",
"x86_64": "i686",
}.get(tools.host_arch, tools.host_arch)

try:
return {
"x86_64": "x86_64",
"i686": "i386",
}[host_arch]
"armv7l": "armhf",
"armv8l": "armhf",
"aarch64": "aarch64",
}[system_arch]
except KeyError as e:
raise UnsupportedHostError(
f"Linux AppImages cannot be built on {host_arch}."
f"Linux AppImages cannot be built on {tools.host_arch}."
) from e

def exists(self) -> bool:
Expand Down Expand Up @@ -222,7 +238,7 @@ class LinuxDeployQtPlugin(LinuxDeployPluginBase, ManagedTool):

@property
def file_name(self) -> str:
return f"linuxdeploy-plugin-qt-{self.arch(self.tools.host_arch)}.AppImage"
return f"linuxdeploy-plugin-qt-{self.arch(self.tools)}.AppImage"

@property
def download_url(self) -> str:
Expand Down Expand Up @@ -333,7 +349,7 @@ def file_path(self) -> Path:

@property
def file_name(self) -> str:
return f"linuxdeploy-{self.arch(self.tools.host_arch)}.AppImage"
return f"linuxdeploy-{self.arch(self.tools)}.AppImage"

@property
def download_url(self) -> str:
Expand Down
20 changes: 13 additions & 7 deletions src/briefcase/platforms/linux/appimage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def project_path(self, app):

def binary_name(self, app):
safe_name = app.formal_name.replace(" ", "_")
arch = LinuxDeploy.arch(self.tools.host_arch)
arch = LinuxDeploy.arch(self.tools)
return f"{safe_name}-{app.version}-{arch}.AppImage"

def binary_path(self, app):
Expand Down Expand Up @@ -175,13 +175,21 @@ class LinuxAppImageCreateCommand(
def output_format_template_context(self, app: AppConfig):
context = super().output_format_template_context(app)

# Add the manylinux tag to the template context.
try:
tag = getattr(app, "manylinux_image_tag", "latest")
manylinux_arch = {
"x86_64": "x86_64",
"i386": "i686",
}[LinuxDeploy.arch(self.tools.host_arch)]
"aarch64": "aarch64",
}[LinuxDeploy.arch(self.tools)]
except KeyError:
manylinux_arch = LinuxDeploy.arch(self.tools)
self.logger.warning(
f"There is no manylinux base image for {manylinux_arch}"
)
Copy link
Member

Choose a reason for hiding this comment

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

Why set the arch as "unknown" and printing a warning, so the next step fails (implicitly) on the Docker image lookup, rather than straight up failing on the arch lookup?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is a bit annoying....and I didn't want to introduce a bunch more changes to accommodate this better.

First, this code runs even if someone passes --no-docker. So, we can't error here if someone isn't even trying to use Docker. But....if this code doesn't run and they don't pass --no-docker next time, then the Dockerfile will definitely be hosed. (This may be best resolved by telling the user to run briefcase create linux appimage again since they want to use Docker now....but then we'd need to track they didn't use it the first time.)

Second, if someone isn't using the Briefcase template for AppImage, then the Dockerfile probably won't care about which specific manylinux image this logic determines anyway. So, I suppose we could assess the proper architecture for the name of the manylinux image after the code checks if app.manylinux exists.....but that just kinda moves the error condition around a bit and doesn't really change the underlying situation.

I don't think any of this is ideal....but I was mostly trying to avoid refactoring a bunch of this right now. Although, this really only presents because this is also introducing support for 32bit arm. If we only support aarch64, this complication (and others) goes away.

This is the balance I landed on given all this....open to other balancing points.

Copy link
Member

Choose a reason for hiding this comment

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

Ah - I hadn't considered the --no-docker case. That makes sense; and I think it's a reasonable compromise under the circumstances. My only thought is whether it might be better to use the actual architecture, rather than "unknown" (i.e., still raise the warning as you are now, but generate manylinux_2_28_armv7h as the manylinux base). That way, you'll still get an explicit pre-warning, but the Dockerfile will be in a state that won't look "wrong" - and when docker fails, the error will be of the form "manylinux_2_28_armv7h does not exist", rather than "manylinux_2_28_unknown does not exist". Both are equally true, but the former is a little more explicit about what doesn't exist... and it might exist at some point in the future.... (I doubt it, but it's at least possible)


# Add the manylinux tag to the template context.
try:
tag = getattr(app, "manylinux_image_tag", "latest")
context["manylinux_image"] = f"{app.manylinux}_{manylinux_arch}:{tag}"
if app.manylinux in {"manylinux1", "manylinux2010", "manylinux2014"}:
context["vendor_base"] = "centos"
Expand All @@ -190,9 +198,7 @@ def output_format_template_context(self, app: AppConfig):
elif app.manylinux.startswith("manylinux_2_"):
context["vendor_base"] = "almalinux"
else:
raise BriefcaseConfigError(
f"""Unknown manylinux tag {app.manylinux!r}"""
)
raise BriefcaseConfigError(f"Unknown manylinux tag {app.manylinux!r}")
except AttributeError:
pass

Expand Down
8 changes: 4 additions & 4 deletions tests/commands/new/test_build_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def main():
]
""",
pyproject_table_linux_appimage="""
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
# Needed to compile pycairo wheel
Expand Down Expand Up @@ -578,7 +578,7 @@ def main():
]
""",
pyproject_table_linux_appimage="""
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
# ?? FIXME
Expand Down Expand Up @@ -748,7 +748,7 @@ def main():
]
""",
pyproject_table_linux_appimage="""
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
]
Expand Down Expand Up @@ -1178,7 +1178,7 @@ def main():
]
""",
pyproject_table_linux_appimage="""
manylinux = "manylinux2014"
manylinux = "manylinux_2_28"

system_requires = [
# Needed to compile pycairo wheel
Expand Down
16 changes: 11 additions & 5 deletions tests/integrations/linuxdeploy/test_LinuxDeploy__properties.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,23 @@ def test_file_path(linuxdeploy, mock_tools):


@pytest.mark.parametrize(
"host_os, host_arch, linuxdeploy_arch",
"host_os, host_arch, is_32bit_python, linuxdeploy_arch",
[
("Linux", "x86_64", "x86_64"),
("Linux", "i686", "i386"),
("Darwin", "x86_64", "x86_64"),
("Linux", "x86_64", False, "x86_64"),
("Linux", "x86_64", True, "i386"),
("Linux", "i686", True, "i386"),
("Linux", "aarch64", True, "armhf"),
("Linux", "aarch64", False, "aarch64"),
("Linux", "armv7l", True, "armhf"),
("Linux", "armv8l", True, "armhf"),
("Darwin", "x86_64", False, "x86_64"),
],
)
def test_file_name(mock_tools, host_os, host_arch, linuxdeploy_arch):
def test_file_name(mock_tools, host_os, host_arch, is_32bit_python, linuxdeploy_arch):
"""Linuxdeploy filename is architecture dependent."""
mock_tools.host_os = host_os
mock_tools.host_arch = host_arch
mock_tools.is_32bit_python = is_32bit_python

linuxdeploy = LinuxDeploy(mock_tools)

Expand Down
26 changes: 26 additions & 0 deletions tests/platforms/linux/appimage/test_create.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,32 @@ def test_finalize_nodocker(create_command, first_app_config, capsys):
"use_non_root_user": True,
},
),
# Linux on aarch64 hardware
(
"manylinux_2_28",
None,
"Linux",
"aarch64",
False,
{
"manylinux_image": "manylinux_2_28_aarch64:latest",
"vendor_base": "almalinux",
"use_non_root_user": True,
},
),
# Linux on arm hardware
(
"manylinux_2_28",
None,
"Linux",
"armv7l",
False,
{
"manylinux_image": "manylinux_2_28_armhf:latest",
"vendor_base": "almalinux",
"use_non_root_user": True,
},
),
# macOS on x86_64
(
"manylinux2014",
Expand Down