Add initial support for link flag #403

tiziodcaio · 2023-10-02T10:15:36Z

The link flag implement a mechanism (only for linux at the moment) to avoid downloading firefox from the mozilla server. Instead it will symlink to the distro installed one

Embrace more cases (recognise developer-edition and others archs).
I need help for fixing the connector in the GUI, at the moment is broken.
TUI works well.

filips123

Symlinks also work on other systems (although I think they require admin on Windows), so the Linux check should probably be removed. Then, the default source path for the runtime should also be updated to detect paths on other OSes.

To allow different Firefox installations (ESR, Developer Edition, Nightly, etc.) in different directories, it might be useful to change the --link argument to allow an optional value to specify the source path. When the path is specified, it should be linked instead of the default path. When it is not, the default path (see above) should be used. This probably also means that the source path also needs to be stored somewhere in the config and configurable through the extension settings.

Another thing is that the extension setup wizard should be updated to allow users to link the runtime instead of downloading it. The extension could also hardcode a few known paths for different Firefox versions on different OSes so users wouldn't have to manually search the path, but also still allow manual input in case the path is different. However, this can be done later.

I don't have much time currently, but I hope I will be able to review PR more next week.

This isn't directly related to your PR, but another way of "linking" the global runtime is using FUSE OverlayFS (#67, #214). So, in the future, another option similar to --link (maybe --overlay) could be added to use FUSE OverlayFS to link the runtime.

native/src/components/runtime.rs

filips123 · 2023-10-08T12:41:08Z

extension/src/sites/manage.js

+    // Listen for use Linked runtime
+    document.getElementById('settings-use-linked-runtime').addEventListener('change', async function () {
+      config.use_linked_runtime = this.checked
+      await setConfig(config)
+    })


