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

Support aliases like LTS #26

Closed
damccorm opened this issue Aug 9, 2019 · 104 comments
Closed

Support aliases like LTS #26

damccorm opened this issue Aug 9, 2019 · 104 comments
Labels
enhancement New feature or request

Comments

@damccorm
Copy link
Contributor

damccorm commented Aug 9, 2019

Right now, we're thinking it makes sense to support lts, latest, current, and LTS version names (e.g. dubinium)

@wesleytodd
Copy link

wesleytodd commented Aug 16, 2019

nodejs/package-maintenance#236

For reference, this is the alias list we are working out.

@JimiC
Copy link

JimiC commented Sep 5, 2019

@damccorm Do you mind if I give this a try to provide a PR?

@damccorm
Copy link
Contributor Author

damccorm commented Sep 6, 2019

@JimiC that would be great! One thing to note is that we would like to make this completely automated (e.g. it shouldn't rely on hardcoding dubinium to 10.16).

I've done a small bit of digging here, here are some things that might be helpful (apologies if you already know any/all of this):

@panva
Copy link
Contributor

panva commented Sep 6, 2019

this may be useful as a reference for using the dist json file.

@JimiC
Copy link

JimiC commented Sep 6, 2019

@damccorm Great!
I had a look at the code and how you are fetching the node version and have an idea on how to tackle this issue. Before I start coding I'll present you my course of actions, in order for everyone to agree on it.

@ljharb
Copy link

ljharb commented Sep 6, 2019

You may also want to reference how https://github.com/nvm-sh/nvm parses that very tab file to support its LTS aliases.

@JimiC
Copy link

JimiC commented Sep 9, 2019

After a lot of thought and searching the docs of popular CI services (Azure Pipelines, AppVeyor, TravisCI, GitLab CI/CD, CircleCi) I come to the conclusion that the following should be supported:

# shorthand of the latest version of node
- node:
    alias:
        - ' '
        - current (?)
        - latest  (?)

# shorthand of node version 4
- argon:
    alias:
        - Argon

# shorthand of node version 6
- boron:
    alias:
        - Boron

# shorthand of node version 8
- carbon:
    alias:
        - Carbon

# shorthand of node version 10
- dubnium:
    alias:
        - Dubnium

# shorthand of node version 12
- erbium (?):
    alias:
        - Erbium

Those with a question mark are cases that I like to discuss with you.

  • node is used as a shorthand in TravisCI and ' ' in AppVeyor. Should we also support those or are we going with our own shorthand like latest?
  • Erbium is pending and can't be yet automated. I'm just mentioning it here as a placeholder.

From my POV lts/* should not be a choice, since the GitHub (Azure Pipelines) matrix.node-version property actually executes an iteration of the values specified, passing a single value of the version each time to create an instance.
lts/* resolves to multiple node versions that can't be interpreted into firing up multiple instances.

Similarly lts=dubnium should not be a choice either, simply specifying dubnium should suffice.

I'm not sure if lts should be a choice either. Which node version should lts point to?

@damccorm
Copy link
Contributor Author

damccorm commented Sep 9, 2019

This generally looks great. A couple thoughts.

I think the idea of lts is that its the current lts. So right now, it would resolve to 10.x, but soon will resolve to 12.x. So lts and current are actually interchangeable here, while latest could resolve to a minor version (e.g. 13.x eventually). As long as we doc what we're doing there I think we'll be fine, the call from our product team based on customer conversations was supporting all 3 of these. I'd stick with these rather than go with a different placeholder, I think its a little more explicit this way.

Agreed on using dubinium instead of lts=dubinium.

Agreed on holding off on Erbium for now.

@JimiC
Copy link

JimiC commented Sep 9, 2019

One more clarification. According to https://nodejs.org/en/

  • lts -> 10.x
  • current -> 12.x which should also be resolved for shorthands ' ', node and latest

Correct?

Note: https://nodejs.org/en/about/releases/

@ljharb
Copy link

ljharb commented Sep 9, 2019

Is there a reason not to use nvm, and get all these conventional aliases for free?

@JimiC
Copy link

JimiC commented Sep 9, 2019

If only nvm for windows was the same as for *nix, Been wanting to start such a project but time is against me.

@damccorm
Copy link
Contributor Author

damccorm commented Sep 9, 2019

@JimiC yeah, you're right. I don't think we should resolve shorthands ' ' and node though. Those are pretty non-descriptive, so if you look at it and don't know what's going on it won't help you.

@JimiC
Copy link

JimiC commented Sep 9, 2019

OK. So we will dump the travis and appveyor shorthands and go with our own latest, current and lts ones.

Summarizing:

# shorthand of the latest version of node
# (as of the time of writing it's `12.x`)
- latest:
    alias:
        - current

# shorthand of the recommended `lts` version of node
# (as of the time of writing it's `10.x`)
- lts

# shorthand of node version 4.x
- argon:
    alias:
        - Argon

# shorthand of node version 6.x
- boron:
    alias:
        - Boron

# shorthand of node version 8.x
- carbon:
    alias:
        - Carbon

# shorthand of node version 10.x
- dubnium:
    alias:
        - Dubnium

# shorthand of node version 12.x
- erbium:
    alias:
        - Erbium

Note: As soon as Erbium is included in the index.json then it will be automatically supported.

@JimiC
Copy link

JimiC commented Sep 9, 2019

Something else that caught my eye while I was experimenting with jobs is that if you specify matrix.node-version: '' in strategy the setup-node resolves it to 10.x mainly because of this https://github.com/actions/setup-node/blob/master/action.yml#L10

Should this resolve be kept? It's not documented and may create support.

@damccorm
Copy link
Contributor Author

damccorm commented Sep 9, 2019

So that's a bit of a tricky question. I think I'd actually be in favor of cutting that. My only concern is that it might break some existing workflows, but that also enables you to not specify a version if you just want to do auth. Lets cut it.

@ljharb
Copy link

ljharb commented Sep 9, 2019

You could use nvm for nonwindows, and write an adapter for the specifier to map to whatever you use for windows - that way you’d match the overwhelmingly most common conventions.

@JimiC
Copy link

JimiC commented Sep 9, 2019

And why would I want to do that when I have an entire underlying code-base. Good thought but for another project, maybe?

@ljharb
Copy link

ljharb commented Sep 10, 2019

@JimiC to be clear, i meant this action/repo could do that for you - not that you’d have or want to do that locally.

@JimiC
Copy link

JimiC commented Sep 10, 2019

@ljharb Still don't get the logic behind your suggestion, but then again it's just me. A use case may help me understand.
@damccorm Even if the user wants to do just auth doesn't this mean that eventually (s)he will need npm?
Bare with me today as I have a bad headache.

@ljharb
Copy link

ljharb commented Sep 10, 2019

@JimiC what i mean is, since this action is controlling the mechanism for managing node, it can pretend to use nvm on non-WSL Windows by manually mapping nvm's versionish syntax to whatever windows tool you're choosing (or do something like, nvm version-remote ${versionish} in WSL and then pass that resolved version to the windows node version manager)

@damccorm
Copy link
Contributor Author

damccorm commented Sep 10, 2019

Even if the user wants to do just auth doesn't this mean that eventually (s)he will need npm?

Yes, but node/npm are installed by default. This action is responsible for making sure the correct version is used, but we do guarantee that a version will be installed. Right now, that's node 10.16.3 and npm 6.10.3 (at least on ubuntu) - that could change and there's no guarantee of which version will be installed

@JimiC
Copy link

JimiC commented Sep 10, 2019

@damccorm Let's reassess.
Q:

Something else that caught my eye while I was experimenting with jobs is that if you specify matrix.node-version: '' in strategy the setup-node resolves it to 10.x mainly because of this https://github.com/actions/setup-node/blob/master/action.yml#L10

Should this resolve be kept? It's not documented and may create support.

Ans:

So that's a bit of a tricky question. I think I'd actually be in favor of cutting that. My only concern is that it might break some existing workflows, but that also enables you to not specify a version if you just want to do auth. Lets cut it.

Removing default from action.yml and if a user sets node-version: '' the action will fail. Shouldn't we handle that?

@JimiC
Copy link

JimiC commented Sep 10, 2019

@ljharb Correct me if I'm wrong. What you are suggesting is that setup-node should have a similar syntax like nvm (i.e nvm install --lts=dubmium, nvm install --lts). Right?
Like node-version: --lts or node-version: --lts=dubnium?

@ljharb
Copy link

ljharb commented Sep 10, 2019

@JimiC lts/* or lts/dubnium, yes :-) the -- form is only for the command line, not for config files.

@maxim-lobanov
Copy link
Collaborator

maxim-lobanov commented Jun 30, 2021

Hello everyone!
We have merged #270 recently and released actions/setup-node@v2.2.0.
It brings support for NVM LTS syntax to actions/setup-node.

A few examples of syntax that is supported now:

  • major versions: 12, 14, 16
  • more specific versions: 10.15, 14.2.0, 16.3.0
  • nvm LTS syntax:
    • specific LTS alias: lts/erbium, lts/fermium
    • latest LTS version: lts/*

We will be very appreciate for your testing of these changes.
Also we will update v2 tag to point to v2.2.0 in a few days.

@mightyiam
Copy link

mightyiam commented May 31, 2022

Thank you for your work.

Can "all supported major Node.js versions" be expressed using the current feature set, please?

@IvanZosimov
Copy link
Contributor

IvanZosimov commented Jun 1, 2022

Hi, @mightyiam 👋 Thanks for your comment, could you add some context to your question, please?

@mightyiam
Copy link

mightyiam commented Jun 1, 2022

Sure. I'd like to test in CI against all currently supported major versions of Node.js. And it would be nice if I didn't have to occasionally bump explicitly specified versions. That's all, really.

@JimiC
Copy link

JimiC commented Jun 1, 2022

@mightyiam Doesn't this suffice?

@mightyiam
Copy link

mightyiam commented Jun 1, 2022

@mightyiam Doesn't this suffice?

What syntax exactly? Node.js currently has three supported releases. How do I target the "maintenance LTS"?

@JimiC
Copy link

JimiC commented Jun 1, 2022

I guess your are not familiar with nvm. lts/* should do the trick.

@mightyiam
Copy link

mightyiam commented Jun 1, 2022

I guess your are not familiar with nvm. lts/* should do the trick.

According to nvm, lts/* expands into the latest major Node.js LTS release, not the maintenance one. Node.js has two supported LTS releases currently: https://nodejs.org/en/about/releases/. There's a maintenance one and an active one. I'm looking for something that might look like LTS/maintenance.

@tooomm
Copy link
Contributor

tooomm commented Jun 1, 2022

Node.js currently has three supported releases. How do I target the "maintenance LTS".

With #481 you should be able to solve this with lts/-1.

To "test in CI against all currently supported major versions of Node.js", you would use lts/-1, lts/* and current.

@mightyiam
Copy link

mightyiam commented Jun 1, 2022

Node.js currently has three supported releases. How do I target the "maintenance LTS".

With #481 you should be able to solve this with lts/-1.

To "test in CI against all currently supported major versions of Node.js", you would use lts/-1, lts/* and current.

Yes, that should do. I'll keep an eye on that. Thanks, everyone.

@JimiC
Copy link

JimiC commented Jun 1, 2022

If I'm not mistaken lts/* returns v14 (which is the maintenance LTS) and v16 (which is the active LTS). Maybe you should elaborate on what exactly you are trying to do.

@tooomm
Copy link
Contributor

tooomm commented Jun 1, 2022

If I'm not mistaken lts/* returns v14 (which is the maintenance LTS) and v16 (which is the active LTS).

Not sure. https://github.com/nvm-sh/nvm#long-term-support states:
You can reference LTS versions in aliases and .nvmrc files with the notation 'lts/*' for the latest LTS.

@dominykas
Copy link

dominykas commented Jun 1, 2022

@mightyiam you could also try something like https://github.com/pkgjs/action which will automatically construct the test matrix and call setup-node for you.

@ljharb
Copy link

ljharb commented Jun 1, 2022

@JimiC you are mistaken. lts/* returns the latest LTS only. You can use lts/-1 etc, or lts/argon, to target a specific maintenance line, but anything that depends on support dates is not part of nvm - nvm’s aliases do not change with the passage of time, only with the release of new node versions.

@JimiC
Copy link

JimiC commented Jun 1, 2022

@JimiC you are mistaken. lts/* returns the latest LTS only. You can use lts/-1 etc, or lts/argon, to target a specific maintenance line, but anything that depends on support dates is not part of nvm - nvm’s aliases do not change with the passage of time, only with the release of new node versions.

@ljharb Thanks for correcting me. Honestly I never use lts/* in my CI/CD as I like to have the control on what node version I'm targeting.

@msimerson
Copy link

msimerson commented Jun 1, 2022

Sure. I'd like to test in CI against all currently supported major versions of Node.js. And it would be nice if I didn't have to occasionally bump explicitly specified versions. That's all, really.

That's exactly what I wanted, which is what I created this node-LTS-versions for. Give it a try.

@mightyiam
Copy link

mightyiam commented Jun 2, 2022

Sure. I'd like to test in CI against all currently supported major versions of Node.js. And it would be nice if I didn't have to occasionally bump explicitly specified versions. That's all, really.

That's exactly what I wanted, which is what I created this node-LTS-versions for. Give it a try.

That works for me. Thank you. Everyone.

@dmitry-shibanov
Copy link
Contributor

dmitry-shibanov commented Jun 7, 2022

Hello everyone. We released a new version of the action with support of lts/-n aliases. The setup-node action supports such syntax.

For now I'm going to close the issue. If you any updates feel free to ping us or create a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.