gn: modernize; add marcin-serwin and emilylange to maintainers; 0-unstable-2025-04-28 -> 0-unstable-2025-06-19 by marcin-serwin · Pull Request #419337 · NixOS/nixpkgs

marcin-serwin · 2025-06-23T18:19:02Z

Things done

Add a 👍 reaction to pull requests you find important.

emilylange

Sorry for taking so long to review. I was busy with other stuff and this wasn't particularly high on my priority list.

pkgs/by-name/gn/gn/package.nix

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

pkgs/by-name/gn/gn/package.nix

emilylange · 2025-07-24T15:30:10Z

pkgs/by-name/gn/gn/package.nix

This is one of the cases where one might want a postFetch in fetchgit that runs the git describe --match initial-commit once and stores it as part of a file.

It would at least make this value less error-prone and accurate.
But it would also make it harder for this to be used in conditionals like done in a4a1a0c.
Though we could and probably should use version for those conditionals anyway. I should just to make my usage of gn.overrideAttrs for chromium and electron-source less cursed.

What do you think? Do you prefer this as is, or would you be fine with having that as part of the src FOD which can then be cated in the actual derivation?

This was my first instinct as well but I changed my mind when I noticed that it would also impact chromium and electron. I would prefer it to be part of the src but this would require duplicating the postFetch script in pkgs/applications/networking/browsers/chromium/update.mjs and pkgs/development/tools/electron/update.py. I can do it but keeping it in sync in 3 places seems to be error prone which is why I opted to do it this way.

What do you think? Is there a way to avoid the duplication that I'm missing? Is the more accurate rev worth the effort of maintaining duplicated code?

We don't need to duplicate the postFetch script in chromium and electron-source, we can just call nix-build with gn.override { } from there.
It still requires duplicating some logic, but not the actual script. And at least for chromium's update.mjs, rewriting the gn fetch logic has been in my To-dos for a while.

Came up with the following proof of concept patch, but we should sync up with the electron maintainers for this and look at how we want to roll this out (gn needs to go through staging, but there will be chromium and electron-source updates on master before the staging cycle lands on master).

But then again, we may want to defer this to another PR.

