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

[CAS] Prefix folder names with package name #48

Closed
brianreavis opened this issue Dec 9, 2015 · 15 comments
Closed

[CAS] Prefix folder names with package name #48

brianreavis opened this issue Dec 9, 2015 · 15 comments

Comments

@brianreavis
Copy link

The downside with checksum folder names is that it can make stacktrace debugging very difficult. It's impossible to tell what module is what without manually opening up each folder.

current: 2796642723573859565633fc6274444bee2f8ce3
suggested (a): async_2796642723573859565633fc6274444bee2f8ce3
suggested (b): async_1.5.0_2796642723573859565633fc6274444bee2f8ce3

@alexanderGugel Is this something you'd be open to?

@alexanderGugel
Copy link
Owner

I agree that it's a problem, on the other hand it kind of defeats the point of having content-addressable directory names for dependencies if you put an arbitrary name in front of it.

That being said we could simply ignore the prefix.... So yes, if you someone figures out an elegant way to implement this, I'm very open to it.

@terinjokes
Copy link

Per http://nixos.org/nixos/about.html, NixOS appends the package names (rather than prepend, as proposed here). The length of the hash is going to be constant, and you can just ignore everything after that.

@rstacruz
Copy link
Contributor

I like it prepended because it'd make stack traces easier to visually skim through.

  in /home/me/project/node_modules/lodash@4.0.0_d3a4db3ac0f..
  in /home/me/project/node_modules/rimraf@2.5.3_a92bde83a..
  in /home/me/project/node_modules/got@4.1.1_ba93da28b..

It will also make ls node_modules easier to read—but that's subjective, you can argue the same for the suffixes :)

@rstacruz
Copy link
Contributor

as a side note, @ is valid in pretty much every modern file system, including Windows's.

@just-boris
Copy link
Collaborator

I don't think that the feature is really necessary.

However, I'd like to put these generic directories with packages into a special folder like node_modules/.ied. Then you will get pretty neat node_modules folder

└── node_modules
    ├── .bin
    ├── .ied  
    │   ├── 007704b84df1d82a87334f0c9f938e2cac97257f
   ... ...   // many packages here, probably you don't want to see them all
    │   └── b09d9dfba0b63b9785058b54275729473bd135b0
    ├── browserify -> .ied/8ed522c44cb00acf0234c9b74828e715391f59fc
    ├── express -> .ied/51f1744ec7fffebfd6e2b295ce5e5cb3c9471abd
    ├── grunt -> .ied/39b1b8b7f0ec6ef2be6c77ed350cda1b935a31a0
    ├── gulp -> .ied/3ee1c61aaaf79cd68fe1dfb0233a42f456d9bd61
    ├── lodash -> .ied/8e3ad7e208585af98fd70ba00dcc2592de325e6e
    ├── mocha -> .ied/2bb91c10ac787d29b5a3e9de0c49e81b89aa8f5b
    └── tape -> .ied/54ec118d96f3a42442a3084ebccd3c93eb6db829

That can be a thing that you are actualy want.

@brianreavis
Copy link
Author

@just-boris How does this simplify anything? Say you get a stacktrace emailed to you – you literally have no idea what's going on unless you ssh into the server to see what all the hashes match up to. The neat directory is merely aesthetic, no?

@rstacruz The @ symbol seems like a great idea 👍

@just-boris
Copy link
Collaborator

@brianreavis that is probably a valid point, thanks for pointing this.

Scoped packages are installed in the folder prefixed with @, so there is nothing wrong to have a folder like @scope/package@0.0.10. Also, the hashsum suffix seems here as redundant because it consists from sha(name + '@' + version), so now we just can replace directory name generating logic and it possibly will work

@davej
Copy link

davej commented Jan 26, 2016

Append or prepend both sound fine, I can see pros and cons of both. Is it work considering a shorter hash too, 10 or 12 hex character perhaps? Are there practical disadvantages to a shorter hash (10 hex chars is still over a trillion uniques)?

@just-boris
Copy link
Collaborator

@davej once again, we can get rid of hashes in folder names at all.

I just haven't tried to implement it, but it seems possible.

@davej
Copy link

davej commented Jan 27, 2016

@just-boris: I thought the folder names were CAS, i.e.. generated from content checksum rather than the package + version. I'm on mobile phone so can't look at the codebase now but from the readme:

node_modules as CAS - Packages are always being referenced by their SHA-1 checksums. Therefore a node_modules directory can be considered to be a Content Addressable Storage, meaning that packages are being identified by their contents, not by arbitrary identifiers, such as package names that are not guaranteed to be unique across different registries.

@just-boris
Copy link
Collaborator

@davej, oh, sorry. There were some changes, and that sentence from the Readme is not actual and should be updated.

Now folders names is not actually a shasums of the content. It was a necessary change to support other package sources, like git of arbitrary tarballs. (They don't provide shasum of its content in advance like npm registry does).

So now, it is easy to change it and bring some humanity into folder names and provide more friendy stack traces for users.

@davej
Copy link

davej commented Jan 27, 2016

Cool, well then I guess it makes perfect sense to drop the hash.

@rstacruz
Copy link
Contributor

dropping the hash sounds like a fantastic idea. really excited for the future of ied.

@rstacruz
Copy link
Contributor

just chiming in with some lessons learned in my own implementation.

you can't always expect name@version to be unique if you're also fetching from non-npm registries (eg, installing from tarballs). this is a possible case in a dependency tree like:

- foo@1.0.0
  - jquery@1.9.0
  - lodash@4.0.0
- bar@1.0.0
  - a patched lodash@4.0.0 fetched from git or http

in those cases, you can use name@version#hash (where hash is derived from the URL or something) for the non-npm-based modules.

@alexanderGugel
Copy link
Owner

Current implementation is fine. Prefixing CAS names with package names kind of defeats the purpose of having CAS names in the first place in my opinion. You can get the package name either from the package.json or from the respective symlink. I don't see an issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants