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

feat: install nodejs into nodejs-prefix, and make it configurable #116

Merged
merged 2 commits into from
Sep 9, 2021

Conversation

jimisaacs
Copy link
Collaborator

Because it's easier to manage permissions when everything that is supposed to be owned by the server process is in the same place. Because the nodejs installation doesn't have any clear requirement to be where it is, figured it could be moved to the etc dir.

Also embedded an npmrc to be picked up by the installed nodejs. There isn't any example to override the npmrc to be embedded during the build process, but that's exactly how I plan do to it.

@ije
Copy link
Member

ije commented Sep 8, 2021

looks great! seems embedded npmrc is a bit limited, if you want to change the registry why not use ENV instead? let's say you are running the server in a docker container.

@jimisaacs
Copy link
Collaborator Author

@ije funny you should suggest that, I was was doing that, but I actually have quite a few namespaces I manage via npmrc, and managing via ENV got pretty unwieldy fast. Though if possible, I do like the idea of having the option for embedding with docker build, or use env vars.

@ije
Copy link
Member

ije commented Sep 8, 2021

@jimisaacs i prefer use configurable npmrc (path/env) instead of embedded npmrc.

@jimisaacs
Copy link
Collaborator Author

jimisaacs commented Sep 8, 2021

@ije Ah, maybe I misunderstood what you meant by ENV (i.e. NPM_* vars)

So my problem is, I'm not sure how you configure it like this. I know there's a hierarchy that goes from $prefix/etc/npmrc which is what I'm using, and $HOME/.npmrc, and then package level .npmrc which I don't think applies here.

Is there a way to configure npm to use an npmrc? Can you elaborate if you have something specific in mind?

@jimisaacs
Copy link
Collaborator Author

@jimisaacs
Copy link
Collaborator Author

@ije are you suggesting to have a configurable path to an npmrc that nodejs.go would copy into place at $PREFIX/etc/npmrc? Sorry, I'm probably jumping to conclusions at this point, will wait for your reply ;)

@jimisaacs
Copy link
Collaborator Author

Ok last comment before I wait. My server environment is running as a non-provisioned user, but if I literally define $HOME node, npm, and yarn all pick it up just fine. So I can use the $HOME/.npmrc for configuration.

That said, that means I can remove the embed portion of this PR, though I'm still wondering what you are thinking.

@ije
Copy link
Member

ije commented Sep 8, 2021

i see, then i suggest create a embed npmrc file dynamic in the build script, here is how i enable the china traffic splitting feature: https://github.com/alephjs/esm.sh/blob/9be43ace1f6b4bd1dbd98586d858e47db678fd05/scripts/build.sh#L9

@ije
Copy link
Member

ije commented Sep 8, 2021

but for docker, env-* maybe make more sence

@jimisaacs
Copy link
Collaborator Author

@ije I think I see what you are saying, but because I solved my issue without code changes, I was actually about to remove all things npmrc related from this, and just keep the nodePrefix stuff. Does that sound ok for now?

@ije
Copy link
Member

ije commented Sep 8, 2021

si, nodePrefix looks great 👍

@jimisaacs jimisaacs changed the title nodejs prefix & embed npmrc install nodejs into nodejs-prefix, and make it configurable Sep 8, 2021
@jimisaacs
Copy link
Collaborator Author

@ije ok, have a look at the new diff when you get a chance

@jimisaacs jimisaacs changed the title install nodejs into nodejs-prefix, and make it configurable feat: install nodejs into nodejs-prefix, and make it configurable Sep 8, 2021
@ije
Copy link
Member

ije commented Sep 9, 2021

looks great 👍

@ije ije merged commit a0e68c2 into esm-dev:master Sep 9, 2021
@jimisaacs jimisaacs deleted the npmrc branch September 10, 2021 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants