search

tessel / t2-cli

Tessel 2 Command Line Interface
MIT License
114 stars 56 forks source link

symlinked modules are not copied over on run/publish #371

Open dtex opened 9 years ago

dtex commented 9 years ago

If there aren't any reasons this can't or shouldn't be done I'll give it a shot.

johnnyman727 commented 9 years ago

Oooof nice catch @dtex!

dtex commented 9 years ago

It's an fstream thing. Trying to determine if it is a bug or intended behavior.

dtex commented 9 years ago

A PR was submitted a couple of years ago but is still open. I've pinged the assignee:

https://github.com/npm/fstream/pull/16

dtex commented 9 years ago

Cautionary note: My use case for including symlinks was so that I could link to local repos of modules I contribute to. Since I am running npm install in those local repos the devDependencies are installed along with the dependencies. It's pretty easy to run out of space when those are get pushed to the Tessel. Running install with --production gets around that.

npm install --production

Also watch out for .git directories.

rwaldron commented 9 years ago

We can add these to the implied .tesselignore entries https://github.com/tessel/t2-cli/blob/master/lib/tessel/deploy.js#L212-L219

dtex commented 8 years ago

Update, ownership of that PR has changed a couple of times and they are asking for tests.

Also note that slim builds are not subject to this problem.

rwaldron commented 8 years ago

Thanks for the update here, we'll have to be sure to re-visit this once the pre-compiled binary modules support is merged

rwaldron commented 8 years ago

@dtex is this still problematic? By default, all build are --slim now

johnnyman727 commented 8 years ago

@dtex is this still an issue?

johnnyman727 commented 8 years ago

@dtex do you know if this is still an issue? Looks like the related issue you linked above is still open so I'm assuming the CLI is still affected...

dtex commented 8 years ago

@johnnyman727 Oof, so sorry. I wasn't getting notices on this which is weird. The problem isn't fixed in fstream but so much work has been done on the T2 deployment stuff that it is unclear to me if we are still dependent on fstream for recursively copying directories within node_modules. On the surface it appears not.

I'll test tonight.

rwaldron commented 8 years ago

s unclear to me if we are still dependent on fstream for recursively copying directories within node_modules. On the surface it appears not.

It's both, depending on which path is taken. The --full flag will take the easy way out and use fstream, whereas the default path (--slim) does all the hoop jumping, directory traversing, nit-picking, roof-topping, re-rocking, crip-walking...

Looking back to the original post... I think your clever workaround isn't necessary anymore. The devDependencies will only be copied if you explicitly do --full, which I think no one should ever do, because that's nutty. Or, you can now list stuff in a .tesselignore! Anyway, I'm still curious about the symlinking in the default (--slim) deployment path