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

chore: replace x86_64-only Flatpaks with amd64 alternatives #215

Merged
merged 6 commits into from
Feb 1, 2025

Conversation

p5
Copy link
Member

@p5 p5 commented Jan 31, 2025

I'm not too sure how the overrides work, so please give this a good review :)

This replaces Firefox with Chromium (the next best option for a browser).
And removes Thunderbird, since it's not really needed when you have a browser.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copied from the ublue-os/packages file, with Firefox replaced with Chromium.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can add this to the package itself when using aarch64. Putting a pin on this

@p5 p5 marked this pull request as ready for review January 31, 2025 21:14
@p5 p5 requested a review from tulilirockz as a code owner January 31, 2025 21:14
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jan 31, 2025
castrojo
castrojo previously approved these changes Feb 1, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 1, 2025
@castrojo castrojo enabled auto-merge February 1, 2025 03:46
@tulilirockz tulilirockz disabled auto-merge February 1, 2025 03:47
@castrojo castrojo enabled auto-merge February 1, 2025 03:47
@tulilirockz
Copy link
Collaborator

Nono!

@tulilirockz
Copy link
Collaborator

Wanted to talk to p5 about this but I think this PR looks good

@castrojo castrojo disabled auto-merge February 1, 2025 03:49
@tulilirockz
Copy link
Collaborator

It looks good! Im just thinking like, the stuff on overrides should be build on top of the existing build, everything that is not explicitly a override should work, always, no matter arch, no matter the overrides. putting the file copying before makes the non-overrides build "impure", thus it can break when certain files are or are not present on the overrides, that locks us into having those always when building images with the overrides like -hwe.

For example, imagine we have some systemd configuration that needs to be there specifically for -dx because of some program we added 10 months ago, and we dont have any idea why not having it breaks the previous steps when building -dx (e.g.: image-info.sh or something), that would lock us into having that file forever, instead of doing an "whitelist" pattern with the overrides

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we can just have a sed -i statement on a script under build_script/aarch64 instead of having this file here - think about the defaults, it should always be the non-overrides, then maybe do stuff like this if strictly necessary. In this case this would just add a bit of overhead when maintaining flatpaks because we would need to manually compare the files and see what we want.

@@ -32,20 +22,34 @@ copy_systemfiles_for() {
WHAT=$1
shift
printf "::group:: ===%s-file-copying===\n" "$WHAT"
rsync -rvK "/var/tmp/system_files_overrides/$WHAT/" /
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this necessary? Usually rsync output is more verbose, I get not relying on it but I think we might need this here?

@tulilirockz tulilirockz force-pushed the update-aarch-with-supported-flatpaks branch from 03b2aaf to f540062 Compare February 1, 2025 19:00
@tulilirockz tulilirockz enabled auto-merge February 1, 2025 19:10
@tulilirockz tulilirockz disabled auto-merge February 1, 2025 19:15
@tulilirockz tulilirockz merged commit 80fdcf0 into main Feb 1, 2025
17 checks passed
@tulilirockz tulilirockz deleted the update-aarch-with-supported-flatpaks branch February 1, 2025 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants