NodeJS_14 #2739
NodeJS_14 #2739
Conversation
|
Executables are searched by default in the |
|
I have no idea how this works, but perhaps all files need to be in the same directory, in case they reference each other? |
|
Yeah, looks like they've to some references to each other. They also reference |
|
Alternatively, set |
|
Or move everything in |
|
The main reason I opted against building node itself in NodeJS.jl was that I wanted it to work with pre-built binary node packages, and it was not clear to me whether that would work if we build node with a different compiler than the official build from the node webpage? For example, if we build it ourselves here, then we are using the default libc etc. libraries that Julia uses, right? And I was under the assumption that might then not work with prebuilt node binary packages because they might assume that the node they are running on would be using the versions of these libraries that the official node uses, which is presumably built with VS C++? Two other random points:
|
Yes, we'd probably run into issues doing that. I'm not planning on building from source here, just wrapping the official binaries probably gives the most benefit here for that specific reason.
Would definitely be handy to have a version like that.
Yes, that was my thinking as well, we do need access to different versions at times. I think that just going with the simple way of |
|
Ah, I didn't actually look at the PR, but now I see that you are also just repacking things, like I did in NodeJSBuilder! Yes, so I think for now that makes sense. I think down the road if we wanted to host Node in process we would actually need to build it ourselves. So maybe the right way to handle that is to just at that point create another jll package, say NostedNodeJS_jll that provides a self compiled version. That would allow us for now to ignore the entire issue :)
Yes, I like that a lot! Only problem I see right now is that NodeJS.jl then would require to have a dependency on ALL the major version jll packages, and they would all be downloaded at install, which seems not ideal... With artifacts we would have the option for the nice lazy stuff, but I guess with jll packages that is not in the cards... Not sure what the best solution for that is, maybe just to ignore? Re file locations: I would definitely first try to use the |
Exactly.
Hmm, yeah, being able to lazy download the artifacts would be nice in |
|
Couldn't find a way to conditionally set It also isn't currently picking up |
|
To have the artifacts lazy, you can pass the To have platform-dependent products... yeah, doing the |
The problem is the extension of the files, we expect |
|
But I feel like installing everything under |
I guess so, they're batch files so technically "executable" even though they're not binary.
Yeah, having done the |
Oh, wait, right, |
|
I guess the last decision to make here then is whether to go with a naming scheme of |
|
I must have missed why we can't have a single package with multiple versions. Does it make sense to have multiple node.js versions running at the same time in the same Julia environment? |
|
Up in #2739 (comment):
I've not come across the issue in practice myself though. |
|
A middle ground that I think we should just go with for now is merge this as |
|
Bump, anything else we need to sort out here? |
|
I was waiting for final comments from @davidanthoff 🙂 |
|
@MichaelHatherly @davidanthoff Quick question here...just started to look into what would be involved in enabling Apple Silicon support and it looks like, unfortunately, the first release with Darwin-arm64 builds is the 16.0 release. Does it make sense to add v16 support this to the PR as a separate file / as a loop over different version numbers? The plotting story for Apple Silicon and Julia is currently thoroughly broken (Plots, Makie, VegaLite) and I'd love to see VegaLite emerge as the first general plotting lib to support Apple Silicon. :) |
|
I'd be in favour of having separate
Yes, getting that sorted would be great. |
|
@MichaelHatherly Sent a PR your way to update the folder structure on this PR, bump node 14 to 14.16.1 |
Folder reorg, bump to 14.16.1
|
Thanks @jeremiahpslewis. |
| Platform("x86_64", "macos"), | ||
| Platform("x86_64", "windows"), | ||
| Platform("i686", "windows"), | ||
| ] |
There was a problem hiding this comment.
So I would just say it would be great if we could cover the same platforms here that I have here.
Also, I would suggest we add MacOS(:aarch64) and just package the x64 binaries into that artifact. They then work via rosetta, and folks on M1 can use it for now. And starting with node 16 we can then start to ship the native apple silicon binaries instead.
There was a problem hiding this comment.
Also, I would suggest we add MacOS(:aarch64) and just package the x64 binaries into that artifact.
Why is that necessary? If someone is using the x86-64 Julia binary with Rosetta there is no need to have a fake aarch64-apple-darwin artifact, the x86-64 would just work
There was a problem hiding this comment.
But we also want it to work for folks that are using the Apple Silicon Julia binaries :)
Co-authored-by: David Anthoff <anthoff@berkeley.edu>
| ArchiveSource("$(url_prefix)-linux-x64.tar.gz", "068400cb9f53d195444b9260fd106f7be83af62bb187932656b68166a2f87f44"; unpack_target = "x86_64-linux-musl"), | ||
| ArchiveSource("$(url_prefix)-linux-arm64.tar.gz", "58cb307666ed4aa751757577a563b8a1e5d4ee73a9fac2b495e5c463682a07d1"; unpack_target = "aarch64-linux-musl"), | ||
| ArchiveSource("$(url_prefix)-linux-armv7l.tar.gz", "54efe997dbeff971b1e39c8eb910566ecb68cfd6140a6b5c738265d4b5842d24"; unpack_target = "arm-linux-musleabihf"), |
There was a problem hiding this comment.
wait, these binaries are only for glibc I assume?
| products = [ | ||
| ExecutableProduct("node", :node), | ||
| FileProduct("bin/npm", :npm), | ||
| FileProduct("bin/npx", :npx), |
There was a problem hiding this comment.
Actually, I'm just realizing that this won't work: bin/npm and bin/npx are cygwin specific (I think). Instead, on Windows there are bin/npm.cmd and bin/npx.cmd batch files present that we should actually expose. But not sure how we can do that, i.e. use a different path for Windows here?
There was a problem hiding this comment.
Could this be the pattern we are looking for?
Probably need to change the node.js 16 code as well, once we figure this out
There was a problem hiding this comment.
Looking into it further, less convinced the above link helps
There was a problem hiding this comment.
I seem to recall https://github.com/JuliaPackaging/Yggdrasil/blob/master/fancy_toys.jl#L15 being useful for dealing with differences in platforms.
There was a problem hiding this comment.
@jeremiahpslewis btw I've given you write access to my fork so you can just push direct to this branch to make this easier to finish off.
There was a problem hiding this comment.
If you don't want bin/npm and bin/npx at all in the Windows tarballs you can't delete them and have a FileProduct with multiple names. But note that FileProducts don't provide much convenience besides a variable pointing to their path, it's not like an ExecutableProduct for which we generate wrapper functions, so you may just ignore these two products in the JLL package.
There was a problem hiding this comment.
I think what we want is a FileProduct named npm that points to bin/npm.cmd on Windows, but to bin/npm everywhere else. Same for npx.
It would actually be nice if one could use ExecutableProudct for batch files...
|
This doesn't seem relevant anymore since 16 is now LTS? |
|
(and #2869 was merged) |
|
Yes, let's just close this one. |
First, I'm aware of
NodeJS.jl😄. I think it might predate Yggdrasil perhaps, hence why it wasn't just written up as a build script here? This is mostly just to reduce the maintenance burden on @davidanthoff, if you don't mind?12.13.1to the latest LTS14.16.0.chmod +xappears to be needed (like it was with Kaleido) on Windows for Julia 1.6. I was gettingpermission denied (EACCES)when usingNodeJS.jlon windows CIs.ExecutableProduct("node", :node),wasn't picking up the binary on Windows when I was trying this locally, perhaps it'll work on CI. Or is there something special I need to do to correctly pick up.exes?