Skip to content

Set skip-non-versioned-files#8208

Merged
alexperovich merged 1 commit intodotnet:mainfrom
alexperovich:skipNonVersioned
Nov 30, 2021
Merged

Set skip-non-versioned-files#8208
alexperovich merged 1 commit intodotnet:mainfrom
alexperovich:skipNonVersioned

Conversation

@alexperovich
Copy link
Member

To double check:

architecture=$3
fi
InstallDotNet "$root" "$version" $architecture 'sdk' 'false' $runtime_source_feed $runtime_source_feed_key
InstallDotNet "$root" "$version" $architecture 'sdk' 'true' $runtime_source_feed $runtime_source_feed_key
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need at least one caller to install dotnet? I was imagining that the SDK would install it but runtimes would not. Looks to me like the default is already true here:

InstallDotNet $dotnetRoot $version "$architecture" $runtime true $runtimeSourceFeed $runtimeSourceFeedKey || {

I wonder why the SDK ends up overwriting? cc @joeloff

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked around, but nobody knows. It was before I joined the SDK, but no one recalls any specific reason (I assume you're referring to the host getting overwritten and causing problems on OSx)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was set to true in one place, and this one was false. I made them both true. It will install if it doesn't exist so this change just means the first sdk installed will produce the dotnet host exe.

@alexperovich alexperovich merged commit d80b16b into dotnet:main Nov 30, 2021
@alexperovich alexperovich deleted the skipNonVersioned branch November 30, 2021 20:53
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.

4 participants