Skip to content

Fix SDK version parsing from global.json in install script on macOS #12339

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

Closed

Conversation

kondratyev-nv
Copy link

Running install script scripts/obtain/dotnet-install.sh with jsonfile argument works on macOS only when sdk section has a single property version. When sdk section has multiple properties, for example,

"sdk": {
    "version":  "5.0.100-preview.6.20310.4",
    "allowPrerelease": true,
    "rollForward": "major"
  }

a version is detected as 5.0.100-preview.6.20310.4allowPrerelease:truerollForward:major.
Also, in a case when version is not the first property in sdk a version can not be detected returning an error dotnet_install: Error: Unable to find the SDK:version node in `global.json` .

The problem seems to be in line:

sdk_list="$(echo -e "${sdk_list}" | tr -d '[[:space:]]')"

On macOS, echo does not have a -e argument, and all lines of key-value pairs in sdk_list are merged into a single line. So, removing it seems to fix it. I don't have a Linux to test the changes, but I hope CI will catch any issues on Linux.

@dnfadmin
Copy link

dnfadmin commented Jul 6, 2020

CLA assistant check
All CLA requirements met.

@kondratyev-nv kondratyev-nv force-pushed the fix_install_script_macos branch from 0a47164 to 961a198 Compare July 6, 2020 09:38
@marcpopMSFT marcpopMSFT requested a review from donJoseLuis July 15, 2020 21:44
@marcpopMSFT
Copy link
Member

@donJoseLuis does the install script change needed in the install-scripts repo and should we remove the copy from this repo to reduce confusion?

@marcpopMSFT
Copy link
Member

Ping @donJoseLuis

@kondratyev-nv
Copy link
Author

@marcpopMSFT Thanks for the help! I've created an issue in install-scripts repo dotnet/install-scripts#61 concerning that problem. Unfortunately, I wan not able to create a similar PR in install-scripts repo, so I'll wait for a response to the issue.

@marcpopMSFT
Copy link
Member

@bozturkMSFT Should he close this and do all of the work in the install-scripts repo?

@kondratyev-nv kondratyev-nv deleted the fix_install_script_macos branch August 20, 2020 18:08
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.

3 participants