--- a/pkgs/applications/networking/browsers/chromium/default.nix +++ b/pkgs/applications/networking/browsers/chromium/default.nix @@ -79,14 +79,7 @@ let pulseSupport ungoogled ; - gnChromium = buildPackages.gn.overrideAttrs (oldAttrs: { - version = if (upstream-info.deps.gn ? "version") then upstream-info.deps.gn.version else "0"; - revNum = 9999; # override to prevent gn updates from causing chromium rebuilds - src = fetchgit { - url = "https://gn.googlesource.com/gn"; - inherit (upstream-info.deps.gn) rev hash; - }; - }); + gnChromium = buildPackages.gn.override upstream-info.deps.gn; }); browser = callPackage ./browser.nix { diff --git a/pkgs/applications/networking/browsers/chromium/info.json b/pkgs/applications/networking/browsers/chromium/info.json index a41fc3d01dd6..be19209585e6 100644 --- a/pkgs/applications/networking/browsers/chromium/info.json +++ b/pkgs/applications/networking/browsers/chromium/info.json @@ -12,6 +12,7 @@ "hash": "sha256-1avxBlK0WLHTru5wUecbiGpSEYv8Epobsl4EfCaWX9A=" }, "gn": { + "version": "0-unstable-2025-05-19", "rev": "ebc8f16ca7b0d36a3e532ee90896f9eb48e5423b", "hash": "sha256-UB9a7Fr1W0yYld6WbXyRR8dFqWsj/zx4KumDZ5JQKSM=" }, @@ -809,6 +810,7 @@ "hash": "sha256-1avxBlK0WLHTru5wUecbiGpSEYv8Epobsl4EfCaWX9A=" }, "gn": { + "version": "0-unstable-2025-05-19", "rev": "ebc8f16ca7b0d36a3e532ee90896f9eb48e5423b", "hash": "sha256-UB9a7Fr1W0yYld6WbXyRR8dFqWsj/zx4KumDZ5JQKSM=" }, diff --git a/pkgs/applications/networking/browsers/chromium/update.mjs b/pkgs/applications/networking/browsers/chromium/update.mjs index 42483eb06b40..7cbe2f6b9180 100755 --- a/pkgs/applications/networking/browsers/chromium/update.mjs +++ b/pkgs/applications/networking/browsers/chromium/update.mjs @@ -62,7 +62,7 @@ for (const attr_path of Object.keys(lockfile)) { chromedriver: !ungoogled ? await fetch_chromedriver_binaries(await get_latest_chromium_release('mac')) : undefined, deps: { depot_tools: {}, - gn: {}, + gn: await fetch_gn(chromium_rev, lockfile_initial[attr_path].deps.gn), 'ungoogled-patches': !ungoogled ? undefined : { rev: ungoogled_patches.rev, hash: ungoogled_patches.hash, @@ -78,12 +78,6 @@ for (const attr_path of Object.keys(lockfile)) { hash: depot_tools.hash, } - const gn = await fetch_gn(chromium_rev, lockfile_initial[attr_path].deps.gn) - lockfile[attr_path].deps.gn = { - rev: gn.rev, - hash: gn.hash, - } - // DEPS update loop lockfile[attr_path].DEPS = await resolve_DEPS(depot_tools.out, chromium_rev) for (const [path, value] of Object.entries(lockfile[attr_path].DEPS)) { @@ -133,10 +127,34 @@ for (const attr_path of Object.keys(lockfile)) { async function fetch_gn(chromium_rev, gn_previous) { const DEPS_file = await get_gitiles_file('https://chromium.googlesource.com/chromium/src', chromium_rev, 'DEPS') - const gn_rev = /^\s+'gn_version': 'git_revision:(?<rev>.+)',$/m.exec(DEPS_file).groups.rev - const hash = gn_rev === gn_previous.rev ? gn_previous.hash : '' + const { rev } = /^\s+'gn_version': 'git_revision:(?<rev>.+)',$/m.exec(DEPS_file).groups + + const cache_hit = rev === gn_previous.rev; + if (cache_hit) { + return gn_previous + } + + const commit_date = await get_gitiles_commit_date('https://gn.googlesource.com/gn', rev) + const version = `0-unstable-${commit_date}` + + const expr = [`(import ./. {}).gn.override { version = "${version}"; rev = "${rev}"; hash = ""; }`] + const derivation = await $nixpkgs`nix-instantiate --expr ${expr}` + + return { + version, + rev, + hash: await prefetch_FOD(derivation), + } +} + + +async function get_gitiles_commit_date(base_url, rev) { + const url = `${base_url}/+/${rev}?format=json` + const response = await (await fetch(url)).text() + const json = JSON.parse(response.replace(`)]}'\n`, '')) - return await prefetch_gitiles('https://gn.googlesource.com/gn', gn_rev, hash) + const date = new Date(json.author.time) + return date.toISOString().split("T")[0] } diff --git a/pkgs/by-name/gn/gn/package.nix b/pkgs/by-name/gn/gn/package.nix index 73fd70be615a..36b07843ab7c 100644 --- a/pkgs/by-name/gn/gn/package.nix +++ b/pkgs/by-name/gn/gn/package.nix @@ -5,21 +5,24 @@ cctools, ninja, python3, + + # Note: Please use the recommended version for Chromium stable, i.e. from + # <nixpkgs>/pkgs/applications/networking/browsers/chromium/info.json + version ? "0-unstable-2025-05-21", + rev ? "ebc8f16ca7b0d36a3e532ee90896f9eb48e5423b", + hash ? "sha256-UB9a7Fr1W0yYld6WbXyRR8dFqWsj/zx4KumDZ5JQKSM=", }: stdenv.mkDerivation (finalAttrs: { pname = "gn"; - version = "0-unstable-2025-05-21"; + inherit version; revNum = 2237; revShort = builtins.substring 0 7 finalAttrs.src.rev; src = fetchgit { # Note: The TAR-Archives (+archive/${rev}.tar.gz) are not deterministic! url = "https://gn.googlesource.com/gn"; - # Note: Please use the recommended version for Chromium stable, i.e. from - # <nixpkgs>/pkgs/applications/networking/browsers/chromium/info.json - rev = "ebc8f16ca7b0d36a3e532ee90896f9eb48e5423b"; - hash = "sha256-UB9a7Fr1W0yYld6WbXyRR8dFqWsj/zx4KumDZ5JQKSM="; + inherit rev hash; }; nativeBuildInputs = [ diff --git a/pkgs/development/tools/electron/info.json b/pkgs/development/tools/electron/info.json index bccecd057e11..67344559ec88 100644 --- a/pkgs/development/tools/electron/info.json +++ b/pkgs/development/tools/electron/info.json @@ -6,8 +6,7 @@ "gn": { "hash": "sha256-EqbwCLkseND1v3UqM+49N7GuoXJ3PlJjWOes4OijQ3U=", "rev": "ed1abc107815210dc66ec439542bee2f6cbabc00", - "url": "https://gn.googlesource.com/gn", - "version": "2025-01-13" + "version": "0-unstable-2025-01-13" } }, "version": "134.0.6998.205" @@ -1315,8 +1314,7 @@ "gn": { "hash": "sha256-vDKMt23RMDI+KX6CmjfeOhRv2haf/mDOuHpWKnlODcg=", "rev": "6e8e0d6d4a151ab2ed9b4a35366e630c55888444", - "url": "https://gn.googlesource.com/gn", - "version": "2025-03-24" + "version": "0-unstable-2025-03-24" } }, "version": "136.0.7103.177" @@ -2640,8 +2638,7 @@ "gn": { "hash": "sha256-UB9a7Fr1W0yYld6WbXyRR8dFqWsj/zx4KumDZ5JQKSM=", "rev": "ebc8f16ca7b0d36a3e532ee90896f9eb48e5423b", - "url": "https://gn.googlesource.com/gn", - "version": "2025-05-21" + "version": "0-unstable-2025-05-21" } }, "version": "138.0.7204.100" diff --git a/pkgs/development/tools/electron/update.py b/pkgs/development/tools/electron/update.py index 0303cb1c682b..e0f1e2fd278b 100755 --- a/pkgs/development/tools/electron/update.py +++ b/pkgs/development/tools/electron/update.py @@ -1,5 +1,5 @@ #! /usr/bin/env nix-shell -#! nix-shell -i python -p python3.pkgs.joblib python3.pkgs.click python3.pkgs.click-log nix nix-prefetch-git prefetch-yarn-deps prefetch-npm-deps gclient2nix +#! nix-shell -i python -p python3.pkgs.joblib python3.pkgs.click python3.pkgs.click-log nix nurl prefetch-yarn-deps prefetch-npm-deps gclient2nix """ electron updater @@ -32,7 +32,7 @@ import urllib.request import click import click_log -from datetime import datetime +from datetime import datetime, UTC from typing import Iterable, Tuple from urllib.request import urlopen from joblib import Parallel, delayed, Memory @@ -44,6 +44,9 @@ SOURCE_INFO_JSON = "info.json" os.chdir(os.path.dirname(__file__)) +# Absolute path of nixpkgs top-level directory +NIXPKGS_PATH = subprocess.check_output(["git", "rev-parse", "--show-toplevel"]).decode("utf-8").strip() + memory: Memory = Memory("cache", verbose=0) logger = logging.getLogger(__name__) @@ -78,26 +81,34 @@ def get_electron_file(electron_tag: str, filepath: str) -> str: ) +@memory.cache +def get_gn_hash(gn_version, gn_commit): + print("gn.override", file=sys.stderr) + expr = f'(import {NIXPKGS_PATH} {{}}).gn.override {{ version = "{gn_version}"; rev = "{gn_commit}"; hash = ""; }}' + out = subprocess.check_output(["nurl", "--hash", "--expr", expr]) + return out.decode("utf-8").strip() + @memory.cache def get_chromium_gn_source(chromium_tag: str) -> dict: gn_pattern = r"'gn_version': 'git_revision:([0-9a-f]{40})'" gn_commit = re.search(gn_pattern, get_chromium_file(chromium_tag, "DEPS")).group(1) - gn_prefetch: bytes = subprocess.check_output( - [ - "nix-prefetch-git", - "--quiet", - "https://gn.googlesource.com/gn", - "--rev", - gn_commit, - ] + + gn_commit_info = json.loads( + urlopen(f"https://gn.googlesource.com/gn/+/{gn_commit}?format=json") + .read() + .decode("utf-8") + .split(")]}'\n")[1] ) - gn: dict = json.loads(gn_prefetch) + + gn_commit_date = datetime.strptime(gn_commit_info["author"]["time"], "%a %b %d %H:%M:%S %Y %z") + gn_date = gn_commit_date.astimezone(UTC).date().isoformat() + gn_version = f"0-unstable-{gn_date}" + return { "gn": { - "version": datetime.fromisoformat(gn["date"]).date().isoformat(), - "url": gn["url"], - "rev": gn["rev"], - "hash": gn["hash"], + "version": gn_version, + "rev": gn_commit, + "hash": get_gn_hash(gn_version, gn_commit), } }

I've modified the patch to use commiter time instead of author. Author time is not monotonic and taking commiter time is consistent with unstableGitUpdater.

Still not a fan of duplicating the update logic in three places. What do you think about making the pkgs/by-name/gn/gn/update.sh script more generic and shelling out to it inside chromium and electron updaters for getting the hash and version?

cc electron maintainers: @yayayayaka @teutat3s @TomaSajt

pkgs/by-name/gn/gn/package.nix

emilylange

I intend to merge this fairly soon after this review has been addressed.

We can further iterate and improve on this at a later date. I think the important bits are mostly solid now. The scope of this PR is already slightly out of hand.

pkgs/by-name/gn/gn/package.nix

pkgs/by-name/gn/gn/update.sh

pkgs/by-name/gn/gn/package.nix

pkgs/by-name/gn/gn/update.sh

pkgs/applications/networking/browsers/chromium/info.json

Signed-off-by: Marcin Serwin <[email protected]>

emilylange

A bunch of hash mismatches due to the rev length change. Other than that, LGTM, thanks!

pkgs/development/tools/electron/info.json

pkgs/by-name/gn/gn/package.nix

Signed-off-by: Marcin Serwin <[email protected]>

Co-authored-by: emilylange <[email protected]> Signed-off-by: Marcin Serwin <[email protected]>

Signed-off-by: Marcin Serwin <[email protected]>

emilylange

Awesome, LGTM now, thanks :)

marcin-serwin · 2025-08-09T17:17:07Z

Thanks for the thorough review.

TomaSajt · 2025-08-09T17:34:20Z

Will this be backported?

marcin-serwin · 2025-08-09T17:49:19Z

We didn't backport #403859 so I guess we still want to avoid backporting updates. I think the modernization parts should be acceptable.

TomaSajt · 2025-08-09T18:04:40Z

The thing is: this changes electron stuff as well, and electron bump PRs do almost always get backported.

Maybe it would only cause issues when a new major electron version gets added, but still: it will be unfortunate if we have to maintain non-trivially different info.json files.

We can deal with it, if we need to, but I'd rather avoid the pain.

emilylange · 2025-08-09T18:46:27Z

We can backport most of this, but that would have to go through staging, in this case staging-25.05.
We likely still end up in a short period where internal APIs and hashes don't align.

Thankfully, the failure mode of this is a loud hard-error, so it will be obvious when this happens.

The only thing this affects are major version bumps, both chromium and electron-source.
Since electron follows every second chromium stable release, there will likely only be 1-2 new major electron-source in the remaining lifetime of release-25.05.

I can assure you, @TomaSajt, that this is on my radar and I won't deliberate break your update flow or make your work more painful than it needs to be, even if we have our differences.

I will keep an eye how quick this PR lands in master and will decide based on that. It's likely that gn in release-25.05 will stay as and gnChromium gets a few more lines to handle the different FODs.

That would also have the added benefit of not having to go through staging-25.05.

How about the following: I will ping you if there is some unexpected manual intervention needed and if you don't hear from me, then there won't.

TomaSajt · 2025-08-09T18:55:36Z

Alright, sure.
I was just mostly wondering if the backporting thing was considered already or not.
I won't block anything, just wanted to share my worries.

nix-owners bot requested review from emilylange, matthewbauer, networkException, primeos and stesie June 23, 2025 18:25

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 7, 2025

marcin-serwin force-pushed the push-zypxonqkutyk branch from 626cefc to 42f424a Compare July 24, 2025 13:31

marcin-serwin changed the title ~~gn: modernize; add myself to maintainers~~ gn: modernize; add marcin-serwin to maintainers; 0-unstable-2025-04-28 -> 0-unstable-2025-05-21 Jul 24, 2025

marcin-serwin removed the request for review from primeos July 24, 2025 13:31

nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025

emilylange requested changes Jul 24, 2025

View reviewed changes

marcin-serwin force-pushed the push-zypxonqkutyk branch from 42f424a to 9a3545f Compare July 27, 2025 09:38

marcin-serwin commented Jul 27, 2025

View reviewed changes

pkgs/by-name/gn/gn/package.nix Outdated Show resolved Hide resolved

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 6, 2025

emilylange requested changes Aug 6, 2025

View reviewed changes

gn: merge generic and package

b0986e8

Signed-off-by: Marcin Serwin <[email protected]>

marcin-serwin force-pushed the push-zypxonqkutyk branch from ecbc0e4 to d227593 Compare August 8, 2025 16:03

marcin-serwin requested a review from emilylange August 8, 2025 16:03

marcin-serwin changed the title ~~gn: modernize; add marcin-serwin to maintainers; 0-unstable-2025-04-28 -> 0-unstable-2025-05-21~~ gn: modernize; add marcin-serwin and emilylange to maintainers; 0-unstable-2025-04-28 -> 0-unstable-2025-06-19 Aug 8, 2025

nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 8, 2025

emilylange requested changes Aug 8, 2025

View reviewed changes

marcin-serwin added 5 commits August 9, 2025 08:52

gn: get revNum and revShort in src's postFetch

44f4405

Signed-off-by: Marcin Serwin <[email protected]>

gn: drop "-unstable" suffix from pname

ec54698

Signed-off-by: Marcin Serwin <[email protected]>

gn: run hooks in build phases

d66400c

Signed-off-by: Marcin Serwin <[email protected]>

gn: remove with lib; from meta

4f436a3

Signed-off-by: Marcin Serwin <[email protected]>

gn: add update script

a760bcc

Signed-off-by: Marcin Serwin <[email protected]>

marcin-serwin and others added 9 commits August 9, 2025 08:52

gn: add marcin-serwin to maintainers

d654f14

Signed-off-by: Marcin Serwin <[email protected]>

gn: make src version overridable

e2e3860

Co-authored-by: emilylange <[email protected]> Signed-off-by: Marcin Serwin <[email protected]>

chromium, electron: use override instead of overrideAttrs for gn

ab74600

Co-authored-by: emilylange <[email protected]> Signed-off-by: Marcin Serwin <[email protected]>

electron: adapt updater to the new gn package

e5a7616

Co-authored-by: emilylange <[email protected]> Signed-off-by: Marcin Serwin <[email protected]>

chromium: adapt updater to the new gn package

9b4cbad

Co-authored-by: emilylange <[email protected]> Signed-off-by: Marcin Serwin <[email protected]>

gn: 0-unstable-2025-04-28 -> 0-unstable-2025-06-19

b59bfde

Signed-off-by: Marcin Serwin <[email protected]>

gn: remove outdated comment

4e3a8af

Signed-off-by: Marcin Serwin <[email protected]>

gn: add emilylange to maintainers

1282ed5

Signed-off-by: Marcin Serwin <[email protected]>

gn: enable ninja verbose mode to show build progress

28cefc6

Signed-off-by: Marcin Serwin <[email protected]>

marcin-serwin force-pushed the push-zypxonqkutyk branch from d227593 to 28cefc6 Compare August 9, 2025 06:54

marcin-serwin requested a review from emilylange August 9, 2025 06:55

emilylange approved these changes Aug 9, 2025

View reviewed changes

nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Aug 9, 2025

K900 changed the base branch from staging to staging-next August 10, 2025 12:58

nixpkgs-ci bot closed this Aug 10, 2025

nixpkgs-ci bot reopened this Aug 10, 2025

K900 merged commit 556405f into NixOS:staging-next Aug 10, 2025
28 of 30 checks passed

emilylange mentioned this pull request Aug 11, 2025

[staging-next] python3Packages.home-assistant-chip-wheels: fix building with newer gn #432683

Merged

13 tasks

emilylange mentioned this pull request Sep 3, 2025

[Backport release-25.05] chromium,chromedriver: 139.0.7258.154 -> 140.0.7339.80 #440001

Merged

13 tasks

marcin-serwin deleted the push-zypxonqkutyk branch September 4, 2025 16:58

Uh oh!

Conversation

marcin-serwin commented Jun 23, 2025

Things done

Uh oh!

emilylange left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilylange Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcin-serwin Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

emilylange Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

marcin-serwin Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emilylange left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilylange left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilylange left a comment

Choose a reason for hiding this comment

Uh oh!

marcin-serwin commented Aug 9, 2025

Uh oh!

TomaSajt commented Aug 9, 2025

Uh oh!

marcin-serwin commented Aug 9, 2025

Uh oh!

TomaSajt commented Aug 9, 2025

Uh oh!

emilylange commented Aug 9, 2025

Uh oh!

TomaSajt commented Aug 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emilylange Jul 24, 2025 •

edited

Loading