Add string path #660
Add string path #660
Conversation
Still defaults to `FFOX`
There was a problem hiding this comment.
Sorry for not reviewing this earlier. I was quite busy with some other things, and when I had time, I wanted to first address some more important issues.
I think the idea in general is fine. It should be tested more, but I can't currently do much work on the extension because of #747.
One thing that should also be tested is what happens when the runtime is updated to a new Firefox version. Appearently, there can be some problems in the FUSE OverlayFS method for runtime linking that when the user updates the runtime from a web app, it can "desync" from the source/global runtime and break (#715). So, it should be tested what happens here.
Support for BSD likely already works, just compilation checks need to be removed/adjusted. Linked runtime is actually also already used in Termux (#684), although with custom patches, but I plan to upstream them. macOS and Windows probably require more work.
| // Listen for use linked Runtime | ||
| document.getElementById('settings-use-linked-rt').addEventListener('change', async function () { | ||
| config.always_patch = this.checked | ||
| await setConfig(config) | ||
| }) |
There was a problem hiding this comment.
It seems handling the runtime path is missing here.
| self.uninstall()?; | ||
|
|
||
| storage.config.use_linked_runtime = true; | ||
| storage.config.linked_runtime_path = FFOX.to_string(); |
There was a problem hiding this comment.
This probybly shouldn't override the user-configured path.
| /// Experimental: Use a linked runtime instead of downloading from Mozilla | ||
| #[cfg(target_os = "linux")] | ||
| #[clap(long)] | ||
| #[clap(long, short)] | ||
| pub link: bool, | ||
| /// Experimental: Use a linked runtime instead of downloading from Mozilla. | ||
| /// Optional: Path of my firefox runtime | ||
| #[cfg(target_os = "linux")] | ||
| #[clap(long, value_name = FFOX)] | ||
| pub path: String, |
There was a problem hiding this comment.
Maybe it would be better if this is a single option/parameter that can optionally take a path. So, when firefoxpwa runtime install --link is ran, the default path is used (depending on the operating system, distribution, etc.), and when the user explicitly specifies firefoxpwa runtime install --link=SOME-PATH, that path is used instead.
This should probably also be reflected in the storage/config. So that the path is only stored when its explicitly set.
| document.getElementById('settings-use-linked-rt').checked = config.use_linked_runtime | ||
| document.getElementById('settings-settings-linked-rt-path').checked = config.linked_runtime_path |
There was a problem hiding this comment.
runtime instead of rt for element IDs seems nicer.
2 participants
Handle more cases; added to the extension's frontend. Needs testing rn.