Skip to content
This repository has been archived by the owner on May 20, 2021. It is now read-only.

Yarn.lock patch #46

Merged
merged 1 commit into from
Dec 28, 2017
Merged

Yarn.lock patch #46

merged 1 commit into from
Dec 28, 2017

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Oct 17, 2017

In some cases like for github dependencies, the yarn.lock resolved url is generated without the sha1. This prevents yarn2nix to do it's magic. #45 is related to that.

This script goes over all the resolved urls and adds back the sha1 if it's missing. Yarn seems to not mind the additional sha1 and will only remove it if that dependency gets bumped.

Right now it's just a prototype. Ideally this should be pushed upstream so that yarn generates yarn.lock files that are useful to us directly.

@zimbatm zimbatm requested a review from moretea October 17, 2017 13:13
@zimbatm
Copy link
Member Author

zimbatm commented Oct 17, 2017

/cc @chrismwendt

@moretea
Copy link
Collaborator

moretea commented Oct 29, 2017

@zimbatm so the idea behind this is to offer this as an extra binary that people can use to fix their yarn.lock?

@zimbatm
Copy link
Member Author

zimbatm commented Oct 30, 2017

yeah but it's a bit hackish. We should probably ask upstream to fix that

@chrismwendt
Copy link

When I run ./bin/yarn2nix-fix-sha1.js, it appends the line type success at the very end, which doesn't look right. Do you see that too? Despite straying from the example at https://yarnpkg.com/en/package/@yarnpkg/lockfile this seems to fix it:

@@ -67,6 +72,6 @@ let json = lockfile.parse(file)
 var pkgs = values(json.object);

 Promise.all(pkgs.map(updateResolvedSha1)).then(() => {
-  let fileAgain = lockfile.stringify(json);
+  let fileAgain = lockfile.stringify(json.object);
   console.log(fileAgain);
 })

};

function updateResolvedSha1(pkg) {
let [url, sha1] = pkg.resolved.split("#", 2)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails on local file dependencies. For example, here's a dependency from yarn.lock without a resolved field:

gojs@./lib/gojs:
  version "1.7.14"

Returning early works: if (!pkg.resolved) { return Promise.resolve(); }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I don't think that local dependencies are supported right now but it's not an excuse to break the generation. Fixed in c2962f2

@zimbatm
Copy link
Member Author

zimbatm commented Nov 3, 2017

Thanks @chrismwendt

@moretea
Copy link
Collaborator

moretea commented Nov 3, 2017

@zimbatm could you add a small section on the README.md? Bullet-point style docs are fine with me. We should just hint to people that this is possible in.

@chrismwendt
Copy link

Do you also need to change this part now that there's a # in the URL?

--- a/bin/yarn2nix.js
+++ b/bin/yarn2nix.js
@@ -64,7 +64,7 @@ function yarnToAst(lockedDependencies) {
         if (/codeload.github.com.*tar.gz\//.test(dep["resolved"])) {
           url = dep["resolved"];
           sha1 = cp.execSync("curl -sS " + url + " | shasum | cut -d \" \" -f 1").toString().trim();
-          var matches = /tar.gz\/(.*)/.exec(url);
+          var matches = /tar.gz\/(.*)#/.exec(url);
           file_name = matches[1];
         } else {
           url = dep["resolved"].split("#")[0];

@zimbatm
Copy link
Member Author

zimbatm commented Nov 3, 2017

actually this part can be skipped now that we have the hash

@zimbatm
Copy link
Member Author

zimbatm commented Dec 18, 2017

I am going to integrate this into the main yarn2nix executable.

  • yarn2nix: patches the yarn.lock and emits the nix code
  • yarn2nix --no-nix: only tries to patch (emits notice on stderr)
  • yarn2nix --no-patch: only prints the outputs, fails if it needs to patch

What do you think?

For resources loaded directly from github, add the missing SHA1 when
invoking yarn2nix.

yarn doesn't touch entries unless they are modified directly so
it should only be needed when bumping the dependencies directly.

Ideally this would be supported by upstream.
@zimbatm zimbatm changed the title Yarnlock fix Yarn.lock patch Dec 18, 2017
@zimbatm
Copy link
Member Author

zimbatm commented Dec 18, 2017

This change breaks backward-compatibility as the lockfile is now passed as a value to the --lockfile argument. In practice I expect most people to use the default.

@zimbatm zimbatm merged commit d6e05a5 into master Dec 28, 2017
@zimbatm zimbatm deleted the yarnlock-fix branch December 28, 2017 22:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants