tink: Refactor and finish fs.js

help-wanted
good-first-patch
tink

(Kat Marchán) #1

This one’s fairly straightforward but is going to involve a bit of rote work and occasional creativity! This is a refactor that I’d like to do, and I was about to start doing it myself when I realized it’d be great for someone just getting started with the codebase – if this is you, then today’s your lucky day!

The task is straightforward and involves two steps:

  1. Rewrite all current functions in lib/node/fs.js to be “optimistic” about original ops. That is, they should try hitting the original file and doing their normal behavior and only falling back to tink if that fails (usually with ENOENT).
  2. Go through the API for Node.js’ fs module and fill out any remaining functions that need to be backfilled. Some of these backfills are special: for example, open needs to extract the file first and let the user have a normal file. fs.write* operations would need their parent directories created if they’re missing, etc.

I’ve started the refactor, which should give you an idea of the changes I’m hoping to do. If you only want to do some of it and tag-team someone else in later, that is also completely fine.

Are you interested in taking this one up? Just reply to this post and claim it! I’m hoping someone will be able to get it done this week, and I don’t expect it to take more than a few hours since it’s so self-contained and involves such small functions, one at a time.


(Suchipi) #2

Hello! I’m interested in taking this task on. I’m pretty familiar with the fs module, so it would be a good way to get some exposure to tink, which I’m interested in.

I haven’t contributed before- how do I contribute? Is the source on GitHub?

EDIT: Just noticed the GitHub Link in your “started the refactor” comment


(Kat Marchán) #3

All yours, then! Thank you so much!

Just make a PR when you’re done. You can ask any questions in this thread as needed!


(Suchipi) #4

What version of node should I use when developing? What version(s) of node does tink support?


(Kat Marchán) #5

Node 10 is the target for the time being


(Suchipi) #6

Go through the API for Node.js’ fs module and fill out any remaining functions that need to be backfilled. Some of these backfills are special: for example, open needs to extract the file first and let the user have a normal file. fs.write* operations would need their parent directories created if they’re missing, etc.

Could you go into further detail about what “extract” and “if their parent directories are missing” mean in this context? It seems like it’s in reference to the cache in pkglock (which I think is where it’s resolving stuff from?) but I’m not super clear on the mental model for how tink works. If I have something in the cache and I need to put it into the real filesystem so the user can write to it, how do I do that? When should they write to the filesystem and when should they write to the cache?


(Suchipi) #7

Also, what’s the difference in meaning between pkglock.resolve returning null vs false?


(Kat Marchán) #8

null means there’s no packages that might have it inside node_modules, and false means the package exists, but the file does not. That’s one of the distinctions that’ll stop making a difference once we switch to optimistic mode, though.

When I talk about “extract” I mean literally writing physical files to node_modules the same way fs.open's override does (and in fact, this override might mean you don’t need to override fs.write* at all). The cache should never be written to except by the installer itself and any other modifications to node_modules happen directly there.


(Suchipi) #9

Okay, cool, makes sense. Thanks!


(Suchipi) #10

Okay, I opened a PR here: https://github.com/npm/tink/pull/24

I only did the optimistic part, didn’t backfill anything- I’ll leave that to someone else.

There were some merge conflicts so sorry if things aren’t quite right.