From 7e247e6fecdb539911ddc7fafdbcf6db18506b84 Mon Sep 17 00:00:00 2001 From: Winter Date: Mon, 12 Dec 2022 22:33:43 -0500 Subject: [PATCH] prefetch-npm-deps: download dev deps for git deps with install scripts Git dependencies with install scripts are built isolated from the main package, so their development dependencies are required. To take advantage of this, #206477 is needed. --- .../node/build-npm-package/default.nix | 5 +- .../node/fetch-npm-deps/default.nix | 11 +++- .../node/fetch-npm-deps/src/main.rs | 2 +- .../node/fetch-npm-deps/src/parse/mod.rs | 65 +++++++++++++++++-- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/pkgs/build-support/node/build-npm-package/default.nix b/pkgs/build-support/node/build-npm-package/default.nix index 26cc678c571e..1c3fb6a74efc 100644 --- a/pkgs/build-support/node/build-npm-package/default.nix +++ b/pkgs/build-support/node/build-npm-package/default.nix @@ -12,6 +12,9 @@ # The output hash of the dependencies for this project. # Can be calculated in advance with prefetch-npm-deps. , npmDepsHash ? "" + # Whether to force the usage of Git dependencies that have install scripts, but not a lockfile. + # Use with care. +, forceGitDeps ? false # Whether to make the cache writable prior to installing dependencies. # Don't set this unless npm tries to write to the cache directory, as it can slow down the build. , makeCacheWritable ? false @@ -32,7 +35,7 @@ let npmDeps = fetchNpmDeps { - inherit src srcs sourceRoot prePatch patches postPatch; + inherit forceGitDeps src srcs sourceRoot prePatch patches postPatch; name = "${name}-npm-deps"; hash = npmDepsHash; }; diff --git a/pkgs/build-support/node/fetch-npm-deps/default.nix b/pkgs/build-support/node/fetch-npm-deps/default.nix index d87071d8559f..41cad9d12ee6 100644 --- a/pkgs/build-support/node/fetch-npm-deps/default.nix +++ b/pkgs/build-support/node/fetch-npm-deps/default.nix @@ -36,8 +36,8 @@ ''; }; - makeTest = { name, src, hash }: testers.invalidateFetcherByDrvHash fetchNpmDeps { - inherit name hash; + makeTest = { name, src, hash, forceGitDeps ? false }: testers.invalidateFetcherByDrvHash fetchNpmDeps { + inherit name hash forceGitDeps; src = makeTestSrc { inherit name src; }; }; @@ -108,6 +108,8 @@ }; hash = "sha256-+KA8/orSBJ4EhuSyQO8IKSxsN/FAsYU3lOzq+awuxNQ="; + + forceGitDeps = true; }; }; @@ -121,6 +123,7 @@ fetchNpmDeps = { name ? "npm-deps" , hash ? "" + , forceGitDeps ? false , ... } @ args: let @@ -131,6 +134,8 @@ outputHash = ""; outputHashAlgo = "sha256"; }; + + forceGitDeps_ = lib.optionalAttrs forceGitDeps { FORCE_GIT_DEPS = true; }; in stdenvNoCC.mkDerivation (args // { inherit name; @@ -161,5 +166,5 @@ dontInstall = true; outputHashMode = "recursive"; - } // hash_); + } // hash_ // forceGitDeps_); } diff --git a/pkgs/build-support/node/fetch-npm-deps/src/main.rs b/pkgs/build-support/node/fetch-npm-deps/src/main.rs index 068a6d2bcd60..57725a922dfd 100644 --- a/pkgs/build-support/node/fetch-npm-deps/src/main.rs +++ b/pkgs/build-support/node/fetch-npm-deps/src/main.rs @@ -97,7 +97,7 @@ fn main() -> anyhow::Result<()> { (out_tempdir.path(), true) }; - let packages = parse::lockfile(&lock_content)?; + let packages = parse::lockfile(&lock_content, env::var("FORCE_GIT_DEPS").is_ok())?; let cache = Cache::new(out.join("_cacache")); diff --git a/pkgs/build-support/node/fetch-npm-deps/src/parse/mod.rs b/pkgs/build-support/node/fetch-npm-deps/src/parse/mod.rs index 95f876017bdd..9ed4bbecc215 100644 --- a/pkgs/build-support/node/fetch-npm-deps/src/parse/mod.rs +++ b/pkgs/build-support/node/fetch-npm-deps/src/parse/mod.rs @@ -1,6 +1,7 @@ use anyhow::{anyhow, bail, Context}; use lock::UrlOrString; use rayon::prelude::*; +use serde_json::{Map, Value}; use std::{ fs, io, process::{Command, Stdio}, @@ -10,17 +11,71 @@ use url::Url; mod lock; -pub fn lockfile(lockfile: &str) -> anyhow::Result> { - let packages = lock::packages(lockfile).context("failed to extract packages from lockfile")?; - - packages +pub fn lockfile(content: &str, force_git_deps: bool) -> anyhow::Result> { + let mut packages = lock::packages(content) + .context("failed to extract packages from lockfile")? .into_par_iter() .map(|p| { let n = p.name.clone().unwrap(); Package::from_lock(p).with_context(|| format!("failed to parse data for {n}")) }) - .collect() + .collect::>>()?; + + let mut new = Vec::new(); + + for pkg in packages + .iter() + .filter(|p| matches!(p.specifics, Specifics::Git { .. })) + { + let dir = match &pkg.specifics { + Specifics::Git { workdir } => workdir, + Specifics::Registry { .. } => unimplemented!(), + }; + + let path = dir.path().join("package"); + + let lockfile_contents = fs::read_to_string(path.join("package-lock.json")); + + let package_json_path = path.join("package.json"); + let mut package_json: Map = + serde_json::from_str(&fs::read_to_string(package_json_path)?)?; + + if let Some(scripts) = package_json + .get_mut("scripts") + .and_then(Value::as_object_mut) + { + // https://github.com/npm/pacote/blob/272edc1bac06991fc5f95d06342334bbacfbaa4b/lib/git.js#L166-L172 + for typ in [ + "postinstall", + "build", + "preinstall", + "install", + "prepack", + "prepare", + ] { + if scripts.contains_key(typ) && lockfile_contents.is_err() && !force_git_deps { + bail!("Git dependency {} contains install scripts, but has no lockfile, which is something that will probably break. Open an issue if you can't feasibly patch this dependency out, and we'll come up with a workaround.\nIf you'd like to attempt to try to use this dependency anyways, set `forceGitDeps = true`.", pkg.name); + } + } + } + + if let Ok(lockfile_contents) = lockfile_contents { + new.append(&mut lockfile(&lockfile_contents, force_git_deps)?); + } + } + + packages.append(&mut new); + + packages.par_sort_by(|x, y| { + x.url + .partial_cmp(&y.url) + .expect("resolved should be comparable") + }); + + packages.dedup_by(|x, y| x.url == y.url); + + Ok(packages) } #[derive(Debug)]