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

firefox: Added checks for new addon behaviour since v91 #133504

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

Qubasa
Copy link
Contributor

@Qubasa Qubasa commented Aug 11, 2021

Motivation for this change

Firefox v91 does not support addons with invalid signature anymore. Firefox ESR needs to be used for nix addon support.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@@ -24470,6 +24470,7 @@ with pkgs;
firefox-esr-wayland = wrapFirefox firefox-esr-91-unwrapped { forceWayland = true; };
firefox-esr-78 = wrapFirefox firefox-esr-78-unwrapped { };
firefox-esr-91 = wrapFirefox firefox-esr-91-unwrapped { };
firefox-esr-unwrapped = firefoxPackages.firefox-esr-91;
Copy link
Member

Choose a reason for hiding this comment

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

firefox-esr-unwrapped being 91.x while firefox-esr is 78.x seems… interesting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but it seems a mistake by a previous committer as firefox-esr-wayland is v91. I will bump firefox-esr

Copy link
Member

Choose a reason for hiding this comment

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

No. Just look through the commit history.

@ajs124 ajs124 requested a review from mweinelt August 11, 2021 18:21
@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 133504 at ab4f238b run on x86_64-linux 1

2 packages built successfully:
  • firefox-esr-unwrapped
  • nixos-install-tools

Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

I will bump firefox-esr

That's honestly not up to you, leave that to us maintainers, please.

a mistake by a previous committer as firefox-esr-wayland is v91.

The sole reason I created firefox-esr-wayland based on 91esr is that I didn't want to introduce this on old 78esr, which I never actually tested on wayland. I'm much more confident in 91 for this and there didn't need to be any backward compat, because it was newly introduced.

Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

Needs more clarification about 91.0 vs 91.0 ESR, also carries a stray change that needs to be removed.


```nix
{
myFirefox = wrapFirefox firefox-unwrapped {
# Nix firefox addons only work in ESR build.
Copy link
Member

Choose a reason for hiding this comment

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

Only in ESR builds meaning 91.0 ESR works, but 91.0 does not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct

Copy link
Member

Choose a reason for hiding this comment

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

Maybe say 78/91 ESR to be clearer here. We can likely expect 91 ESR to be the last ESR series to allow for unsigned addons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I don't think so because there needs to be a way for corporations to deploy internal addons not published in the firefox addon store~ I rewrote it to Nix firefox addons only work in the firefox-esr package . Is that okay?

nixos/doc/manual/from_md/release-notes/rl-2111.section.xml Outdated Show resolved Hide resolved
nixos/doc/manual/release-notes/rl-2111.section.md Outdated Show resolved Hide resolved
@Qubasa Qubasa requested a review from mweinelt August 26, 2021 17:43
@mweinelt mweinelt merged commit 7e1cdd2 into NixOS:master Aug 26, 2021
@Qubasa Qubasa deleted the fix_firefox2 branch August 26, 2021 17:52
aszlig added a commit to aszlig/nixpkgs that referenced this pull request Jun 21, 2022
Firefox 61 started to enforce signatures for add-ons and since
commit d031843, we get an evaluation
error that recommends the user to switch to Firefox ESR.

This isn't an option for everyone and as I also pointed out in the pull
request[1] introducing the above commit, I've been building Firefox like
this:

  let
    firefoxNoSigning = firefox-unwrapped.overrideAttrs (lib.const {
      MOZ_REQUIRE_SIGNING = false;
    });
  in wrapFirefox firefoxNoSigning {
    nixExtensions = ...;
  }

However, this only works after manually modifying nixpkgs (or copy &
paste wrapper.nix elsewhere) every time I want to have a new Firefox
version. Of course, this gets annoying and tedious after a while, so
this motivated me to properly fix this to not only check for an ESR
version but also check the value of MOZ_REQUIRE_SIGNING.

Note that I'm using toString here to check for the value because there
are several ways (false, null, "", ...) to set the environment variable
to an empty string and toString makes sure that it really is the desired
behaviour. I specifically checked the Firefox source and also tested
this with multiple values and only building with MOZ_REQUIRE_SIGNING
set to an empty string seems to work (no "0", "false" or other
variants).

Additionally, there is another method to allow unsigned add-ons, which
is by using the --with-unsigned-addon-scopes configure option[2].
Unfortunately, this does not work with nixExtensions because we don't
have (or want) a central directory where those add-ons reside.

Given that nixExtensions disallows manually installing add-ons, setting
MOZ_REQUIRE_SIGNING to false should be safe in this case.

[1]: NixOS#133504
[2]: https://bugs.archlinux.org/task/63075

Signed-off-by: aszlig <aszlig@nix.build>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants