Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolved registry overrides #486

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Resolved registry overrides #486

wants to merge 4 commits into from

Conversation

everett1992
Copy link

@everett1992 everett1992 commented Nov 3, 2021

Create a mechanism that allows users to create package-locks that can be used with different custom registries.

link to rfc

@everett1992
Copy link
Author

We can take this off the agenda tho I'd appreciate implementation input from someone familiar with package-lock and arborist.

I think metaFromNode in lib/shrinkwrap is responsible for getting data to write to a lockfile.
https://github.com/npm/arborist/blob/main/lib/shrinkwrap.js#L301-L304

How do I tell if a node was resolved from a registry or a literal http dependency?

everett1992 pushed a commit to everett1992/arborist that referenced this pull request Nov 16, 2021
Start npm/rfcs#486. This implements `$disable-write-resolves` without
creating an option. Next I'll figure out how to plumb a npm config
option thru to shrinkwrap.
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Nov 24, 2021
everett1992 pushed a commit to everett1992/arborist that referenced this pull request Dec 16, 2021
Start npm/rfcs#486. This implements `$disable-write-resolves` without
creating an option. Next I'll figure out how to plumb a npm config
option thru to shrinkwrap.
everett1992 pushed a commit to everett1992/arborist that referenced this pull request Jan 3, 2022
Implement `$disable-write-resolves` described in npm/rfcs#486.  I named
the option `omitLockfileRegistryResolved` but that can be changed later.

Put simply, this option causes npm to create lock files without a
`resolved` key for registry dependencies forcing npm to use the current
configured registry and resolve package tarball urls on install. This
fixes install errors when users change registries and the recorded
resolved url is incorrect.

This option causes slower installs because npm must fetch each packages
manifest to find the tarball url, but it's the most comprehensive
solution to this problem. Options like recording always the default
registry, or recording a special 'current registry' sigil will break if
registries host tarballs at different paths. For example
`${REGISTRY}/npm/-/npm-8.3.0.tgz` only works if all registries host
tarballs at `npm/-/npm-8.3.0.tgz`.
@everett1992
Copy link
Author

I've drafted this RFC in npm/arborist#352. Has there been more discussion including registry in lockfile? I think omitting resolved is the most comprehensive option because it supports switching between registries that host tarballs at different paths. But that makes it also more expensive than other options because npm must re-resolve the tarball url from package manifest. So I'm considering alternatives

Maybe npm could guess that a dependency without resolved is hosted at <registry>/<name>/-/<name>-<version>.tgz, tho that would be even slower when tarballs are hosted at a different path.

Maybe we implement this RFC and an option that supports switching between registries that host tarballs at the same path without paying the resolve cost. @isaacs mentioned wanting to use a sigil to mean 'the current registry', such as resolved: ${registry}/npm/-/npm-8.1.0.tgz but had issues implementing that without requiring a new lockfile version or affecting other npm clients. But we already have a sigil that means 'the current registry' - https://registry.npmjs.org! We could record registry.npmjs.org as the resolved url even when using a different registry. Then when npm reads the package-lock it will replace registry.npmjs.org with the current registry.

We need to replace the registry part of the resolved url with https://registry.npmjs.org but arborist doesn't record the registries it used. Arborist knows the resolved url of a node, but not where the registry ends and the path begins.

http://private.npmjs.org/a/b/c.tgz could be split more than one way

registry path recorded
http://private.npmjs.org/a/b c.tgz https://registry.npmjs.org/c.tgz
http://private.npmjs.org/a a/b/c.tgz https://registry.npmjs.org/a/b/c.tgz

We could work backward from the package name and registry config. We need the name because scopes can use different registries. Node#name may be the folder name, an alias or won't include the scope. Node#pkg.name is not guaranteed to be set. I think we'd have to use edgesIn specs or read package.json.

fritzy pushed a commit to npm/cli that referenced this pull request Jan 19, 2022
Implement `$disable-write-resolves` described in npm/rfcs#486.  I named
the option `omitLockfileRegistryResolved` but that can be changed later.

Put simply, this option causes npm to create lock files without a
`resolved` key for registry dependencies forcing npm to use the current
configured registry and resolve package tarball urls on install. This
fixes install errors when users change registries and the recorded
resolved url is incorrect.

This option causes slower installs because npm must fetch each packages
manifest to find the tarball url, but it's the most comprehensive
solution to this problem. Options like recording always the default
registry, or recording a special 'current registry' sigil will break if
registries host tarballs at different paths. For example
`${REGISTRY}/npm/-/npm-8.3.0.tgz` only works if all registries host
tarballs at `npm/-/npm-8.3.0.tgz`.
@everett1992
Copy link
Author

@darcyclarke would you add this to the agenda for the next RFC meeting? The PR implementing this RFC has a Needs Discussion tag.

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Mar 31, 2022
everett1992 pushed a commit to everett1992/cli that referenced this pull request Apr 4, 2022
Implement `$disable-write-resolves` described in npm/rfcs#486.  I named
the option `omitLockfileRegistryResolved` but that can be changed later.

Put simply, this option causes npm to create lock files without a
`resolved` key for registry dependencies forcing npm to use the current
configured registry and resolve package tarball urls on install. This
fixes install errors when users change registries and the recorded
resolved url is incorrect.

This option causes slower installs because npm must fetch each packages
manifest to find the tarball url, but it's the most comprehensive
solution to this problem. Options like recording always the default
registry, or recording a special 'current registry' sigil will break if
registries host tarballs at different paths. For example
`${REGISTRY}/npm/-/npm-8.3.0.tgz` only works if all registries host
tarballs at `npm/-/npm-8.3.0.tgz`.
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Apr 20, 2022
@darcyclarke
Copy link
Contributor

Removing the Agenda label as we've discussed this & have work-in-flight to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants