chromium: replace update.nix with Python impl #72932

alyssais · 2019-11-06T20:17:21Z

update.nix was a huuuuge hack, abusing checksum collisions, etc., and
was extremely difficult to read and maintain, especially because
values from update.nix were also used in the derivations themselves!

I've replaced this with an implementation in Python, which I chose for
readability. Chromium is quite big, and we have to download three of
them, so I added some parallelism. It could still be faster, but this
is good enough for now.

I also removed some stuff that didn't ever seem to have been used,
like i686 binary checksums (which didn't seem to exist even at the
time the script was written). Rather than generating Nix, I chose to
generate JSON, since Python can do that in the standard library and
Nix can read it.

I also set update.py as an updateScript, so Chromium can now
automatically be updated!

Fixes #89635.

samueldr

Without having ran it, simply reading, this passes the sniff test. This looks like a manageability improvement.

I'll leave the actual approval and validation to @aszlig :).

aszlig

Thanks a lot for this, I guess it's time to get rid of the hash collision workaround :-)

While I'd be happy to merge this right in, this unfortunately is a step backwards from the updater we have so far, so I commented on the specifics accordingly. So while it's not necessary to be a 100% in parity with the previous updater, the new updater has a few particular annoyances that I think need to be addressed.

Also, since you changed a few things in the Nix expressions of Chromium itself, let's also ping @bendlas and @ivan here.

aszlig · 2019-11-06T20:49:27Z

pkgs/applications/networking/browsers/chromium/update.py

+from urllib.request import urlopen
+
+DEB_URL = "https://dl.google.com/linux/chrome/deb/pool/main/g"
+BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official";


Suggested change

BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official";

BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official"

aszlig · 2019-11-06T20:55:21Z

pkgs/applications/networking/browsers/chromium/update.py

+BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official";
+
+def nix_prefetch_url(url, hash="sha256"):
+    out = subprocess.check_output(["nix-prefetch-url", "--type", hash, url])


