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

Allow missing hashes #5

Closed
wants to merge 3 commits into from
Closed

Allow missing hashes #5

wants to merge 3 commits into from

Conversation

cnr
Copy link
Contributor

@cnr cnr commented Nov 1, 2019

(this includes the commit from my other PR that updates to Megaparsec 7.*)

yarn.lock allows hashes to be omitted in the resolved field, so I made them optional

@Profpatsch
Copy link
Owner

Can you explain this change in some more detail? What does it mean for these hashes to be optional?

@cnr
Copy link
Contributor Author

cnr commented Nov 4, 2019

we've encountered a few yarn.lock files where the hash is missing in the resolved field; something like:

mypackage@1.0.0
  resolved "https://example.com/file.tgz"

as opposed to

mypackage@1.0.0
  resolved "https://example.com/file.tgz#somesha256"

Some of the source fetchers in yarn allow you to omit the hash, which will happily skip the validation step.

(why anyone would ever use this "feature" is beyond me..)

@Profpatsch
Copy link
Owner

we've encountered a few yarn.lock files where the hash is missing in the resolved field; something like:

Oh, okay. This might make it interesting for yarn2nix, but I’m used to the logic of the yarn lock being completely bonkers.

Out of interest, are you using yarn-lock as a standalone library?

(why anyone would ever use this "feature" is beyond me..)

If npm is the mother of bad design decisions, yarn is its firstborn. :)

@Profpatsch
Copy link
Owner

One more thing, can you paste a complete lockfile into a gist where you noticed that? I’d like to see the whole thing if possible, to figure out the reason maybe.

@cnr
Copy link
Contributor Author

cnr commented Nov 5, 2019

I haven't been able to find examples in public repos yet -- but it's something I came across when parsing yarn.lock files in our company's repos.

I wonder if it's behavior from old versions of yarn, which is still allowed for backward compatibility

Out of interest, are you using yarn-lock as a standalone library?

Yes -- though I'd be happy to submit a PR to yarn2nix to support these changes. Might need some help figuring out how to best handle the missing hashes

@Profpatsch
Copy link
Owner

Profpatsch commented Nov 6, 2019

Yes -- though I'd be happy to submit a PR to yarn2nix to support these changes. Might need some help figuring out how to best handle the missing hashes

I’m not sure how fast I can come around to thinking through this, but we can merge the changes to this library nonetheless.

@Profpatsch
Copy link
Owner

Hm, I really don’t know what I should think about this.

If FileRemote hashes are not guaranteed, then the basic premise of yarn2nix is kind of gone.
Can’t you give me any example of what distinguishes files without hashes from files with hashes? Is it only in old versions of yarn maybe? What’s the version of the yarn file?

@cnr
Copy link
Contributor Author

cnr commented Nov 18, 2019

Here are a couple:

yarn.lock

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1
[...]
cacheman@appleboy/cacheman:
  version "2.0.5"
  resolved "https://codeload.github.com/appleboy/cacheman/tar.gz/fca43cd43b5782909947482a13ba2127e3f37929"
  dependencies:
    cacheman-memory "^1.0.2"
    ms "^0.7.1"
[...]
"underscore-deep-extend@github:fossas/underscoreDeepExtend":
  version "1.1.5"
  resolved "https://codeload.github.com/fossas/underscoreDeepExtend/tar.gz/045f2e86fbe0ddb06e8d542d8b7524dd75f5b9d5"
[...]

It looks like these were added years ago when we switched to yarn. Excerpts of the diff from package-lock.json:

[...]
-    "underscore-deep-extend": {
-      "version": "github:fossas/underscoreDeepExtend#045f2e86fbe0ddb06e8d542d8b7524dd75f5b9d5"
-    },
[...]
-        "cacheman": {
-          "version": "github:appleboy/cacheman#fca43cd43b5782909947482a13ba2127e3f37929",
-          "requires": {
-            "cacheman-memory": "1.0.2",
-            "ms": "0.7.3"
-          }
-        },
[...]

notably, the integrity fields are missing there

In package.json

[...]
    "cacheman": "^2.1.0",
[...]
    "underscore-deep-extend": "github:fossas/underscoreDeepExtend",
[....]

Not sure what to make of that.

It looks like yarn used to traverse node_modules when importing a project, but even if it had imported from package-lock.json, the integrity fields would be missing

@Profpatsch
Copy link
Owner

It looks like these were added years ago when we switched to yarn.

Hmm, what happens if you re-generate them with a modern yarn then (if that’s possible)?

@cnr
Copy link
Contributor Author

cnr commented Nov 20, 2019

Regenerating (with yarn import) still omits the hashes 🙁

Even if the hashes were there, our use case requires us to successfully parse all valid yarn lock files. I'd understand if you don't want to support that though -- and I don't mind maintaining a fork for our specific usecase

@Profpatsch
Copy link
Owner

Even if the hashes were there, our use case requires us to successfully parse all valid yarn lock files. I'd understand if you don't want to support that though -- and I don't mind maintaining a fork for our specific usecase

In this case, let’s add another case besides FileRemote that doesn’t have a hash, instead of making the hash fields optional.

@cnr
Copy link
Contributor Author

cnr commented Dec 16, 2019

Added two new data constructors, FileRemoteNoIntegrity and FileLocalNoIntegrity. The names are kind of verbose

@Profpatsch
Copy link
Owner

The names are kind of verbose

I’m fine with verbose names :)

@Profpatsch
Copy link
Owner

Thanks, I wasn’t able to push to this PR, so I rebased, added a release and pushed to master. The new release is also on hackage

@Profpatsch Profpatsch closed this Dec 22, 2019
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.

None yet

2 participants