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: 79.0 -> 80.0, firefox-esr-78: 78.1.0esr -> 78.2.0esr, firefox-esr-68: 68.11.0esr -> 68.12.0esr #96454

Merged
3 commits merged into from Aug 29, 2020

Conversation

stigtsp
Copy link
Member

@stigtsp stigtsp commented Aug 27, 2020

Motivation for this change

Security and bug fixes

firefox: 79.0 -> 80.0
https://www.mozilla.org/en-US/firefox/80.0/releasenotes/
https://www.mozilla.org/security/advisories/mfsa2020-36/

firefox-esr-78: 78.1.0esr -> 78.2.0esr
https://www.mozilla.org/en-US/firefox/78.2.0/releasenotes/
https://www.mozilla.org/security/advisories/mfsa2020-38/

firefox-esr-68: 68.11.0esr -> 68.12.0esr
https://www.mozilla.org/en-US/firefox/68.12.0/releasenotes/
https://www.mozilla.org/security/advisories/mfsa2020-37/

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 pkgs 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@stigtsp
Copy link
Member Author

stigtsp commented Aug 27, 2020

Local build and testing is underway. Hm, not enough ram to build 80.0

@ajs124
Copy link
Member

ajs124 commented Aug 27, 2020

The 80.0 build failed for me, when I did these bumps, which is why I didn't open a PR. Haven't gotten around to debugging it yet, though.

@stigtsp stigtsp marked this pull request as draft August 27, 2020 11:31
@vcunat
Copy link
Member

vcunat commented Aug 28, 2020

/build/firefox-80.0/obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/IOUtils.h:17:10: fatal error: nspr/prio.h: No such file or directory
   17 | #include "nspr/prio.h"
      |          ^~~~~~~~~~~~~
compilation terminated.

We have ${nspr.dev}/include/prio.h... actually I don't understand why nspr is using so "ambiguous" include paths like <prio.h> directly.

@vcunat
Copy link
Member

vcunat commented Aug 28, 2020

Well... probably nothing important, as with the following patch it built fine:

