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

Idea: collaboration with pnpm #182

Open
zkochan opened this issue Nov 8, 2016 · 34 comments
Open

Idea: collaboration with pnpm #182

zkochan opened this issue Nov 8, 2016 · 34 comments

Comments

@zkochan
Copy link
Collaborator

zkochan commented Nov 8, 2016

I am thinking about how could we collaborate (maybe even merge).

I already see many people that are creating issues and commenting in both repos and pnpm was based on the ideas of ied (or maybe even a fork? i don't know the whole story). So why don't we unite efforts?

I really like the new issue created by @alexanderGugel about rewriting ied to Go - #181. It is something that I was thinking about as well.

I might be wrong, but I think currently pnpm has less issues than ied. It runs good on Windows, can install packages globally, works mostly well when running with the --preserve-symlinks option. What if we will recommend to use pnpm and will try to solve all the issues there. In the meantime we will be working on rewriting ied to Go. The two projects can use the same store structure, CLI and config files in order to guarantee a smooth switch once the rewrite is ready.

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 8, 2016

cc @iamstarkov, @andreypopp, @rstacruz

@billiegoose
Copy link
Collaborator

Ooh ooh! If you guys are writing a new package manager & thinking about a common store directory structure, I want in. I started on a (highly) experimental package manager a little while ago. One of my goals is to eventually be able to require() dependency tarballs directly, rather than have to extract them first. This adds some safety from having immutable dependencies, and allows you to verify the tarball checksum at any point in the future (say, just before loading) to make sure it hasn't been altered. (My initial prototyping utilized archivemount to mount the tarballs in the FUSE file system in read-only mode. ) BUT it has the interesting effect that you cannot put symlinks inside the package's node_modules folder, because the package folder is read-only. So I had to invent an entirely new store directory structure that is different from pnpm and ied 1.x. (Actually it looks similar to the directory structure used by ied 2.x... although I can't tell because ied 2.x has never worked on my machine.) Any opportunity to collaborate would be great for motivating me to continue working on the project. :)

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 16, 2016

I think a common store directory structure is a good idea. What do you think about creating a spec-project that would describe the store structure. It can be called something like Modules Store specs?

@alexanderGugel
Copy link
Owner

Yes, that sounds like a good idea. Sorry for the delayed response, I just checked out @wmhilton's module loader... looks really nice. I like it.

But back to the spec: Yes, I think formalising the symlink-based installation procedure is a good idea. On the one hand it makes a stronger case for not "deprecating" the preserve-symlink module resolution, on the other way it would provide some common ground for ied, pnpm and whatever comes next.

I would suggest we keep this issue open as a discussion forum for ongoing work on this spec while writing some technical documentation on how the resolution is supposed to work (including the currently available module resolution in Node, since we might have to patch Node if everything goes wrong).

I'm going to come up with an initial draft this weekend, if anyone else wants to own this (@zkochan you seem exited about this 😄 ) and / or already has some docs, I would suggest we work from there. Otherwise I'm going to write some really serious specifications docs on this and we collaborate on that? 🎉 🎈

Thanks @zkochan for opening this issue!

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 17, 2016

That sounds great! And I totally would like to be a collaborator there!

I am not really familiar with the ied store structure but let me describe the pnpm one. And I think there are many good ideas in the pnpm store structure.

.modules.yaml

First of all we have a .modules.yaml in the root of every node_modules folder. It contains currently just one property with the store location. E.g.:

storePath: 'C:\Users\zoltan.kochan\.pnpm\.store\nested'

We use this like a checksum. If someone tries to install a new package using a diferent store, pnpm throws an exception. One node_modules should use one store. (On the other hand, one store can be used by many node_modules)

.store.yaml

In store.yaml we store the dependency graph of the store. We need it mostly for uninstall, as far as I remember. It is in the root of the store and looks like this:

pnpm: 0.45.0
type: nested
preserveSymlinks: true
packages:
  'C:/Users/zoltan.kochan/.pnpm':
    dependencies:
      npmrc: [email protected]
      gulp: [email protected]
  [email protected]:
    dependents:
      - 'C:/Users/zoltan.kochan/.pnpm'
  'C:/src/resources/foo':
    dependencies:
      debug: [email protected]
      compression: [email protected]
      require-from-string: [email protected]
      express-cluster: [email protected]
      safe-eval: [email protected]
      snabbdom-to-html: [email protected]
      node-fetch: [email protected]
      express-status-monitor: [email protected]
      tap-spec: [email protected]
      snabbdom: [email protected]
      tape: [email protected]
      request: [email protected]
      express: [email protected]
  [email protected]:
    dependents:
      - 'C:/src/resources/foo'
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - 'E:/src/resources/foo'
    dependencies:
      ms: [email protected]
...

Store directory structure

pnpm tries to make user friendly names for directories in the store. So for [email protected] there will be a directory called like that in the store. For a package resolved via github, there would be a directory called as the repo URL (escaped to be a valid folder name).

I think the store structure should be human friendly, when possible. I would suggest to make the structure something like this: <package source>/<package name>/<package version>.

So for a package resolved from the official npm registry, the path to the package would look like: .store/registry.npmjs.org/gulp/2.1.0/

to a git-hosted package mayby something like: .store/github/alexGugel/ied/b246270b53e43f1dc469df0c9b9ce19bb881e932/

@billiegoose
Copy link
Collaborator

@alexanderGugel why thank you! it's not really much more than experiment at the moment.

You know, that .store.yaml file looks a lot like a yarn.lock file...

# yarn lockfile v1
abbrev@1:
  version "1.0.9"
  resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.0.9.tgz#91b4792588a7738c25f35dd6f63752a2f8776135"

accepts@~1.3.3, [email protected]:
  version "1.3.3"
  resolved "https://registry.node-modules.io/tarballs/accepts/1.3.3.tgz#c3ca7434938648c3e0d9c1e328dd68b622c284ca"
  dependencies:
    mime-types "~2.1.11"
    negotiator "0.6.1"

accord@^0.26.0:
  version "0.26.3"
  resolved "https://registry.node-modules.io/tarballs/accord/0.26.3.tgz#7a6f07dfe204f67f0d48ee6c7b0fe40723183010"
  dependencies:
    convert-source-map "^1.2.0"
    glob "^7.0.5"
    indx "^0.2.3"
    lodash.clone "^4.3.2"
    lodash.defaults "^4.0.1"
    lodash.flatten "^4.2.0"
    lodash.merge "^4.4.0"
    lodash.partialright "^4.1.4"
    lodash.pick "^4.2.1"
    lodash.uniq "^4.3.0"
    resolve "^1.1.7"
    semver "^5.3.0"
    uglify-js "^2.7.0"
    when "^3.7.7"
...

except yours is a doubly-linked list because it has dependents too. I suppose that makes uninstalling packages simpler? Because you'd be doing reference counting rather than garbage collection? I kinda like that.

So for a package resolved from the official npm registry, the path to the package would look like: .store/registry.npmjs.org/gulp/2.1.0/

I like that idea! But I'd say we make the second example .store/github.com/... for consistency. Namespace using domains. Almost everything has a website. Even if it's non-traditional (ipfs, gittorrent, dat) you can still namespace by domain (ipfs.io, datproject.org) even if the content isn't physically stored on that domain.

This is really interesting!

@iamstarkov
Copy link

@wmhilton

You know, that .store.yaml file looks a lot like a yarn.lock file...

that was intention

@iamstarkov
Copy link

I suppose that makes uninstalling packages simpler?

yep

@billiegoose
Copy link
Collaborator

I was focused on other projects this weekend, did I miss any spec-writing action?

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

I've just created a repo and added you guys as collaborators: https://github.com/zkochan/package-store-spec

We probably can create PRs there with suggestions and discuss each part of the spec

@alexanderGugel
Copy link
Owner

Nice! I'm still at Overview and Motivation though 😄
@zkochan Do you want to move your repo to the ied wiki or keep it separate?

@alexanderGugel
Copy link
Owner

@zkochan I added what I have so far to the Wiki. I would suggest we move the discussion to there, so anyone can edit. Then we can move it to a common organisation or sth. Thoughts?

https://github.com/alexanderGugel/ied/wiki/Spec

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

@alexanderGugel yeah, sure, I have no objections.

@alexanderGugel
Copy link
Owner

Also I referenced a talk I recently gave at Node Live about the installation mechanism, directory structure etc. (https://www.youtube.com/watch?v=mNhZrd1VgPs) Might be useful, since it explains the directory struct quite well etc.

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

I added some "Known Issues" to the spec.

And created a draft for the store spec: https://github.com/alexanderGugel/ied/wiki/Package-Store-Spec

Great presentation!

@billiegoose
Copy link
Collaborator

Ugh. Not a fan of Github's wiki. There's no "preview diff", no way to add comments about specific lines like you can in pull requests. Can we move the spec back to zkochan's repo?

@billiegoose
Copy link
Collaborator

For now, I've just edited https://github.com/alexanderGugel/ied/wiki/Package-Store-Spec
But it feels so wrong. I wish Github wiki's had a "Discussion" page like Wikipedia.

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

I tried to answer your questions there. Yeah, it is not really convenient this way.

@alexanderGugel
Copy link
Owner

alexanderGugel commented Nov 22, 2016

We can move it into a separate directory and I add you guys as contributors? Thoughts?

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

works for me

@alexanderGugel
Copy link
Owner

alexanderGugel commented Nov 22, 2016

Cool, cool. 😄

@zkochan @wmhilton I gave you guys push access and created a new spec branch and directory.
I added all the beautiful docs we have so far in there.

I would suggest we organise major proposals in form of PRs to this branch (no direct pushing to master please 😄 ), then have someone else "approve" them.

https://github.com/alexanderGugel/ied/tree/spec/spec

Any thoughts / concerns about this workflow?

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 22, 2016

@alexanderGugel I am fine with working on this in the ied repo. However, I am afraid we'll create too much noise for the people that are watching ied 😄

@alexanderGugel
Copy link
Owner

Noise is always good! :trollface:

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 24, 2016

In store.yaml, should the paths be normalized? We normalize them in pnpm because Windows paths, when escaped, look very ugly (C:\\some\\path)

@iamstarkov
Copy link

as far as it crossplatform, i dont see any problem in either way of doing things

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 24, 2016

Do we want support Node prior to 6.3.0? I would suggest to not :-) It gives some flexibility. Like no need to create the subfolder in the store for the package. (you know the package/ in ied and _/ in pnpm).

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 24, 2016

On the other hand, with a subfolder we'd be able to save the tarball next to the package directory

@billiegoose
Copy link
Collaborator

Got distracted by the holiday and family visiting.

I am good with working on the spec in a subdirectory of ied. I thought it was bizarre at first but you are right, it will attract more attention this way which is good.

@zkochan what was added in node 6.3 that makes the intermediate directory unnecessary?

@billiegoose
Copy link
Collaborator

Not sure if I should make a new issue or just add this to the thread. I've been thinking about zkochan's comment:

zkochan: Is it OK to use git tags? Git tags are mutable. Someone can tag a different commit

And my original thought was, well npm allows mutable tags like latest. I thought I was addressing that by requiring annotated (full) git tags. But something in the way zkochan worded that made me think about security. What if someone maliciously changed git tags? Or for that matter replaced versioned packages in npm, assuming they figured out how to publish over an existing package. Even man in the middle attacks or simply file corruption. I suggest we store an exact hash of the package in the store.yml in addition to the semver. Maybe we need to include a hash in the filesystem path too to guarantee that unique packages have unique locations, but if the store is the source-of-truth I don't think that's necessary. There's not a scenario where we'd want to support two packages with the same (repository, package name, package version) but different content is there? The only scenario I can think of would be supporting patched modules.

@zkochan
Copy link
Collaborator Author

zkochan commented Nov 27, 2016

@zkochan what was added in node 6.3 that makes the intermediate directory unnecessary?

In Node 6.3.0 the --preserve-symlinks option was added to Node. Why it is important I tried to describe here

Regarding git tags my suggestion would be to always resolve them to commits. So you can install via a git tag but it will be saved in the store as a commit. So theoretically if you do a pnpm update, the dependency can install a new commit which is marked with the latest git tag.

@billiegoose
Copy link
Collaborator

Damn peer dependencies. :-) Yes, that behavior is definitely needed for any modules that rely on Node's require.resolve implementation. But we should really convince the world to stop doing that and manage peer dependencies more explicitly.

I agree with the git tags suggestion then.

@alexanderGugel
Copy link
Owner

Regarding git tags my suggestion would be to always resolve them to commits. So you can install via a git tag but it will be saved in the store as a commit. So theoretically if you do a pnpm update, the dependency can install a new commit which is marked with the latest git tag.

Definitely. Using the actual shasum is the right approach IMO. Shasums are great. They're sick awesome.

@rstacruz
Copy link
Contributor

Just chiming in and saying you guys are awesome. ❤️

@zkochan
Copy link
Collaborator Author

zkochan commented Jan 2, 2017

A PoC of global (machine) store implemented on pnpm: pnpm/pnpm#524

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

No branches or pull requests

5 participants