| Message ID | 20240812122539.573337-2-enguerrand.de-ribaucourt@savoirfairelinux.com |
|---|---|
| State | New |
| Headers | show |
| Series | npm: improve fetcher and recipetool compatibility | expand |
On Mon, 2024-08-12 at 14:25 +0200, Enguerrand de Ribaucourt wrote: > The NPM package.json documentation[1] states that git URLs may contain > a commit-ish suffix to specify a specific revision. When running > `npm install`, this revision will be looked for on any branch of the > repository. > > The bitbake implementation however translates the URL stored in > package.json into a git URL to be fetch by the bitbake git fetcher. The > bitbake fetcher git.py, enforces the branch to be master by default. If > the revision specified in the package.json is not on the master branch, > the fetch will fail while the package.json is valid. > > To fix this, append the ";nobranch=1" suffix to the revision in the git > URL to be fetched. This will make the bitbake git fetcher ignore the > branch and respect the behavior of `npm install``. > > This can be tested with the following command: > $ devtool add --npm-dev https://github.com/seapath/cockpit-cluster-dashboard.git -B version > Which points to a project which has a package.json with a git URL: > ```json > "devDependencies": { > "cockpit-repo": "git+https://github.com/cockpit-project/cockpit.git#d34cabacb8e5e1e028c7eea3d6e3b606d862b8ac" > } > ``` > In this repo, the specified revision is on the "main" branch, which > would fail without this fix. > > [1] https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies > > Co-authored-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> > Signed-off-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> > Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> > --- > bitbake/lib/bb/fetch2/npmsw.py | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/bitbake/lib/bb/fetch2/npmsw.py b/bitbake/lib/bb/fetch2/npmsw.py > index b55e885d7b..d8ed9df327 100644 > --- a/bitbake/lib/bb/fetch2/npmsw.py > +++ b/bitbake/lib/bb/fetch2/npmsw.py > @@ -184,6 +184,7 @@ class NpmShrinkWrap(FetchMethod): > uri = URI("git://" + str(groups["url"])) > uri.params["protocol"] = str(groups["protocol"]) > uri.params["rev"] = str(groups["rev"]) > + uri.params["nobranch"] = "1" > uri.params["destsuffix"] = destsuffix > > url = str(uri) Is it possible to determine and add the branch name where needed? I'd prefer to try and keep the behaviours similar in more cases if we can and avoid disabling checks we added for good reasons... Cheers, Richard
On 15/08/2024 16:14, Richard Purdie wrote: > On Mon, 2024-08-12 at 14:25 +0200, Enguerrand de Ribaucourt wrote: >> The NPM package.json documentation[1] states that git URLs may contain >> a commit-ish suffix to specify a specific revision. When running >> `npm install`, this revision will be looked for on any branch of the >> repository. >> >> The bitbake implementation however translates the URL stored in >> package.json into a git URL to be fetch by the bitbake git fetcher. The >> bitbake fetcher git.py, enforces the branch to be master by default. If >> the revision specified in the package.json is not on the master branch, >> the fetch will fail while the package.json is valid. >> >> To fix this, append the ";nobranch=1" suffix to the revision in the git >> URL to be fetched. This will make the bitbake git fetcher ignore the >> branch and respect the behavior of `npm install``. >> >> This can be tested with the following command: >> $ devtool add --npm-dev https://github.com/seapath/cockpit-cluster-dashboard.git -B version >> Which points to a project which has a package.json with a git URL: >> ```json >> "devDependencies": { >> "cockpit-repo": "git+https://github.com/cockpit-project/cockpit.git#d34cabacb8e5e1e028c7eea3d6e3b606d862b8ac" >> } >> ``` >> In this repo, the specified revision is on the "main" branch, which >> would fail without this fix. >> >> [1] https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies >> >> Co-authored-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> >> Signed-off-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> >> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> >> --- >> bitbake/lib/bb/fetch2/npmsw.py | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/bitbake/lib/bb/fetch2/npmsw.py b/bitbake/lib/bb/fetch2/npmsw.py >> index b55e885d7b..d8ed9df327 100644 >> --- a/bitbake/lib/bb/fetch2/npmsw.py >> +++ b/bitbake/lib/bb/fetch2/npmsw.py >> @@ -184,6 +184,7 @@ class NpmShrinkWrap(FetchMethod): >> uri = URI("git://" + str(groups["url"])) >> uri.params["protocol"] = str(groups["protocol"]) >> uri.params["rev"] = str(groups["rev"]) >> + uri.params["nobranch"] = "1" >> uri.params["destsuffix"] = destsuffix >> >> url = str(uri) > > Is it possible to determine and add the branch name where needed? > > I'd prefer to try and keep the behaviours similar in more cases if we > can and avoid disabling checks we added for good reasons... The current npm.bbclass implementation is actually trying to reproduce the behavior of the real `npm install` tool that reads package.json/npm-shrinkwrap.json and installs all dependencies. The package.json specification has a specific format which is frequently used like above to explicit commit hashes to fetch but it doesn't allow to specify the branch [1]. This form is usually written for dependencies which do not declare a version like cockpit-repo here (discussed all throughout this patch list). So to behave similarly to the `npm install` tool, we should not specify a branch. We would actually need to edit package.json into a diverging format when we want to be compatible with as many package.json projects as possible. The alternative would be to pre-fetch to see which branch we want, then pass that in the FetchMethod query here. But we would just displace the problem? At its base, package.json git references cannot specify a branch. Thank you very much for your review, [1] https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies > > Cheers, > > Richard >
diff --git a/bitbake/lib/bb/fetch2/npmsw.py b/bitbake/lib/bb/fetch2/npmsw.py index b55e885d7b..d8ed9df327 100644 --- a/bitbake/lib/bb/fetch2/npmsw.py +++ b/bitbake/lib/bb/fetch2/npmsw.py @@ -184,6 +184,7 @@ class NpmShrinkWrap(FetchMethod): uri = URI("git://" + str(groups["url"])) uri.params["protocol"] = str(groups["protocol"]) uri.params["rev"] = str(groups["rev"]) + uri.params["nobranch"] = "1" uri.params["destsuffix"] = destsuffix url = str(uri)
The NPM package.json documentation[1] states that git URLs may contain a commit-ish suffix to specify a specific revision. When running `npm install`, this revision will be looked for on any branch of the repository. The bitbake implementation however translates the URL stored in package.json into a git URL to be fetch by the bitbake git fetcher. The bitbake fetcher git.py, enforces the branch to be master by default. If the revision specified in the package.json is not on the master branch, the fetch will fail while the package.json is valid. To fix this, append the ";nobranch=1" suffix to the revision in the git URL to be fetched. This will make the bitbake git fetcher ignore the branch and respect the behavior of `npm install``. This can be tested with the following command: $ devtool add --npm-dev https://github.com/seapath/cockpit-cluster-dashboard.git -B version Which points to a project which has a package.json with a git URL: ```json "devDependencies": { "cockpit-repo": "git+https://github.com/cockpit-project/cockpit.git#d34cabacb8e5e1e028c7eea3d6e3b606d862b8ac" } ``` In this repo, the specified revision is on the "main" branch, which would fail without this fix. [1] https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies Co-authored-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> Signed-off-by: Tanguy Raufflet <tanguy.raufflet@savoirfairelinux.com> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> --- bitbake/lib/bb/fetch2/npmsw.py | 1 + 1 file changed, 1 insertion(+)