--- a/pkgs/development/libraries/nspr/default.nix
+++ b/pkgs/development/libraries/nspr/default.nix
@@ -37,6 +37,7 @@ stdenv.mkDerivation {
   postInstall = ''
     find $out -name "*.a" -delete
     moveToOutput share "$dev" # just aclocal
+    ln -s . "$dev/include/nspr"
   '';
 
   buildInputs = [] ++ stdenv.lib.optionals stdenv.isDarwin [ CoreServices ];

@stigtsp
Copy link
Member Author

stigtsp commented Aug 28, 2020

Well... probably nothing important, as with the following patch it built fine:

Thx!

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

Will build and then test firefox. Should we add nspr to the title? So this pr is easier to find if something breaks?

@stigtsp
Copy link
Member Author

stigtsp commented Aug 28, 2020

The change to nspr will trigger a mass rebuild, this should be avoided imho. I'm not very familiar with building firefox, so any help is welcome. :)

Currently looking into:

  • disabling --with-system-nspr
  • patching in an other path to the nspr includes

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

Why does this occurs? Is it a packaging problem in nspr? Should it be fixed either way?

@vcunat
Copy link
Member

vcunat commented Aug 28, 2020

500--1000... isn't really a large rebuild. That's way smaller than the usual FF updates where we had to bump nss. But certainly, if you find a nice way of avoiding it, I won't mind. My patch was a quick hack anyway; I don't understand why the problem started.

@ajs124
Copy link
Member

ajs124 commented Aug 28, 2020

There already is a nspr and nss bump in staging right now, through #96185

That's probably unrelated to what we're seeing here, but just so everyone is aware in case it is related.

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

So maybe somehow include the nspr fix in staging? (Would be greate if FF 80 got into 20.09.

Firefox build on my side, and everything seems working.

@vcunat
Copy link
Member

vcunat commented Aug 28, 2020

Firefox has always been backported so far, so I expect it will make it in any case. EDIT: current staging most likely won't make it to 20.09, BTW.

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

Firefox has always been backported so far, so I expect it will make it in any case. EDIT: current staging most likely won't make it to 20.09, BTW.

That's why I mentioned that i would like FF in 20.09. I'm currently trying to build it with the old nspr, and like 4 lines fixed in firefox.
But even my biggest machine takes some time building Firefox. So maybe I can report success tomorrow

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

Oh, I just noticed those errors, but it can be that they are from my config, which is kinda broken. This errors occur when right clicking an tab, trying to open it in another container (with the mozzilla container addon)

console.warn: LoginRecipes: "getRecipes: falling back to a synchronous message for:" "https://github.com"
JavaScript error: resource://gre/modules/TelemetrySession.jsm, line 1166: Error: TelemetryStopwatch: finishing nonexisting stopwatch. Histogram: "FOG_EVAL_WINDOW_RAISED_S", key: ""
Gdk-Message: 20:16:16.192: Unable to load hand2 from the cursor theme
JavaScript error: resource://gre/modules/ExtensionChild.jsm, line 813: DataCloneError: The object could not be cloned.
JavaScript error: , line 0: AbortError: The operation was aborted.
Gdk-Message: 20:17:01.923: Unable to load hand2 from the cursor theme
Gdk-Message: 20:17:03.838: Unable to load hand2 from the cursor theme
Gdk-Message: 20:17:22.635: Unable to load hand2 from the cursor theme
Gdk-Message: 20:17:23.412: Unable to load split_h from the cursor theme
Gdk-Message: 20:17:23.412: Unable to load sb_h_double_arrow from the cursor theme
Gdk-Message: 20:17:23.429: Unable to load hand2 from the cursor theme
Gdk-Message: 20:17:23.495: Unable to load hand2 from the cursor theme
JavaScript error: resource://gre/modules/ExtensionChild.jsm, line 813: DataCloneError: The object could not be cloned.
JavaScript error: , line 0: AbortError: The operation was aborted.

@Kloenk
Copy link
Member

Kloenk commented Aug 28, 2020

Exactly tested your last commit (just build it my self). That firefox works for me, and also the error is not there anymore.
So looks good to me now

@stigtsp
Copy link
Member Author

stigtsp commented Aug 28, 2020

Did a substituteInPlace on IOUtils.{h,cpp}, this seems to work and avoids having to patch nspr. Rebuilding, and testing.

 postPatch = ''
    rm -rf obj-x86_64-pc-linux-gnu
  '' + lib.optionalString (lib.versionAtLeast ffversion "80") ''
    substituteInPlace dom/system/IOUtils.h \
      --replace '#include "nspr/prio.h"'          '#include "prio.h"'

    substituteInPlace dom/system/IOUtils.cpp \
      --replace '#include "nspr/prio.h"'          '#include "prio.h"' \
      --replace '#include "nspr/private/pprio.h"' '#include "private/pprio.h"' \
      --replace '#include "nspr/prtypes.h"'       '#include "prtypes.h"'
  '';

Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

I expect it will be OK as usual. I used each for at least a few minutes. I can't see anything risky for us in announcements.

@Kloenk
Copy link
Member

Kloenk commented Aug 29, 2020

Anyone tested the Firefox esr updates?

@stigtsp
Copy link
Member Author

stigtsp commented Aug 29, 2020

Anyone tested the Firefox esr updates?

Did a very quick test of firefox-esr-68, firefox-esr-78 and firefox. All seem OK.

@ghost ghost merged commit 057b30b into NixOS:master Aug 29, 2020
@ghost
Copy link

ghost commented Aug 29, 2020

This needs a backport to 20.03, is anyone interested in preparing the PR and doing basic testing?

@vcunat
Copy link
Member

vcunat commented Aug 29, 2020

I can certainly do the builds and a few minutes of usage.

@Kloenk
Copy link
Member

Kloenk commented Aug 29, 2020

I can certainly do the builds and a few minutes of usage.

Should I help building/testing? I also have a buildfarm

vcunat pushed a commit that referenced this pull request Aug 30, 2020
(cherry picked from commit 057b30b)
PR for master: #96454.
vcunat pushed a commit that referenced this pull request Aug 30, 2020
(cherry picked from commit ba671f6)
PR for master: #96454.
vcunat pushed a commit that referenced this pull request Aug 30, 2020
(cherry picked from commit c408178)
PR for master: #96454.
@vcunat
Copy link
Member

vcunat commented Aug 30, 2020

Done. Anyone can still continue testing and commenting, though in case of 20.03 the update is likely to hit channels soon (today), contrary to nixos-unstable that's blocked ATM.

This pull request was closed.
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

4 participants