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

jitsu deploy not bundling symlinks #379

Closed
sam3k opened this issue Jan 22, 2013 · 20 comments

Comments

Projects
None yet
9 participants
@sam3k
Copy link

commented Jan 22, 2013

How to reproduce:

  1. App with symlinks (other than symlinks inside node_modules)
  2. jitsu deploy
  3. Checking what was sent via: tar tvvf npm pack

Symlinks are not bundled in the deploy.

@AvianFlu

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2013

This is an npm issue.

@sam3k

This comment has been minimized.

Copy link
Author

commented Jan 22, 2013

@AvianFlu is there a workaround? Also, do you know if there is an issue in npm to keep track of this?

@dougnukem

This comment has been minimized.

Copy link

commented Feb 5, 2013

Ran into this issue myself fixed it by hacking in a predeploy step that fully resolved my symlink and then a postdeploy step to convert it back to a symlink so local dev is using the symlink:

http://stackoverflow.com/questions/14698684/nodejitsu-deploy-of-webrtc-demo-failing-to-server-client-side-javascript/14698971#14698971

"scripts": {
    "predeploy": "cd ./example/public && rm webrtc.io.js && cp ../../node_modules/webrtc.io-client/lib/webrtc.io.js webrtc.io.js",
    "postdeploy": "cd ./example/public && rm webrtc.io.js && ln -s ../../node_modules/webrtc.io-client/lib/webrtc.io.js webrtc.io.js",
    "start": "node example/server.js"
  }
@sam3k

This comment has been minimized.

Copy link
Author

commented Feb 5, 2013

@doughnukem. This is great! I'll give this a go. Thanks.

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2013

Yeah, npm purposefully excludes symlinks. We have an issue on it, I'll dig it up if anyone still cares.

@brianneisler

This comment has been minimized.

Copy link

commented Feb 20, 2013

@nathan7, would be awesome if you could dig up that issue. Running in to the same problem on my end.

@mmalecki

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2013

It'd be great to make this configurate in fstream-npm.

Assigning this to @nathan7 for figuring out and such.
Nathan, would be great if you could let us know if it'd be possible at all.

/cc @isaacs

@ghost ghost assigned edef1c Mar 7, 2013

lanterndev pushed a commit to getlantern/lantern-ui that referenced this issue Apr 3, 2013

_pants
@isaacs

This comment has been minimized.

Copy link

commented Apr 12, 2013

As long as you guys are using npm to create tarballs, it won't include symbolic links. They're not portable, and npm has to be portable.

It'd probably be good to avoid using npm pack and instead just build your own thing with node-tar.

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2013

@isaacs We pretty much want to mirror npm behaviour except for the symlinks and respecting .jitsuignore - I wonder what parts of npm we can reuse

@isaacs

This comment has been minimized.

Copy link

commented Apr 19, 2013

You probably just have to swap out fstream-npm for a similar fstream-jitsu or something. That's where those rules are set.

@3rd-Eden

This comment has been minimized.

Copy link
Member

commented Apr 19, 2013

If symlinks aren't portable, then why do we want to support them @mmalecki / @nathan7 , fixing this issue only partially is silly imho. Wouldn't it make more sense to maybe introduce another field in the package.json like symlinks where users key specify an object where the key is the location of the symlink and the value the path of the original file.. We can probably just resolve these in our module foundry and add it the snapshot?

Also during the packing we could just output a warning that the application contains symlinks and that our platform doesn't support these.

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2013

@3rd-Eden They are not portable to Windows. Every modern OS supports it. OS X, Linux, SmartOS. (heck, windows has something close)

@3rd-Eden

This comment has been minimized.

Copy link
Member

commented Apr 19, 2013

@nathan7 that's why i'm suggesting a different approach.

@isaacs

This comment has been minimized.

Copy link

commented Apr 19, 2013

Does nodejitsu's hosting stuff run on Windows?

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2013

@isaacs Nope, SmartOS.

@mmalecki

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2013

I do not think that we should care about Windows at all.
Also, if this feature proves problematic to implement, let's ditch it. I'm fine with going whatever npm thinks is fine.

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2013

@mmalecki I have an fstream-jitsu in progress (a fork of fstream-npm, we can pull in future upstream changes easily)

@indexzero

This comment has been minimized.

Copy link
Member

commented Apr 20, 2013

@mmalecki you are technically correct, the best kind of correct. Even if we were to hypothetically add Azure as a deployment target, we would run ontop of their Linux VMs.

The only concern I have is that the solution needs to be runnable on Windows clients (i.e. I should still be able to deploy from a Windows machine to nodejitsu).

@edef1c

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2013

@indexzero Unices have symlinks and we handle them incorrectly. Windows doesn't have symlinks, we won't ever encounter them, thus we're handling them correctly already there.

@mmalecki

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2013

I'm all for closing as WONTFIX and not forking upstream libraries.

@mmalecki mmalecki closed this Apr 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.