For the extension settings, you can check how settings like settings-always-patch are implemented. So, this code should be inside setTimeout(async () => { (around line 875).

tiziodcaio · 2023-10-08T14:24:02Z

Symlinks also work on other systems (although I think they require admin on Windows), so the Linux check should probably be removed. Then, the default source path for the runtime should also be updated to detect paths on other OSes.

Well.. At the moment I can work into a Linux OS only, because of this I inserted the checks... I thought that was the simplest method to implement without breaking it in macOS (you need more work because of the .app bundle) and Windows (symbolic and hard links needs the administrator privileges)

To allow different Firefox installations (ESR, Developer Edition, Nightly, etc.) in different directories, it might be useful to change the --link argument to allow an optional value to specify the source path. When the path is specified, it should be linked instead of the default path. When it is not, the default path (see above) should be used. This probably also means that the source path also needs to be stored somewhere in the config and configurable through the extension settings.

Yeah... I really want to make the thing more flexible, because at the moment I tested from Arch Linux only, and that's because it needs a bit of testing and it's immature merge yet in my opinion.

Another thing is that the extension setup wizard should be updated to allow users to link the runtime instead of downloading it. The extension could also hardcode a few known paths for different Firefox versions on different OSes so users wouldn't have to manually search the path, but also still allow manual input in case the path is different. However, this can be done later.

I thought about that, but I went into the backend firstly, so I've not studied how to implement into the extension yet

This isn't directly related to your PR, but another way of "linking" the global runtime is using FUSE OverlayFS (#67, #214). So, in the future, another option similar to --link (maybe --overlay) could be added to use FUSE OverlayFS to link the runtime.

Oh. I didn't see it. Interesting idea... But I'm not really know well the OverlayFS technology so I will wait someone more expert 😉

filips123 · 2023-10-10T10:32:17Z

Well.. At the moment I can work into a Linux OS only, because of this I inserted the checks... I thought that was the simplest method to implement without breaking it in macOS (you need more work because of the .app bundle) and Windows (symbolic and hard links needs the administrator privileges)

I almost forgot macOS uses .app bundles and requires special handling... So, it's probably fine if support for macOS is added later.

And for Windows, other types of shortcuts/links are supported, and some of them appearently don't require admin privileges. For example, Rustup uses junctions instead of symlinks on Windows to prevent requiering admin. However, it uses a lot of low-level unsafe code and direct calls to Windows API, which doesn't seem like a good solution, so this can also wait until there is a better one.

Also, another thing related to symlinks/junctions is that we need to check what happens when they are deleted. Will it only remove the symlink itself or also try to delete the actual content? This should be checked for normal deleting through the file manager, shell and functions that are used in the native program (remove_dir_all and remove_dir_contents).

I thought about that, but I went into the backend firstly, so I've not studied how to implement into the extension yet

Yeah, the extension stuff (mainly the setup wizard) can also wait until the native implementation is ready.

VarLad · 2023-10-23T17:30:35Z

@filips123 (apologies for the out of context question)
But I wonder if this PR would help in alleviating the challenges in packaging PWAsForFirefox as a flatpak extension?

filips123 · 2023-10-24T11:16:03Z

@VarLad The only thing it would help with is that you wouldn't need to have two instances of Firefox installed (which is not really required but can still be useful, even without Flatpak). However, Flatpak still can't be supported properly until they add support for Native Messaging API.

VarLad · 2023-10-24T11:23:36Z

@filips123 Why not package this as a flatpak extension?

filips123 · 2023-10-26T20:26:15Z

I'm still not sure how useful would that be and it it could completely work. The native program needs some additional permissions (to create desktop files and icons) and I don't know if this can work as an extension. Additionally, it makes it hard to use alternative browser (either as a main browser or as an app runtime).

But I can check if it can be done. If it can work at least partially, it's probably still better than nothing, at least until native messaging is properly supported.

tiziodcaio · 2023-10-29T17:41:54Z

Another problem would be the update of firefox, since we should check if the binary is going to be updated. Maybe an hard link instead of softlink on the firefox & firefox-bin..? I will try in these days

tiziodcaio · 2024-01-22T17:20:03Z

I think is more mature now, I cleaned up a bit. What do you think it is missing to be merged with experimental support?

The binary won't update with distro packages and meanwhile it will break itself. Maybe implement some kind of checksum and replace? Or hard link?
Add to the extension GUI some options.
Add support to:
- Windows
- macOS
- BSD (test)

filips123 · 2024-01-22T22:22:04Z

Why is copying binaries required? Does it not work when using symlinks?

The binary won't update with distro packages and meanwhile it will break itself.

This is probably the main thing that should be implemented before this can be merged.

Normal symlinks would probably be the best option if you can make them work. Hard links are also probably quite easy to implement, but don't work across partitions (I think), which might be a problem on systems that have a separate /home partition. I don't know if there are any other "easy" options. Maybe the program could check if the source binaries have different modification time and copy them right before launching the web app (as it's already done for profile and runtime patching).

It also shouldn't be that hard to implement BSD support. I think there are a still few places left which limit this to Linux and should be fixed. Also, I think that Firefox on BSD is in /usr/local/lib/firefox/ (this needs to be checked), so the source path needs to change dependeng on the OS. Support for other OSes can be added in the future.

Also regarding paths, it would be nice if the source path can be configured by the user (see my first comment). However, in this case, the path might also need to be stored in the config, which is probably more work, so it can be done later.

Extension UI can probably also be added later, or I can implement it myself (as it's a bit harder now because of localization).

tiziodcaio · 2024-01-22T22:32:24Z

Binaries has to be copied due to firefox, which at the moment it starts itself it grabs its configs from the folder in which it was put, symlinks are not enough. Copies are only 700KB, so is not a problem of weight, but only in cases of updating it might break, so as we said we need to think how to implement it.

I discarded hard links because they are harder to understand, and I am the first with a separated home partition (already tested and yes, because of that it cannot work).

We should check more paths in which Firefox might be installed, like flatpak, snaps, distros and BSDs.

Source path should not be neither complicated to implement, it got lost in the comments.

I am agreeing on extension. Maybe keeping it to test in tui mode it would the best idea at the moment.

So we will keep at the moment this feature unix-only, then if we have time we can think about windows and finally MacOS (is the most complicated with all the bundles thing in my opinion)

filips123 · 2024-01-23T15:32:42Z

Binaries has to be copied due to firefox, which at the moment it starts itself it grabs its configs from the folder in which it was put, symlinks are not enough. Copies are only 700KB, so is not a problem of weight, but only in cases of updating it might break, so as we said we need to think how to implement it.

Then, unless there is some trick to make Firefox work with symlinks, this could probably be solved if the program checks if binaries are outdated every time when launching a web app and copy them again when needed.

For example, a similar thing is already used to check if the patches should be applied.

In this case, the program could do something like this:

Do this inside SiteLaunchCommand.
Check if use_linked_runtime is enabled.
If enabled, get the source Firefox path (which probably needs to be stored in the config if it is configurable by the user).
Then, check file modification dates for binaries in the source and target directories.
If the source dates are newer than existing target ones, copy them over.

Also, eventually, I would like to get this work even when immutable-runtime is enabled. One purpose of immutable-runtime feature is that when distros provide Firefox with their own custom patches which are necessary to run it (like on Nix and Guix, see #204), users can't accidentally overwrite it with the normal Mozilla version with firefoxpwa runtime install. Then, distros would probably provide their runtime in a global /usr/share/firefoxpwa/runtime. In this case, the link option is still useful when users want to link another Firefox version (still with distro patches though), and should then link the runtime into user-specific directory which takes precedence over the global one.

However, to make this work, I will also need to change how the runtime directory is determined, so it's out of scope for this PR. For now, it's fine if linking is disabled when immutable-runtime is enabled.

native/src/console/site.rs

tiziodcaio · 2024-02-08T17:45:27Z

There are still some problems with linking... Re putting in draft

filips123 · 2024-02-08T18:01:23Z

I probably won't have much time in the next two weeks, but then I can review and probably merge it.

BROKEN - I need help for fixing the connector in the GUI. The TUI works well.

Added "support" to BSD, needed testing Disabled when using immutable-runtime feature Removed/cleaned some branches of rust code for readability

Because otherwise it will fail due to existance of old files

filips123 · 2024-03-28T21:32:26Z

There are some clippy and compile errors on Windows and macOS, mostly because of runtime.link which doesn't exist on those systems.

There is also a compile error on Linux, but it is unrelated to this PR, and I don't know yet why it happens...

Also, I reverted the extension change for now.

tiziodcaio · 2024-03-28T23:26:42Z

Great! I decided to push the changes onto a feature, and I fixed the clippy errors on macOS/windows!

filips123 · 2024-03-30T12:21:53Z

I will try to fix a few remaining CI failures, and then I think this initial support should be ready. In the future, I will also change how detecting runtime works (as I mentioned before), but it might take some time and I don't know when it will be ready.

Also, should this feature be enabled by default in the release builds, or should it be disabled until support is improved (with support for custom paths, other systems and extension UI)?

filips123

I think this initial version is fine. I will probably keep it disabled by default for now though, so you will have to compile it manually to enable the feature.

filips123 requested changes Oct 8, 2023

View reviewed changes

filips123 mentioned this pull request Dec 11, 2023

NixOS Packaging #204

Closed

tiziodcaio commented Jan 23, 2024

View reviewed changes

native/src/console/site.rs Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

tiziodcaio marked this pull request as ready for review February 8, 2024 16:28

tiziodcaio requested a review from filips123 February 8, 2024 16:29

tiziodcaio marked this pull request as draft February 8, 2024 17:45

tiziodcaio added 6 commits March 20, 2024 21:59

Add link flag

2b1876e

BROKEN - I need help for fixing the connector in the GUI. The TUI works well.

Fix serializing

19773f9

Mix fixes

9300fdc

Added "support" to BSD, needed testing Disabled when using immutable-runtime feature Removed/cleaned some branches of rust code for readability

Always uninstall after trying to link it

234daf9

Because otherwise it will fail due to existance of old files

Add initial support for checksumming.

101bc17

Compare instead of store an hash

2e249b6

tiziodcaio force-pushed the main branch from 0a014bc to 2e249b6 Compare March 20, 2024 21:14

"Fix" borked support to bsd...

1841cc5

tiziodcaio marked this pull request as ready for review March 22, 2024 14:40

Discard changes to extension/src/sites/manage.js

53aca9a

Use a feature instead of OS checks

c788a7b

filips123 added 2 commits March 30, 2024 14:33

Reformat and fix a few problems

a123e6c

Merge branch 'main' into PR

e4fc0b6

filips123 force-pushed the main branch from 44d24d1 to e4fc0b6 Compare March 30, 2024 13:36

filips123 approved these changes Apr 27, 2024

View reviewed changes

filips123 changed the title ~~Add link flag~~ Add initial support for link flag Apr 27, 2024

filips123 merged commit ea94d33 into filips123:main Apr 27, 2024
18 checks passed

filips123 added this to the 2.12.0 milestone May 1, 2024

filips123 mentioned this pull request Oct 8, 2024

about:config changes not setting #578

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial support for link flag #403

Add initial support for link flag #403

tiziodcaio commented Oct 2, 2023 •

edited

Loading

filips123 left a comment

filips123 Oct 8, 2023

tiziodcaio commented Oct 8, 2023

filips123 commented Oct 10, 2023

VarLad commented Oct 23, 2023

filips123 commented Oct 24, 2023

VarLad commented Oct 24, 2023

filips123 commented Oct 26, 2023

tiziodcaio commented Oct 29, 2023 •

edited

Loading

tiziodcaio commented Jan 22, 2024 •

edited

Loading

filips123 commented Jan 22, 2024

tiziodcaio commented Jan 22, 2024

filips123 commented Jan 23, 2024

This comment was marked as outdated.

tiziodcaio commented Feb 8, 2024

filips123 commented Feb 8, 2024

filips123 commented Mar 28, 2024 •

edited

Loading

tiziodcaio commented Mar 28, 2024

filips123 commented Mar 30, 2024

filips123 left a comment

Add initial support for link flag #403

Add initial support for link flag #403

Conversation

tiziodcaio commented Oct 2, 2023 • edited Loading

filips123 left a comment

Choose a reason for hiding this comment

filips123 Oct 8, 2023

Choose a reason for hiding this comment

tiziodcaio commented Oct 8, 2023

filips123 commented Oct 10, 2023

VarLad commented Oct 23, 2023

filips123 commented Oct 24, 2023

VarLad commented Oct 24, 2023

filips123 commented Oct 26, 2023

tiziodcaio commented Oct 29, 2023 • edited Loading

tiziodcaio commented Jan 22, 2024 • edited Loading

filips123 commented Jan 22, 2024

tiziodcaio commented Jan 22, 2024

filips123 commented Jan 23, 2024

This comment was marked as outdated.

tiziodcaio commented Feb 8, 2024

filips123 commented Feb 8, 2024

filips123 commented Mar 28, 2024 • edited Loading

tiziodcaio commented Mar 28, 2024

filips123 commented Mar 30, 2024

filips123 left a comment

Choose a reason for hiding this comment

tiziodcaio commented Oct 2, 2023 •

edited

Loading

tiziodcaio commented Oct 29, 2023 •

edited

Loading

tiziodcaio commented Jan 22, 2024 •

edited

Loading

filips123 commented Mar 28, 2024 •

edited

Loading