-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Do i need to commit .yarn/releases folder to my repo? #7741
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
Comments
So far the answer seems to be yes. It becomes a hard-dependency of the project to run or even install as you intend. |
Sooo... I am new to yarn, this question is exactly what I am trying to figure out. I am using Typescript and I need to make sure that other devs on this repo won't have an issue. I am assuming I need to commit the I see thumbsdown so I am confused as to what is proper procedure for this. |
Also was confused.
So, I'm guessing it goes into the repo, indeed. |
Yes, that's what that setting was made for https://yarnpkg.com/configuration/yarnrc#yarnPath |
As in git theory it's an anti-pattern
i was wondering: is there some alternatives? For instance:
… that'd would solve the above listed concerns and allow for "cleaner" and also smaller repositories. Is there something like that available or being worked on? |
I'd like to add that this could be a security risk in some cases: one could easily sneak malicious code into one of the bundles and make it look like a "normal" yarn release or plugin update. It would be quite complicated to notice anything abnormal as there isn't any integrity check. Also, to elaborate on @Philzen response, it is considered an "anti-pattern" in git not only because it looks ugly (it does), but also because it is very sub-optimal as git isn't able to properly compute and compress deltas on such minified asset. Basically any repo that follows yarn guidelines will get slower and slower with time because of this. Most package managers have abandoned vendoring nowadays (for good). Yarn docs should at least recommend using git LFS or something similar to prevent this. I see that this goes slightly towards the right direction thanks to corepack, but yarn could do this natively (i.e. without corepack) and plugins are, I think, still an unsolved issue. I honestly fail to see a justification for doing this: yarn releases, plugins and you-name-it could be put in a configuration file and a lockfile, and downloaded when needed, just like any dependency in any dependency management system? I found pinyarn that attempts to workaround this issue. |
@gilbsgilbs did you see the part of the yarn docs that recommend this in gitattributes?
From what I understand, this tells git to optimize the files (addresses your concern). If you don't like it, why not just ignore the folder in your gitignore?
UPDATE - I guess I failed to copy |
Yes, I saw that, thanks for pointing it out. As far as I understood, this will prevent the files from showing up in diffs/PRs which indeed reduces the burden and may actually help a bit with performance (I'm not actually 100% sure on this part), but git will still have to calculate deltas on binary files for no reason. A better advice regarding performance would probably be to use git lfs that would at least allow git to store pointers in its index rather than whole blobs. Those advices though make things slightly worse regarding security: you definitely won't be able to notice anything suspicious in the diffs if they don't even show up.
This is not sufficient because yarn then complains the module is missing. I ended up manually installing yarn through corepack (on top of gitignoring .yarn/releases) which works fine in my case. Still, this is way more cumbersome than the current "official way" (which is basically 100% automatic regardless of the version of yarn installed in the system), and I reckon vendoring yarn and its plugins shouldn't be promoted at all (not only to me but to any user):
For instance, regarding your CI issue, I think it could be solved elegantly by relying on your CI's caching features to put yarn releases and plugins in a cache. It would be quite easy for yarn to invalidate the releases or the plugins from the cache in case of a version mismatch. But TBH, downloading a few kilobytes in a CI server is generally not a huge deal when pnp/node_modules usually consists in downloading tens if not thousands of megabytes of dependencies. |
I'm not an expert on git, but I do believe it's efficient and not quite what you suggest, eg: https://stackoverflow.com/questions/48301920/what-types-of-binary-files-does-git-keep-deltas-for Also git LFS is not as portable (and uses similar storage mechanisms under the hood), and is more akin to committing the files to a separate repository, based on my limited understanding.
Isn't this the same with
Having CI going out to the network and trusting 1,000s of packages from 1,000s of authors that you have no control over is arguably insecure, whereas committing code you reviewed is arguably secure. In either case, I also don't feel that yarn encourages one way over the other, they give you the option as noted above. Yarn can also verify the integrity of the committed files by going out to the network with the checksums, if you want, which also addresses your concern (and if there's no way for yarn to verify the
That's fair, perhaps decoupling this suggestion into a new issue report would make it easier for others to comment on this proposal?
I don't think these are mutually exclusive. I commit my lockfile, and the
There's nothing stopping me from doing that, but I prefer to commit them since it takes time for your CI's cache feature to restore the files, in the case of my CI provider it is much faster to |
Seems like I missed the
With yarn v1/npm, you could always use the program that was installed with you system / docker image. I think the situation with berry is quite different as it may refuse to run in some cases. Also, I disagree that encouraging committing a minified JS files in any repo is “reasonable” in any case, but I fully respect and appreciate that you have a different opinion. For example, if you have very strong optimization requirements or very strict firewall rules and can't afford making packages before going to production, it might make sense to commit dependencies along with your project. However I don't think this is the case for most people using yarn (I might be wrong though).
Not talking about the same vectors here. Downloading untrusted packages is not the same as allowing internal or external contributors making malware patch pass through code reviews. Both issues need to be taken care of, but berry definitely added opportunities for the latter by encouraging the vendoring minified and unverified assets.
I agree that a checksum on the releases and plugins is a way to address the security concern I'm raising. I've yet to find such option. Also, if such option exists, the fact that it isn't enabled by default is an issue in itself as most vulnerable people are those who aren't aware of the issue.
👍
I fully agree lockfiles and vendoring are not mutually exclusive and I don't think a breaking change is required to bring lockfiles for yarn releases and plugins. For example, you can commit your Also, to eliminate any confusion, my point is about lockfiles for yarn releases and plugins, not much about the
I'd be curious to know how significant the difference would be. That makes me think: another rationale behind yarn vendoring could be to save them bandwidth. That would also be understandable, but I think it should be documented if this is the case. |
I agree. I would suggest [another] ticket for that. There's no reason to minify code being committed to the repo and run in a nodeJS context, and no one should be publishing minified code to npm (hot take, I know) since the consumer should be able to use their own build system and choose their own target (es5, es6, etc.)... so I totally agree with you here! Edit: Actually on further thought it makes sense to commit minified code assuming yarn provides a command to "eject" and review the raw source, which I think it does. Looks like with yarn plugins, you can write your own custom 3rd party "linker" that does whatever you want, including encourage committing un-minified code :)
Totally agreed!
I see it as more like yarn is providing a pathed path for people who want to do it, which is nice. I don't feel like they're "pushing" it on me, but yeah it saves everyone bandwidth and I do recall that was alluded to somewhere in the docs :) You do have some great feedback, I would just maybe filter it through a lens of understanding that yarn isn't meant to appease to one specific use case -- surely you understand yarn isn't going to prohibit an existing use case relied upon by tens of thousands of it's user-base, but you are correct about all the tradeoffs and you have some really great suggestions on how to optimize the tradeoffs :) |
## Description This PR removes unnecessary `.yarn` folders from the repo and prevents them from being checked-in / scanned by CodeQL in the future. ### Why Having yarn binaries checked-in is a security risk and CodeQL complains about issues in the scripts (which we can't fix, or even see, as they're obfuscated as binary files). Removing the `.yarn` folder for older samples is not a problem, but newer samples which use Yarn 3, such as `NativeModuleSample/cpp-lib`, need the Yarn 3 binaries checked-in. Unfortunately there's no way to get around this. Yarn refuses to acknowledge this anti-pattern and security issue (see yarnpkg/yarn#7741) and the "solution" to require users to install Yarn 3 via `corepack` does not work in GitHub CI (see actions/setup-node#480). I've tried every workaround combination in these two issues and none work. Closes #1016 ## Screenshots N/A ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/react-native-windows-samples/pull/1017)
Do you want to request a feature or report a bug?
Feature (i think)
I believe i need to commit the full .yarn/releases folder into the repository for others to use. This means that a full build of yarn is added to my repo. It would be much more usable if the required release is automatically downloaded if it doesn't exist.
What is the current behavior?
Yarn crashes if the required version is not installed
What is the expected behavior?
To install the specified yarn version if the file is not found.
The text was updated successfully, but these errors were encountered: