~andir/nixpkgs-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH] fetchYarnDeps: fix broken fetching logic for github releases

Adam Joseph <adam@westernsemico.com>
Details
Message ID
<20240724220111.19563-2-adam@westernsemico.com>
DKIM signature
pass
Download raw message
a month ago
Patch: +16 -3 
When a dependency references a github *release* URL, that dependency must be
fetched using https rather than git, since github does not require that
release tarballs have any relationship whatsoever to the git history.

This commit causes them to be fetched using https, not git.

A test case (which fails prior to this commit, and passes afterwards) is included.
---
 pkgs/build-support/node/fetch-yarn-deps/index.js         | 9 ++++++---
 .../build-support/node/fetch-yarn-deps/tests/default.nix | 4 ++++
 .../node/fetch-yarn-deps/tests/github-release.lock       | 6 ++++++
 3 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock

diff --git a/pkgs/build-support/node/fetch-yarn-deps/index.js b/pkgs/build-support/node/fetch-yarn-deps/index.js
index 91e3d1014661..400b64d2e920 100755
--- a/pkgs/build-support/node/fetch-yarn-deps/index.js
+++ b/pkgs/build-support/node/fetch-yarn-deps/index.js
@@ -104,11 +104,14 @@ const downloadPkg = (pkg, verbose) => {
	const [ url, hash ] = pkg.resolved.split('#')
	if (verbose) console.log('downloading ' + url)
	const fileName = urlToName(url)
	const s = url.split('/')
	if (url.startsWith('https://codeload.github.com/') && url.includes('/tar.gz/')) {
		const s = url.split('/')
		return downloadGit(fileName, `https://github.com/${s[3]}/${s[4]}.git`, s[s.length-1])
	} else if (url.startsWith('https://github.com/') && url.endsWith('.tar.gz')) {
		const s = url.split('/')
	} else if (url.startsWith('https://github.com/') && url.endsWith('.tar.gz') &&
		(
			s.length <= 5 ||    // https://github.com/owner/repo.tgz#feedface...
			s[5] == "archive"   // https://github.com/owner/repo/archive/refs/tags/v0.220.1.tar.gz
		)) {
		return downloadGit(fileName, `https://github.com/${s[3]}/${s[4]}.git`, s[s.length-1].replace(/.tar.gz$/, ''))
	} else if (isGitUrl(url)) {
		return downloadGit(fileName, url.replace(/^git\+/, ''), hash)
diff --git a/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix b/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
index 8057d05ba72c..2d563dac87dc 100644
--- a/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
+++ b/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
@@ -17,6 +17,10 @@
    yarnLock = ./github.lock;
    sha256 = "sha256-DIKrhDKoqm7tHZmcuh9eK9VTqp6BxeW0zqDUpY4F57A=";
  };
  githubReleaseDep = testers.invalidateFetcherByDrvHash fetchYarnDeps {
    yarnLock = ./github-release.lock;
    sha256 = "sha256-g+y/H6k8LZ+IjWvkkwV7JhKQH1ycfeqzsIonNv4fDq8=";
  };
  gitUrlDep = testers.invalidateFetcherByDrvHash fetchYarnDeps {
    yarnLock = ./giturl.lock;
    sha256 = "sha256-VPnyqN6lePQZGXwR7VhbFnP7/0/LB621RZwT1F+KzVQ=";
diff --git a/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock b/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock
new file mode 100644
index 000000000000..267a2510067d
--- /dev/null
+++ b/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock
@@ -0,0 +1,6 @@
"libsession_util_nodejs@https://github.com/oxen-io/libsession-util-nodejs/releases/download/v0.3.19/libsession_util_nodejs-v0.3.19.tar.gz":
  version "0.3.19"
  resolved "https://github.com/oxen-io/libsession-util-nodejs/releases/download/v0.3.19/libsession_util_nodejs-v0.3.19.tar.gz#221c1fc34fcc18601aea4ce1b733ebfa55af66ea"
  dependencies:
    cmake-js "^7.2.1"
    node-addon-api "^6.1.0"
-- 
2.44.0
Florian Klink <flokli@flokli.de>
Details
Message ID
<jyzgbu4mz7mqnqwcyo2mr4eehhyu2kw7hibxhgvpblaxsczqzz@hfm2msyh5tht>
In-Reply-To
<20240724220111.19563-2-adam@westernsemico.com> (view parent)
DKIM signature
pass
Download raw message
a month ago 
https://github.com/NixOS/nixpkgs/pull/329863

On Wed, Jul 24, 2024 at 03:01:12PM GMT, Adam Joseph wrote:
>When a dependency references a github *release* URL, that dependency must be
>fetched using https rather than git, since github does not require that
>release tarballs have any relationship whatsoever to the git history.
>
>This commit causes them to be fetched using https, not git.
>
>A test case (which fails prior to this commit, and passes afterwards) is included.
>---
> pkgs/build-support/node/fetch-yarn-deps/index.js         | 9 ++++++---
> .../build-support/node/fetch-yarn-deps/tests/default.nix | 4 ++++
> .../node/fetch-yarn-deps/tests/github-release.lock       | 6 ++++++
> 3 files changed, 16 insertions(+), 3 deletions(-)
> create mode 100644 pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock
>
>diff --git a/pkgs/build-support/node/fetch-yarn-deps/index.js b/pkgs/build-support/node/fetch-yarn-deps/index.js
>index 91e3d1014661..400b64d2e920 100755
>--- a/pkgs/build-support/node/fetch-yarn-deps/index.js
>+++ b/pkgs/build-support/node/fetch-yarn-deps/index.js
>@@ -104,11 +104,14 @@ const downloadPkg = (pkg, verbose) => {
> 	const [ url, hash ] = pkg.resolved.split('#')
> 	if (verbose) console.log('downloading ' + url)
> 	const fileName = urlToName(url)
>+	const s = url.split('/')
> 	if (url.startsWith('https://codeload.github.com/') && url.includes('/tar.gz/')) {
>-		const s = url.split('/')
> 		return downloadGit(fileName, `https://github.com/${s[3]}/${s[4]}.git`, s[s.length-1])
>-	} else if (url.startsWith('https://github.com/') && url.endsWith('.tar.gz')) {
>-		const s = url.split('/')
>+	} else if (url.startsWith('https://github.com/') && url.endsWith('.tar.gz') &&
>+		(
>+			s.length <= 5 ||    // https://github.com/owner/repo.tgz#feedface...
>+			s[5] == "archive"   // https://github.com/owner/repo/archive/refs/tags/v0.220.1.tar.gz
>+		)) {
> 		return downloadGit(fileName, `https://github.com/${s[3]}/${s[4]}.git`, s[s.length-1].replace(/.tar.gz$/, ''))
> 	} else if (isGitUrl(url)) {
> 		return downloadGit(fileName, url.replace(/^git\+/, ''), hash)
>diff --git a/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix b/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
>index 8057d05ba72c..2d563dac87dc 100644
>--- a/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
>+++ b/pkgs/build-support/node/fetch-yarn-deps/tests/default.nix
>@@ -17,6 +17,10 @@
>     yarnLock = ./github.lock;
>     sha256 = "sha256-DIKrhDKoqm7tHZmcuh9eK9VTqp6BxeW0zqDUpY4F57A=";
>   };
>+  githubReleaseDep = testers.invalidateFetcherByDrvHash fetchYarnDeps {
>+    yarnLock = ./github-release.lock;
>+    sha256 = "sha256-g+y/H6k8LZ+IjWvkkwV7JhKQH1ycfeqzsIonNv4fDq8=";
>+  };
>   gitUrlDep = testers.invalidateFetcherByDrvHash fetchYarnDeps {
>     yarnLock = ./giturl.lock;
>     sha256 = "sha256-VPnyqN6lePQZGXwR7VhbFnP7/0/LB621RZwT1F+KzVQ=";
>diff --git a/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock b/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock
>new file mode 100644
>index 000000000000..267a2510067d
>--- /dev/null
>+++ b/pkgs/build-support/node/fetch-yarn-deps/tests/github-release.lock
>@@ -0,0 +1,6 @@
>+"libsession_util_nodejs@https://github.com/oxen-io/libsession-util-nodejs/releases/download/v0.3.19/libsession_util_nodejs-v0.3.19.tar.gz":
>+  version "0.3.19"
>+  resolved "https://github.com/oxen-io/libsession-util-nodejs/releases/download/v0.3.19/libsession_util_nodejs-v0.3.19.tar.gz#221c1fc34fcc18601aea4ce1b733ebfa55af66ea"
>+  dependencies:
>+    cmake-js "^7.2.1"
>+    node-addon-api "^6.1.0"
>-- 
>2.44.0
>

-- 
Florian Klink
Adam Joseph <adam@westernsemico.com>
Details
Message ID
<172257886406.11925.5259480545118045463@localhost>
In-Reply-To
<jyzgbu4mz7mqnqwcyo2mr4eehhyu2kw7hibxhgvpblaxsczqzz@hfm2msyh5tht> (view parent)
DKIM signature
pass
Download raw message
a month ago 
Quoting Florian Klink (2024-07-25 02:41:04)
> https://github.com/NixOS/nixpkgs/pull/329863

Thank you flokli!!

  - a
Reply to thread Export thread (mbox)