Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Update the SDK muxer behavior #4118

Merged
merged 4 commits into from
May 4, 2018
Merged

Update the SDK muxer behavior #4118

merged 4 commits into from
May 4, 2018

Conversation

johnbeisner
Copy link

Update the SDK muxer behavior; modify the SDK muxer tests.

Prior SDK muxer behavior:

  • If no global.json file is found, or the global.json doesn't contain a specified SDK version, the latest installed SDK version is used. Latest SDK version can be either release or pre-release - the highest version number wins.
  • If global.json does contain a specified SDK version:
    • If the specified SDK version is found on the machine, that exact version is used.
    • If the specified SDK version cannot be found on the machine, the latest installed SDK patch-version of the specified SDK version is used. Latest installed SDK patch-version can be either release or pre-release - the highest version number wins.
    • If the specified SDK version and an appropriate SDK patch-version cannot be found, an error is thrown.

Newer SDK muxer behavior:

  • If no global.json file is found, or the global.json doesn't contain a specified SDK version, the latest installed SDK version is used. Latest SDK version can be either release or pre-release - the highest version number wins.
  • If global.json does contain a specified SDK version:
    • If the specified SDK version is found on the machine, that exact version is used.
    • If the specified SDK version cannot be found on the machine, the latest installed SDK patch-version of the specified SDK version which is semantically greater than the specified SDK version is used. Selected SDK patch-version can be either release or pre-release.
    • If the specified SDK version and an appropriate SDK patch-version cannot be found, an error is thrown.

@davidebbo
@dotnet/dotnet-cli

Related: https://github.com/dotnet/core-setup/issues/3805

@johnbeisner johnbeisner self-assigned this May 2, 2018
nguerrera
nguerrera reviewed May 2, 2018
View reviewed changes
src/corehost/cli/fxr/fx_muxer.cpp Outdated
(ver.get_major() == specified.get_major() && ver.get_minor() == specified.get_minor() &&
(ver.get_patch() / 100) == (specified.get_patch() / 100)))
(ver.get_patch() / 100) == (specified.get_patch() / 100) && (ver == std::max(ver, specified))))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/corehost/cli/fxr/fx_muxer.cpp Outdated
// Pick the greatest version that differs only in the 'minor-patch' if a global cli version is specified.
// If a global cli version is specified:
// pick the greatest version that differs only in the 'minor-patch'
// and is semantically greater than or equal to the global cli version specified.

This comment was marked as spam.

Sign in to view

@nguerrera
Copy link

Is master the right branch for this? I would hope that we can get it fixed in 2.1.

@johnbeisner
Copy link
Author

@jaredpar
@KathleenDollard
@steveharter
@mairaw
@davidebbo
@dotnet/dotnet-cli
Can I get an approval on this PR; or further discussion would be welcomed.

@jaredpar
Copy link
Member

jaredpar commented May 4, 2018

LGTM

I'm hesistant to actually submit an approve review. The code looks good to me but I'm not very familiar with the contents of this repository. But to my eye it looks good.

@nguerrera
Copy link

I've approved, but it would be good for @steveharter or @eerhardt to look as well.

@eerhardt
Copy link
Member

eerhardt commented May 4, 2018

Is master the right branch for this? I would hope that we can get it fixed in 2.1.

The process in the dotnet/core* repos is to first get the PR into master, and then once approved in shiproom, port/cherry-pick the change into the appropriate release branch.

@nguerrera
Copy link

nguerrera commented May 4, 2018 •
edited
Loading

The process in the dotnet/core* repo

@eerhardt Ah yes. I always forget that.

@johnbeisner johnbeisner merged commit 3294cd2 into dotnet:master May 4, 2018
@KathleenDollard KathleenDollard mentioned this pull request Jun 21, 2018
Merged
@johnbeisner johnbeisner deleted the UpdateSDKMuxer branch December 11, 2018 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nguerrera nguerrera nguerrera approved these changes

@eerhardt eerhardt eerhardt approved these changes

@dsplaisted dsplaisted Awaiting requested review from dsplaisted

@jaredpar jaredpar Awaiting requested review from jaredpar

@livarcocc livarcocc Awaiting requested review from livarcocc

@KathleenDollard KathleenDollard Awaiting requested review from KathleenDollard

@steveharter steveharter Awaiting requested review from steveharter

@mairaw mairaw Awaiting requested review from mairaw

Assignees

@johnbeisner johnbeisner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@johnbeisner @nguerrera @jaredpar @eerhardt