-
Notifications
You must be signed in to change notification settings - Fork 147
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
add rfc about registry url in lock file #84
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @vernak2539 that is a great start!
Just some cleanup and more concrete examples would be enough.
Let me know if this isn't enough, first time writing an RFC. | ||
|
||
|
||
As a dev that generated their lock file against REGISTRY_A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we need a more technical explanation with examples of yarn.lock and config parameter names and concrete URL examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bestander updated the PR. Let me know if more is needed
|
||
# How We Teach This | ||
|
||
This would largely be in the internal workings, and wouldn't require teaching. npm 5 does this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just need an entry in docs page, maybe here https://yarnpkg.com/en/docs/yarnrc.
A short blog post would be great too.
It is straight forward, so no need to mention anything else here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, @vernak2539.
I've left a few comments
|
||
# Motivation | ||
|
||
We currently have two internal registries/mirrors. We generate the lock file against one we have in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add more details please.
Short examples how the yarn.lock files are different would be useful
our download to error as yarn cannot retrieve the packages. | ||
|
||
The expected output is for yarn to recognise the registry URL supplied (via either cli flag or | ||
`.yarnrc` or `.npmrc`) and download from that if the registry used in the lock file is not available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to do it in .npmrc, just mention .yarnrc.
no longer have access to the original registry that was used to generate the lock file. This causes | ||
our download to error as yarn cannot retrieve the packages. | ||
|
||
The expected output is for yarn to recognise the registry URL supplied (via either cli flag or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please provide the name of the flag
I would like to designate an alternate registry URL in my `.yarnrc` or `.npmrc` (REGISTRY_B) | ||
And I would like yarn to download my dependencies from the alternate URL (REGISTRY_B) | ||
|
||
I feel like the simplest solution would be to add a "hash" or "commit-ish" field to entries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a lot of text that does not add much value compared to the previous section.
Here we need to see concrete examples how Yarn will behave with the new feature.
I suggest something like this:
If yarn.lock file contains
abbrev@1:
version "1.1.0"
resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.0.tgz#d0554c2256636e2f56e7c2e5ad183f859428d81f"
And .yarnrc has setting override-registry={"registry.yarn.pkg.com": "registry.npm.org"}
Then Yarn will fetch abbrev-1.1.0.tgz
from https://registry.npm.org/abbrev/-/abbrev-1.1.0.tgz#d0554c2256636e2f56e7c2e5ad183f859428d81f
instead of the one specified in yarn.lock.
|
||
This, with the version already provided, could then be used to generate a URL on the fly or at the beginning of operations if an alternate registry is specified in the scenarios above. | ||
|
||
Or, the `resolved` field could change to not include the hostname/origin, which would then be inserted during operations. This would |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this proposal simple and not have many possible solutions in one place, you can list them below in the alternatives section
already, so it would be inherent. | ||
|
||
> If you generated your package lock against registry A, and you switch to registry B, npm will now try to install the packages from registry B, instead of A. If you want to use different registries for different packages, use scope-specific registries (npm config set @myscope:registry=https://myownregist.ry/packages/). Different registries for different unscoped packages are not supported anymore. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's just copy what npm5 does.
Make a proposal to match npm5 behavior with similar examples and configs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want a proposal to do just like npm, nothing would need to change, just yarn would have to take into account the registry in the rc file, and use the registry in that no matter how the lock file was generated. I think that's where I was going with the original one, but that would be a fundamental change in behaviour, probably a breaking change
I've updated the PR as well as my comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I contribute something here? Very eager to see this go through - it is a dealbreaker at the moment for us since we (like @vernak2539 ) use different registries in different build environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csvan, everyone's contribution is welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
our use case is sharing code between external (outside the company, seeing directly public npm repos) and internal developers (i.e. working inside a company, with proxy and an artifactory mirroring the public npm repos). moreover, there is a ci that also sees only internal repos.
actually, there are two issues with repos in yarn.lock:
- while reading (i.e. when installing npm modules)
- while writing (i.e. when new modules are added)
my remarks / desires (without regard whether they are fulfilled by the suggested solutions above):
for 1.: basically, what we need is to have a single yarn.lock that is valid and working in both environments, so it can reside in our git repo. even if we would be fine without them, yarn.lock must have embedded repo URLs for backward compatibility. hence we would like to have a single set of URLs, which should be: public URLs for public available npm packages and private (i.e. pointing to the artifactory repo) URLs for private packages (actually, the company has separate artifactory URLs for public and private npm packages). obviously, access to the private packages is only possible from inside the company, so this would break for external developers - which is OK.
for 2.: the desire to have a single set of URLs both valid inside and outside the company (obviously this is only possible for public packages) would mandate that public URLs are written into yarn.lock even when a dependency on a public npm package is added by an internal developer and thus served by the internal artifactory repo. note that this use case does typically not occur during a build process (e.g. in a ci/cd). of course, the package namager has no other means to decide whether a package is public than configuration and the final package URL (in our case, it would be possible to make this distinction, as private packages have a different internal URL, in other cases there might be a set of rules giving such a distinction - which in turn might be overshooting to support).
thus we would probably need means to override a certain public repo by a private one as a local configuration property (not going into the git repo - e.g. in .npmrc) and simultaneously using this override "backwards" when writing URLs into yarn.lock. note also that the override cannot only be only a host, as the URL path and/or the protocol under which npm modules are visible may differ between different repos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any update on this rfc ?
Overall I like it. If not then let's merge it and wait for a PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, waiting for more comments.
If none please ping me to merge this as is in 5 days.
I think this is a good approach but why not go one step further and remove the registry URLs from the lockfile completely? |
|
Hey, thanks to @csvan for pointing this out! We're feeling this pain when moving registries (yarnpkg/yarn#4817)... This might seem trivial, but I want to make sure (since I did not see this mentioned) - will the added flag also recreate the package checksum? (the checksum ultimately includes the repository url, so if a package was re-published to another registry we'll get a checksum mismatch). |
Commented on the linked issue, I think this is a separate thing |
We can probably add a new field per resolution and signal this easily. That should be forward and backwards compatible. Only thing is new lock files wouldn't work with old Yarn versions but I think that was never a concern.
With my suggestion above, I'm not sure if it would be that big of a difference. The advantage here is avoiding extra processing per field. |
@BYK, I am ready to discuss the change. :) Stripping npm/yarn domain needs way more than just one step forward, we need to think of the migration path. |
To keep the ball rolling I'll merge this as is. @vernak2539 are you eager to send a PR? |
Regarding the implementation, I have two comments:
|
I also note the following comment quoting the npm behavior:
Why do we need to support a registry=>registry mapping, then? |
Since this could be slated for yarn v2, we could ignore backwards compatibility. IMO adding another setting to override the registry is just going to cause confusion between when to use |
Any plan to implement a solution within yarn after all those years ? |
No description provided.