Skip to content

Support multiple versions in single invocation #240

Merged
MaksimZhukov merged 22 commits intoactions:mainfrom
Xlient:support-multiple-versions
Nov 23, 2021
Merged

Support multiple versions in single invocation #240
MaksimZhukov merged 22 commits intoactions:mainfrom
Xlient:support-multiple-versions

Conversation

@Xlient
Copy link
Copy Markdown
Contributor

@Xlient Xlient commented Oct 21, 2021

Description:
Adds support to install multiple versions of .NET with a single invocation. Specified using a multiline string
Example :

- uses: actions/checkout@v2
- uses: actions/setup-dotnet@v1
  with:
    dotnet-version: |
       3.1.x
       5.0.x
- run: dotnet build <my project>

Related issue:
fixes #146

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

 removed  ver 5.0 from multiple versions  test
* Updated README & action.yml to reflect changes
* modified verify  ps1 test
@GGG-KILLER
Copy link
Copy Markdown
Contributor

GGG-KILLER commented Oct 21, 2021

I believe it would be better if this used YAML arrays instead of a comma separated string.

Given that YAML has builtin support for arrays, it is a bit counterintuitive to use a comma-separated string instead of YAML arrays imo.

@Xlient
Copy link
Copy Markdown
Contributor Author

Xlient commented Oct 21, 2021

I believe it would be better if this used YAML arrays instead of a comma separated string

Hello@GGG-KILLER, while I agree it would be more ideal to use an array, I don't believe the actions toolkit supports getting input as an array of strings with the YAML syntax.

@GGG-KILLER
Copy link
Copy Markdown
Contributor

GGG-KILLER commented Oct 21, 2021

I believe it would be better if this used YAML arrays instead of a comma separated string

Hello@GGG-KILLER, while I agree it would be more ideal to use an array, I don't believe the actions toolkit supports getting input as an array of strings with the YAML syntax.

True, after checking the other setup-* actions, it seems the standard method of testing against multiple versions is to use matrix builds.

Found another action that accepts multiple inputs and comma separated seems to be standard practice.

* Added seperate input for multiple versions and subsequent logic to handle
  multiple versions from new input
* updated test  for multiple versions logic in installer.test
*Updated README
@GGG-KILLER
Copy link
Copy Markdown
Contributor

Just my 2 cents, but wouldn't it be better to just have a single dotnet-version input and use getMultilineInput on that?

I feel like having two differently named options for the same thing (one accepting a single input and the other accepting multiple) is a bit counter intuitive.

Also, on another note, the multiple version test and README are still using dotnet-version instead of dotnet-versions

@Xlient
Copy link
Copy Markdown
Contributor Author

Xlient commented Oct 27, 2021

Also, on another note, the multiple version test and README are still using dotnet-version instead of dotnet-versions

Thanks for catching that 👍🏾

@vsafonkin vsafonkin added the feature request New feature or request to improve the current logic label Nov 1, 2021
@vsafonkin
Copy link
Copy Markdown

vsafonkin commented Nov 9, 2021

Hi @Xlient, maybe it will be better to create dotnetInstaller object for each dotnet version? In this case we should change nothing in installer.ts logic. I mean something like this:

if (versions) {
      const includePrerelease: boolean =
        (core.getInput('include-prerelease') || 'false').toLowerCase() === 'true';

      let dotnetInstaller: installer.DotnetCoreInstaller;
      for (const version of versions) {
          dotnetInstaller = new installer.DotnetCoreInstaller(
              version,
              includePrerelease
          );
          await dotnetInstaller.installDotnet();
      }
}

@Xlient
Copy link
Copy Markdown
Contributor Author

Xlient commented Nov 9, 2021

Hello @vsafonkin, I've updated setup-dotnet.ts and tests to reflect your suggested refactorings

@zcsizmadia zcsizmadia mentioned this pull request Nov 9, 2021
2 tasks
@vsafonkin
Copy link
Copy Markdown

@Xlient, great, thank you!

@MaksimZhukov
Copy link
Copy Markdown
Contributor

I think we should change a bit the action.yml and README:

description: 'Optional SDK version to use. If not provided, will install global.json version when available. Examples: 2.2.104, 3.1, 3.1.x'

-->

description: 'Optional SDK version(s) to use. If not provided, will install global.json version when available. Examples: 2.2.104, 3.1, 3.1.x'

and

optionally downloading and caching a version of dotnet by SDK version and adding to PATH

-->

optionally downloading and caching version(s) of dotnet by SDK version(s) and adding to PATH

@GGG-KILLER
Copy link
Copy Markdown
Contributor

GGG-KILLER commented Nov 11, 2021

optionally downloading and caching a version of dotnet by SDK version and adding to PATH

-->

optionally downloading and caching version(s) of dotnet by SDK version(s) and adding to PATH

Maybe the caching part should be removed as well? Seems like it hasn't been working since v1.7.0 (see #141)

- updated README & action.yml
- moved addToPath -> getDotet in installer.test
- add logic to remove duplicate sdk in verification test
 Please enter the commit message for your changes. Lines starting
fixed verification test
@MaksimZhukov MaksimZhukov merged commit 5507021 into actions:main Nov 23, 2021
@RehanSaeed
Copy link
Copy Markdown

How does this work in conjunction with global.json?

I currently use global.json to install a current version and separately install older versions by hardcoding a version.

@vsafonkin
Copy link
Copy Markdown

Hi @RehanSaeed, if you have dotnet-version input in your workflow with multiple versions, only these versions will be installed. The version from global.json will be installed if the dotnet-version input is empty. In your case, I suppose, you can use following:

- name: Setup dotnet
  uses: actions/setup-dotnet@v1.9.0
          
- name: Setup dotnet
  uses: actions/setup-dotnet@v1.9.0
    with:
      dotnet-version: |
        5.0.x
        6.0.x

It installs version from global.json and other required versions from the second invocation of action.

@Xlient Xlient deleted the support-multiple-versions branch November 30, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request to improve the current logic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multiple versions in a single invocation

8 participants