Improve and update the packaging system #17488
Conversation
The desktop packaging structure has been improved. All files that are only related to packaging the desktop application (e.g. metadata files) have been moved from the assets and packages folders into a new packaging folder. The folder contains a subfolder for each operating system. All links to any of the moved packaging files in the GitHub workflow, build.rs and other packaging files have been updated accordingly.
The metadata packaged in the Windows executable has been improved, updated and corrected. Previously, a static RC file had been used to provide Windows metadata. However, it had been outdated, wrong (the InternalName value was not in line with the Microsoft documentation) and lacked information (e.g. the debug flag had never been set). Additionally, a static RC file is not a good way to package Windows metadata as it's hard to dynamically update. Therefore, it has been replaced by the winresource crate. The winresource crate is now used by the build script to dynamically generate an up-to-date RC file and embed it in the Windows executable.
The metadata packaged in the macOS .app has been improved and corrected. The Info.plist file has been restructured to improve readability and group similar entries. Additional information has also been added to it, namely: The list of supported languages, the current Ruffle version, the minimum required system version and a readable copyright. An outdated entry has been removed. The version information, minimum required system version and current year (for the copyright) is now dynamically entered while running the GitHub workflow to make sure it always stays up-to-date. Additionally, the minimum required macOS version has been updated (as Rust's minimum supported version has changed) and is now also used as --minimum-deployment-target parameter (previously, 10.0 had wrongly been used).
The creation of a DMG file has been added to the macOS build workflow. It replaces the tar.gz file for macOS and is created and uploaded for each release. dmgbuild is used to create it. A background image for the DMG file has been added as well. Additionally, some wording has been improved.
The creation of an AppImage has been added to the Linux build workflow. It is now bundled inside the Linux tar.gz instead of the raw binary. This leads to a higher distribution compatibility and a better desktop integration. Linuxdeploy is used to create it. The Linux build workflow is now run inside a docker container with the oldest still-supported LTS version. This should be done when building AppImages to be compatible with all still-supported Linux distribution versions. In order to use the container, a new step has been added which installs the required software the default GitHub runner already had preinstalled. Additionally, an SVG Ruffle icon has been added, as well as a 512x512px raster version. They have been created out of the existing macOS SVG icon (which has been renamed). They are used for the new AppImage. The desktop entry file has also been updated to use the correct icon and executable name.
The creation of single architecture packages has been added to the macOS build workflow. It now produces one x86-64, one arm64 and one universal package. As the process of packaging the executable into an app bundle inside a DMG image is the same for all packages, the common logic has been extracted and moved into a new shell script. This shell script is called in both jobs with the necessary parameters and environment variables. This also allows for the arm64 image to be packaged using a newer compression format and filesystem, as older systems don't have to be supported. In order for the single architecture packages to be built, the Safari build artifact needs to be downloaded. Therefore, the build job now depends on the build-web job, and the necessary artifacts are downloaded in it.
The readability of the GitHub Actions workflow "Release nightly" has been improved. Most noticably, the package names have been improved: Previously, the name parts had been separated with dashes, and underscores have been used to separate elements inside some parts (e.g. time or architecture). This had lead to a bad readability as underscores visually separate the text more than dashes. Therefore, the underscores and dashes have been switched. Additionally, the text is now capitalized, which also improves the readability. Other files that depend on the packages and use the package names have been adapted accordingly to use the new package names. Other readability improvements have been added as well, namely: - Line breaks have been added to long lines. - Unzip commands have been improved to use the -d argument. - Unnecessary and duplicated steps and paths have been removed. - Step names have been improved. Additionally, quotation marks have been added to GitHub Actions variables so that the workflow will still work if the variables contain spaces.
Am I understanding right, that it'll now be impossible to just download the raw binary from releases? |
|
Also, I get the feeling that this could/should be split up to almost a PR per commit? 😶 That way each part can be bikeshedded properly. 😅 Taking into consideration the dependencies among them for the splitting of commits and sequencing of PRs of course... |
|
@adrian17 Yeah, the AppImage is now uploaded instead of the raw binary. But that's an improvement: The AppImage just contains the binary together with metadata, such as icons, that help with desktop integration and dependencies that aren't installed on every distribution. There's no reason to ship raw binaries instead (that's usually seen as bad Linux support / unprofessional; I don't know any other modern software that does this). |
|
@torokati44 I get the thought; it would have been a different, but also valid approach. Personally, I prefer to bundle all changes that affect a specific part or system. This way I can make sure that it's clear what my desired outcome is and it groups together related changes (which also makes it easier to keep things consistent). And it allows me to better separate the different commits (e.g. if I had split it up, I would have done some of the stuff I did in the "Improve readability" commit in each PR as much of it results from other commits, which would make it less easy to see the real distinct changes). |
|
(Btw: Initially, this PR also contained a new Windows installer I created, but @Dinnerbone beat me to it in #17335) |
Lots of server-side software are distributed as raw binary. I don't see any problem with doing so. The advantage of distributed as raw binary is that it's easier to be extracted from the archive and it doesn't have to be mounted to run it(aka doesn't require FUSE to run it). |
Huh, interesting. I didn't know that, but I meant consumer software (which is usually packaged as AppImage / .deb / etc.). There is a new static runtime that doesn't require FUSE to be installed anymore, but Linuxdeploy hasn't been updated yet. It can be run without it though by using the parameter |
I think Blender is also distributed like this. It's in an archive only because it consists of multiple files. |
There was a problem hiding this comment.
I agree with @torokati44, we should split this PR into multiple ones, as IMO too much's going on in here 😅
| package_prefix: ruffle-nightly-${{ steps.current_time_underscores.outputs.formattedTime }} | ||
| package_prefix: Ruffle_Nightly_${{ steps.current_time_dashes.outputs.formattedTime }} |
There was a problem hiding this comment.
Currently, the name parts are separated with dashes, and underscores are used to separate elements inside some parts (e.g. time or architecture). This leads to a bad readability as underscores visually separate the text more than dashes (see ruffle-nightly-2024_08_12-linux-x86_64.tar.gz). Therefore, the underscores and dashes have been switched. Additionally, the text is now capitalized, which also improves the readability (see Ruffle_Nightly_2024-08-12_Linux_x86-64.tar.gz; this also uses the canonical spelling of x86-64 and ISO 8601).
Originally I did not want to change package names as they might be used downstream. But if we decide to rename the packages, I think we should really stick to ruffle-<version>-<platform>-<arch> or ruffle-<version>-<triple> (but that's perhaps too technical?).
- I do not agree that underscores separate text more than dashes. Usually underscores are part of identifiers whereas dashes separate identifiers. Consider e.g. Debian tuples or Cargo targets, not mentioning use in programming languages.
- I do not agree that capitalized text improves readability, it is rather uncommon to see technical filenames with capitalized text.
- I do not see a reason why we should care about ISO 8601, we should care more about proper versioning instead of basing filenames on dates.
For stable releases we will use versions like 1.2.3, whereas for nightly we can use 1.2.3-nightly.2024.5.6. That way filenames will have the same format for both.
There was a problem hiding this comment.
The problem is: These are not technical filenames. These are the filenames every user will see after downloading the package, and some will even decide the version to download depending on it (when using GitHub releases).
Therefore, they have to be as easily readable and understandable as possible. E.g. when downloading Firefox, the name is Firefox Installer.exe or Firefox 129.0.1.dmg - easy to read and understand for every user.
Dashes are in the same height as letters and blend under while underscores create a visible gap (and are longer). Re-ad_Th-is_Sent-ence_He-re is much easier to read than re_ad-th_is-sen_tence-he_re - the gaps are in the wrong place.
There was a problem hiding this comment.
I would argue they should be technical filenames. GitHub is just not friendly for non-technical users, it was never its goal. That's why the most basic (and some advanced) downloads are linked from the Ruffle website. Users are redirected to GitHub not because GH's user-friendly, but because it's easy to do so. GitHub does not provide any way of differentiating assets based on OS, platform, or architecture. It does not have any way of detecting which OS the user has in order to show what to download. The releases page is just not meant for that, and I don't believe we should force it to be a bit user-friendly by playing with asset names. The user which lands on releases already is required to have technical knowledge about their system and in general.
When it comes to filenames after downloading the package (from the website), I believe it's a wrong place to change that. If we decide that in fact these filenames are non-user-friendly and we want to change them, we should change them on the website, not here (and I suspect it would currently be a bit difficult as we're redirecting to GH).
There was a problem hiding this comment.
I don't really see any reason for keeping worse filenames. Maybe you're right that it doesn't matter than much to some people as I think it does, but it's still just better than doing nothing, even if it's not important.
The filenames should be consistent across the board. You're right that if the website has different filenames, we should change them there as well.
But also, I would argue it doesn't matter why users are redirected to Github. As long as they are, the filenames on GitHub matter. And it doesn't hurt to at least try and make it easier for less tech-savvy users to still download Ruffle when they are redirected.
There was a problem hiding this comment.
These filenames are used by the website to figure out which file is which, and present the relevant stuff to users. If we want friendly names it should be done via the website, github names are indeed technical and these should be considered closer to release artifacts instead of user deliverables.
| Icon=icon | ||
| Exec=ruffle_desktop |
There was a problem hiding this comment.
- Is the directory name change necessary? All directories in Ruffle are lowercase and in general non-user-related directories rarely have uppercase names, I really do not see a reason to change that.
- Why are we changing these values specifically? The executable name
ruffleis widely used, the icon has a unique name. Changing these will also break Flatpak builds.
There was a problem hiding this comment.
Changing these will also break Flatpak builds.
Yep, this will definitely be reverted when I rebase this PR. I was changing the values just to match the current ones: The executable cargo builds is named ruffle_desktop and the icons are named icon. Therefore I adapted the file to get working AppImages. But yeah, I'll change that when rebasing.
About the other thing: It's a new directory structure, collecting and containing all files that are (only) used for packaging. Using the canonical OS names is just nicer and better to read, I don't see why we should unnecessarily restrict ourselves.
There was a problem hiding this comment.
About the other thing: It's a new directory structure, collecting and containing all files that are (only) used for packaging. Using the canonical OS names is just nicer and better to read, I don't see why we should unnecessarily restrict ourselves.
That reasoning is subjective. Currently the only directories that start with uppercase letters are (excluding tests):
./.github/ISSUE_TEMPLATE./desktop/assets/Assets.xcassets./desktop/assets/Assets.xcassets/RuffleBlue.colorset./desktop/assets/Assets.xcassets/RuffleMacIcon.iconset./desktop/assets/Assets.xcassets/RuffleOrange.colorset./desktop/packages/macOS/Contents./web/packages/extension/safari/package/Contents
I believe all of them have a reason why they are named like that which is unrelated to readability (most of them are of course macos-related). Every other directory starts with a lowercase character. IMO it's just unnecessary to break that rule, as we will start seeing directories starting with uppercase letter here and lowercase letter there, wondering which one it is or should be.
There was a problem hiding this comment.
The last two directories are actually removed in this PR.
But for all folders but ISSUE_TEMPLATE, there are no technical reasons to have their names upper case, it's just convention. The path of Assets.xcassets is just given to actool, it could have any other name. And the colorset folders again are just names listed in Info.plist. They could be lowercase.
The reason why those are uppercase is the same as why the platform names should be: convention. Going from the other side, the reason why most other folders have lower-case numbers is because those are module and crate names, which are lowercase by convention. They are not referencing any existing name that are capitalized in a specific way.
I don't see any reason to keep convention for all other folders, but break the convention in this specific case. It's most consistent and more readable to capitalize the OS name in their proper way and stick to the convention.
There was a problem hiding this comment.
We should refer to the product we ship as "ruffle" as opposed to "ruffle desktop", the binary and crate name internally is because this is something of a monorepo and we need the internal distinction. Users do not.
For directory names: I personally don't care enough to argue one way or another as long as we're functional and consistent. We refer to OS names as an identifier lowercase most places, it makes sense that directory names follow suite. The mac one is macOS with a capital right now because it's part of the app bundle, which is an Apple specification.
There was a problem hiding this comment.
Yes, I will change it back to ruffle, especially since that's relied upon by the (newer) Flathub integration. I'll revert this when I rebase it (I can do that in the next few days).
| pkgver=@VERSION@ | ||
| pkgver=@DASH_TIME@ |
There was a problem hiding this comment.
I believe we cannot change the version just like that. vercmp says that 2024.08.18 > 2024-08-19, so we would have to increment epoch here, but again, I do not see a reason for this change.
There was a problem hiding this comment.
Thanks for noticing
There was a problem hiding this comment.
Do we want to remove the background? We had some discussions on Discord about changing the icon in order to conform to Flathub's quality guidelines. I believe currently both of them do not conform to the guidelines as the first one fills too much of the canvas (it touches the edges) and the second one fills too little of the canvas.
There was a problem hiding this comment.
This isn't about a Linux specific icon for Flatpak, but to have a general SVG file of our icon. The other icon is only the SVG version macOS specific icon with the macOS template and rounded corners. It's still there, just renamed. We can also add a Linux version conforming to the Flatpak guidelines.
There was a problem hiding this comment.
I know that we're talking about a generic icon for Ruffle on Linux. The question is then whether we should make an icon specific for Flatpak, why not just have one icon for Linux? The Flapak one is semantically exactly the same and conforms to the XDG specifications, it's not a custom, Flatpak-specific icon.
There was a problem hiding this comment.
I guess we could do that.
The reason why I did it this way is because I considered the raw R to be the default / "real" / uncostomized Ruffle icon and the macOS icon as well as the Flatpak icon just different because of different platform standards and recommendations.
But of course we can also break that up and generally make different platforms have different icons.
Maybe it would be good to also get input of other devs.
There was a problem hiding this comment.
Windows needs its own icon in its own format, mac needs its own icon in its format, it makes sense that we have one for linux which is used by flatpak and whatever else unless they too end up needing their own icon in their own format.
I know that on master we've already renamed this to a linux specific icon - and I think that's the right thing to do. This isn't the ruffle icon, it's a ruffle icon.
There was a problem hiding this comment.
on master we've already renamed this to a linux specific icon
I don't think so. We also have a Linux-specific (or at least Flathub-specific) icon which fits the Flathub guidelines.
This is just the "raw" icon without any background. I created it by taking the macOS icon and removing its background.
I do think it makes sense to keep this one, as the other icons derive from it, e.g. the macOS or the Flathub icon (by filling this raw Ruffle icon with a blue background).
And since the macOS icon was previously named "icon.svg", renaming it to "icon macOS.svg" to make this clear (and ideally naming the other specific icons in the same way) and keeping the "raw" ruffle icon as "icon.svg" is definitely an improvement (and this is the change this PR does).
| name: macos-aarch64 | ||
| name: macOS_arm64 |
There was a problem hiding this comment.
arm64 is a non-standard name of aarch64 (unless it's standard on macos?)
There was a problem hiding this comment.
Apple originally named their backend arm64 before they merged. But the main reason I changed this is because this name is more commonly known and makes it more intuitive to read and understand the workflow.
n0samu
added
A-desktop
There was a problem hiding this comment.
What's the source of this file? It doesn't appear to match the icon we're using on macos.
There was a problem hiding this comment.
This is the icon originally made for Linux (Flatpak) by Doomsdayrs
There was a problem hiding this comment.
Whoops, you're right. Thanks for noticing this! This was just named "icon" which was pretty nondescriptive. As I thought it was the macOS icon I simply renamed it (and created a new icon.svg which was just this without the background to have the "origin" version of the Ruffle icon the other ones derived of).
But yeah, I was wrong. I'll find the real macOS icon and fix this, naming the real one "icon macOS.svg" and this "icon Flathub.svg" so it's clear for each icon what it is and what it was made for.
There was a problem hiding this comment.
There's no such thing as a "Flathub icon", there's just the Linux icon. It's an updated version, made from scratch, and the original icon made for Flatpak is not used anymore (it wasn't conforming to relevant guidelines).
Also (as Dinnerbone has mentioned) the icon is package/OS-specific, and for macOS it should be placed under packages/macOS/, similarly to the Linux one which is placed under packages/linux/.
There was a problem hiding this comment.
The packages folder has been replaced by the assets and assets/packaging folders. I can place it in the OS-specific subfolders of packaging, but right now, they only contain files that are actually needed for packaging and not source files like these SVGs
This pull request improves and updates the packaging system:
It adds the creation of AppImages (for Linux) and DMG images and single architecture packages (for macOS) to the build system. Additionally, it improves, updates and corrects the packaged metadata for Windows and macOS. It also generally improves the desktop packaging structure and workflow readability.
New packages
x86-64andarm64, additionally to the existing universal package) for macOS. This also allows for thearm64image to be packaged using a newer compression format and filesystem, as older systems don't have to be supported.Improved metadata
winresourcecrate, which is used by the build script to dynamically generate an up-to-date RC file and embed it in the Windows executable.Info.plistfile (adding additional information, removing an outdated entry and improving its readability) and makes sure it always stays up-to-date by dynamically adding the version information, minimum required system version and current year while running the GitHub workflow.Other noticable changes
Currently, the name parts are separated with dashes, and underscores are used to separate elements inside some parts (e.g. time or architecture). This leads to a bad readability as underscores visually separate the text more than dashes (see
ruffle-nightly-2024_08_12-linux-x86_64.tar.gz). Therefore, the underscores and dashes have been switched. Additionally, the text is now capitalized, which also improves the readability (seeRuffle_Nightly_2024-08-12_Linux_x86-64.tar.gz; this also uses the canonical spelling of x86-64 and ISO 8601).--minimum-deployment-targetparameter (currently, 10.0 is wrongly being used).