This will break the whole updating process if omahaproxy refers to a tarball that doesn't exist. That's why I did the md5/sha1 collision in the first place (it's basically the same as builtins.tryEval (builtins.fetchurl url)).

For example, right now the following is returned by omahaproxy:

os channel current_version previous_version current_reldate previous_reldate branch_base_commit branch_base_position branch_commit true_branch v8_version

linux dev 80.0.3955.4 79.0.3945.16 11/01/19 10/29/19 1be17a97af38b81c6b4bd01cd698e8329f62857b 710886 2fa66c8c902846e47cab48edf8aea6c8b4e3a2d8 3955 8.0.113

linux beta 79.0.3945.16 78.0.3904.85 10/31/19 10/30/19 e4635fff7defbae0f9c29e798349f6fc0cce4b1b 706915 66fad076c1051f8d10555c8fe1de36f6a8effc69 3945 7.9.317.14

linux stable 78.0.3904.97 78.0.3904.87 11/06/19 10/31/19 675968a8c657a3bd9c1c2c20c5d2935577bbc5e6 693954 2fa4ee555b8b5b382d68e1118ab7e6eac293a039 3904 7.8.279.23

However, https://commondatastorage.googleapis.com/chromium-browser-official/chromium-78.0.3904.97.tar.xz currently returns with HTTP 404.

Furthermore, the use of nix-prefetch-url here will shovel the whole tarball into memory, so executing these in parallel will cause trouble on machines with limited amount of memory.

Addendum to that and to explain the reason for the builtins.tryEval (builtins.fetchurl url) hack:

The old updater not only checked the main omahaproxy URL but also http://omahaproxy.appspot.com/history and whenever a given tarball doesn't exist in the main URL, the updater would walk back the history until it finds a version where the tarball actually exists.

This is done only via a curl -I (HTTP HEAD) request and is used to get an overview of all the latest versions that actually exist in the upstream bucket.

Only the result of this is compared against the existing upstream-info.nix, so that the updater always produced a valid result of the latest available versions, even if some upstream tarballs are non-existing.

Shouldn't nix-prefetch-url return an error if the url to be fetched shows a 404?

Other than that, yeah, the updater could be extended to also probe http://omahaproxy.appspot.com/history (and we'd need to add this to the src attribute too)

Shouldn't nix-prefetch-url return an error if the url to be fetched shows a 404?

Of course and that's why I introduced that hash collision hack to do builtins.tryEval (builtins.fetchurl url) natively in Nix.

Other than that, yeah, the updater could be extended to also probe http://omahaproxy.appspot.com/history (and we'd need to add this to the src attribute too)

Especially since back in the days where I was maintaining Chromium, this happened very frequently (like on almost every single update). Not sure however if this is still the case, but from my comment above it still happened in November 2019.

Yeah, I think we might want to have the update script figure out the real location where things can be downloaded from, and then just write that URL to upstream-info.json, removing all the complicated "look in many different places" logic at build time.

aszlig · 2019-11-06T21:01:41Z

pkgs/applications/networking/browsers/chromium/update.py

+    # binaries at the same.  The tasks aren't dependent on each other
+    # -- it's just more annoying to assemble the final channels
+    # dictionary that way.
+    channels = dict(pool.starmap(prefetch_source, channels.items()))


Given that upstream-info.json is not checked before, this will always download all the tarballs even though no update is needed, so I'd suggest reading upstream-info.json and only download tarballs if there is a new version available.

aszlig · 2019-11-06T21:07:23Z

pkgs/applications/networking/browsers/chromium/update.py

+    channels = dict(pool.starmap(prefetch_source, channels.items()))
+    channels = dict(pool.starmap(prefetch_bin, channels.items()))
+
+argv = ['nix-instantiate', '--eval', '--json', '-A', 'chromium.meta.position']


This will fail if the script is run from any other directory other than the repository root, so I'd instead use something like this:

os.path.dirname(os.path.abspath(__file__))

... which I think is also easier to see what this is supposed to do.

aszlig · 2019-11-06T21:10:41Z

pkgs/applications/networking/browsers/chromium/common.nix

@@ -99,18 +100,22 @@ let
  versionRange = min-version: upto-version:
    let inherit (upstream-info) version;
        result = versionAtLeast version min-version && versionOlder version upto-version;
-        stable-version = (import ./upstream-info.nix).stable.version;
+        stable-version = with builtins;
+          (fromJSON (readFile ./upstream-info.json)).stable.version;


I'd suggest using lib.importJSON here.

aszlig · 2019-11-06T21:11:28Z

pkgs/applications/networking/browsers/chromium/default.nix

@@ -31,10 +31,11 @@ in let
  chromium = {
    inherit stdenv llvmPackages;

-    upstream-info = (callPackage ./update.nix {}).getChannel channel;
+    upstream-info = with builtins;
+      (fromJSON (readFile ./upstream-info.json)).${channel};


lib.importJSON again.

aszlig · 2019-11-06T21:35:13Z

@alyssais: Note also the eval error from ofborg.

stale · 2020-06-01T05:24:08Z

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the
related code and @ mention them in a comment.
Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
irc.freenode.net.

flokli · 2020-06-06T11:53:07Z

@alyssais any chance you could address @aszlig's comments? I just opened #89635, not seeing this PR already exists.

alyssais · 2020-08-22T16:51:11Z

@aszlig @flokli I think I’ve addressed everything now? It’s been a while since I last looked at this, though, so I’d appreciate some more eyes.

flokli · 2020-08-27T08:44:26Z

This needs another rebase, and I'd propose making update.py executable. Apart from that, LGTM! 🚀

alyssais · 2020-08-27T10:27:09Z

This needs another rebase,

Done. Our chromiumDev was out of date, so I added a separate commit bumping that with the old one before running the new one so it wasn’t just upgraded silently.

and I'd propose making update.py executable.

It already is?

update.nix was a huuuuge hack, abusing checksum collisions, etc., and was extremely difficult to read and maintain, especially because values from update.nix were also used in the derivations themselves! I've replaced this with an implementation in Python, which I chose for readability. Rather than generating Nix, I chose to generate JSON, since Python can do that in the standard library and Nix can read it. I also set update.py as an updateScript, so Chromium can now automatically be updated! Fixes: NixOS#89635

flokli · 2020-09-05T09:26:59Z

Sorry, forgot to take a look at this again. Did the rebase, and ran the updater again.

Kazimazi · 2020-09-16T00:02:00Z

What does chromiumDev 87 need to start in native wayland? It always uses xwayland.
Edit: might be this issue Linux builds no longer respects --ozone-platform=wayland

alyssais requested review from samueldr, aszlig and worldofpeace November 6, 2019 20:17

samueldr reviewed Nov 6, 2019

View reviewed changes

aszlig requested changes Nov 6, 2019

View reviewed changes

aszlig requested review from ivan and bendlas November 6, 2019 21:30

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020

primeos mentioned this pull request Jun 6, 2020

chromium: use a less frightening updater script #89635

Closed

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 6, 2020

alyssais force-pushed the chromium-update branch from 1a1005b to bd5ce41 Compare August 22, 2020 14:40

ofborg bot added the 2.status: merge conflict label Aug 22, 2020

alyssais force-pushed the chromium-update branch from bd5ce41 to b95bb47 Compare August 22, 2020 16:48

alyssais removed the 2.status: merge conflict label Aug 22, 2020

alyssais force-pushed the chromium-update branch from b95bb47 to 7303756 Compare August 22, 2020 16:50

alyssais force-pushed the chromium-update branch from 7303756 to a3c15b3 Compare August 22, 2020 18:41

ofborg bot requested a review from msteen August 22, 2020 21:21

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Aug 22, 2020

alyssais force-pushed the chromium-update branch from a3c15b3 to d441300 Compare August 23, 2020 09:27

alyssais force-pushed the chromium-update branch from d441300 to 85b60fd Compare August 27, 2020 10:17

ofborg bot added the 10.rebuild-linux: 1 label Aug 27, 2020

alyssais added 2 commits September 5, 2020 11:08

chromiumDev: 86.0.4238.0 -> 86.0.4240.8

5811b6c

flokli force-pushed the chromium-update branch from 85b60fd to de69b70 Compare September 5, 2020 09:21

flokli added 2 commits September 5, 2020 11:24

chromiumDev: 86.0.4240.8 -> 87.0.4252.0

5da6656

chromiumBeta: 85.0.4183.83 -> 86.0.4240.22

6c92847

flokli merged commit 1fbc28c into NixOS:master Sep 5, 2020

flokli mentioned this pull request Sep 5, 2020

google-chrome: add libxkbcommon+wayland for ozone/wayland #97201

Merged

10 tasks

alyssais deleted the chromium-update branch April 9, 2021 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chromium: replace update.nix with Python impl #72932

chromium: replace update.nix with Python impl #72932

alyssais commented Nov 6, 2019 •

edited

samueldr left a comment

aszlig left a comment

aszlig Nov 6, 2019

aszlig Nov 6, 2019

aszlig Nov 6, 2019

flokli Jun 6, 2020

aszlig Jun 6, 2020

flokli Jun 6, 2020

aszlig Nov 6, 2019

aszlig Nov 6, 2019

aszlig Nov 6, 2019

aszlig Nov 6, 2019

aszlig commented Nov 6, 2019

stale bot commented Jun 1, 2020

flokli commented Jun 6, 2020

alyssais commented Aug 22, 2020

flokli commented Aug 27, 2020

alyssais commented Aug 27, 2020

flokli commented Sep 5, 2020

Kazimazi commented Sep 16, 2020 •

edited

	BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official";
	BUCKET_URL = "https://commondatastorage.googleapis.com/chromium-browser-official"

os	channel	current_version	previous_version	current_reldate	previous_reldate	branch_base_commit	branch_base_position	branch_commit	true_branch	v8_version
linux	dev	80.0.3955.4	79.0.3945.16	11/01/19	10/29/19	1be17a97af38b81c6b4bd01cd698e8329f62857b	710886	2fa66c8c902846e47cab48edf8aea6c8b4e3a2d8	3955	8.0.113
linux	beta	79.0.3945.16	78.0.3904.85	10/31/19	10/30/19	e4635fff7defbae0f9c29e798349f6fc0cce4b1b	706915	66fad076c1051f8d10555c8fe1de36f6a8effc69	3945	7.9.317.14
linux	stable	78.0.3904.97	78.0.3904.87	11/06/19	10/31/19	675968a8c657a3bd9c1c2c20c5d2935577bbc5e6	693954	2fa4ee555b8b5b382d68e1118ab7e6eac293a039	3904	7.8.279.23

chromium: replace update.nix with Python impl #72932

chromium: replace update.nix with Python impl #72932

Conversation

alyssais commented Nov 6, 2019 • edited

samueldr left a comment

Choose a reason for hiding this comment

aszlig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aszlig commented Nov 6, 2019

stale bot commented Jun 1, 2020

flokli commented Jun 6, 2020

alyssais commented Aug 22, 2020

flokli commented Aug 27, 2020

alyssais commented Aug 27, 2020

flokli commented Sep 5, 2020

Kazimazi commented Sep 16, 2020 • edited

alyssais commented Nov 6, 2019 •

edited

Kazimazi commented Sep 16, 2020 •

edited