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

dotnet-install.sh no longer working with both --version and --quality specified #285

Closed
SteveL-MSFT opened this issue Jun 21, 2022 · 7 comments · Fixed by #286
Closed

dotnet-install.sh no longer working with both --version and --quality specified #285

SteveL-MSFT opened this issue Jun 21, 2022 · 7 comments · Fixed by #286

Comments

@SteveL-MSFT
Copy link
Contributor

SteveL-MSFT commented Jun 21, 2022

Description

Our bootstrapper script for PowerShell 7 has been specifying both the version and quality and has been working until a few days ago when it started returning an error. If I modify our script to pass just the --version and leave out --quality it works. This is a breaking change and the error message seems to indicate this is a bug and not intended behavior. Note that the .ps1 doesn't have this problem.

Reproduction Steps

./dotnet-install.sh -v 7.0.100-preview.5.22307.18 -q daily -skipnonversionedfiles

Expected behavior

dotnet should install the version specified

Actual behavior

dotnet_install: Error: Either Quality or Version option has to be specified. See https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#options for details.

Regression?

Yes, starting failing this week

Known Workarounds

Remove --quality and only specify --version. The PowerShell script dotnet-install.ps1 doesn't have this problem where we continue to specify both version and quality.

Configuration

7.0.100-preview.5.22307.18

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@SteveL-MSFT
Copy link
Contributor Author

This PR appears to have introduced a change that is causing the regression https://github.com/dotnet/install-scripts/pull/278/files

@SteveL-MSFT
Copy link
Contributor Author

SteveL-MSFT commented Jun 22, 2022

Here's the relevant section:

    if [[ "$normalized_version" != "latest" ]] && [ -n "$normalized_quality" ]; then
        say_err "Either Quality or Version option has to be specified. See https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#options for details."
        return 1
    fi

Based on the error message, the check is that either --version or --quality has to be specified. So it seems that this should be:

    if [[ -z "$normalized_version" && -z "$normalized_quality" ]]; then
        say_err "Either Quality or Version option has to be specified. See https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#options for details."
        return 1
    fi

@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Jun 22, 2022

Hi @SteveL-MSFT ,

Thank you for the detailed explanation and provided PR.
We consider your change .

@dagood
Copy link
Member

dagood commented Jun 22, 2022

Based on the error message, the check is that either --version or --quality has to be specified.

Looking at the issue linked in the PR description, this doesn't seem right to me:

The PowerShell version of the script has the check implemented the same way, so #286 is making them different.

I think "Either" is intended to clarify that "or" is exclusive, not inclusive, in "Either Quality or Version option has to be specified." ("But not both".)

Based on #247, it looks like specifying both --version and --quality isn't expected to work in the first place. And I think this makes sense: if you already have the version number, why is quality also being specified? At best, it seems to me that it's redundant, and otherwise, it contradicts the version number.

@bekir-ozturk
Copy link
Contributor

@dagood has pretty much nailed it (thank you!). I have created an issue so that we can revert the latest change: #287.

The bottom line is, if you specified both --quality and --version, the quality used to be ignored. Which meant that you may not have been getting the quality you asked for. With the recent update, you get an error instead which will hopefully help you write the right command that will give you the build you want.

If you want to keep the old behavior, please simply omit --quality.

@YuliiaKovalova
Copy link
Member

Hi @SteveL-MSFT,

As you can see from the description above, existing behavior is valid.
But we do agree, that message is misleading so it is changed PR: 288.
I expect to have it deployed tomorrow.

Sorry for delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants