[Design]Use global.json and switch to netcoreapp2.2 #661

ryanbrandenburg · 2018-05-02T19:48:23Z

No description provided.

natemcmaster

A good start. Commented below with some more things to consider.

natemcmaster · 2018-05-02T21:05:25Z

files/KoreBuild/scripts/KoreBuild.psm1

@@ -431,7 +423,9 @@ function __get_dotnet_sdk_version {
        Write-Warning "dotnet SDK version overridden by KOREBUILD_DOTNET_VERSION"
        return $env:KOREBUILD_DOTNET_VERSION
    }
-    return Get-Content (Join-Paths $PSScriptRoot ('..', 'config', 'sdk.version'))


Since we don't generate global.json anymore, let's also remove usages of KOREBUILD_DOTNET_VERSION, both in .ps1 and .sh scripts.

natemcmaster · 2018-05-02T21:06:56Z

files/KoreBuild/scripts/common.sh

@@ -61,7 +61,8 @@ __ensure_macos_version() {

 __get_dotnet_sdk_version() {
    local src="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-    version=$(< "$src/../config/sdk.version" head -1 | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')
+
+    version=$(< "$src/../../../global.json" head -1 | grep -Po '"version":.*?[^\\]",' global.json)


This only works when building inside aspnet/BuildTools. You'll need to look for the global.json file in the repo root, not a relative path to this script.

Also, it would be better to use jq (and fallback to python) to read the json file. See

BuildTools/scripts/bootstrapper/run.sh

Lines 219 to 234 in d9eb8a5

if jq '.' "$config_file" >/dev/null ; then

config_channel="$(jq -r 'select(.channel!=null) | .channel' "$config_file")"

config_tools_source="$(jq -r 'select(.toolsSource!=null) | .toolsSource' "$config_file")"

else

__warn "$config_file is invalid JSON. Its settings will be ignored."

fi

elif __machine_has python ; then

if python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'))" >/dev/null ; then

config_channel="$(python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'));print(obj['channel'] if 'channel' in obj else '')")"

config_tools_source="$(python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'));print(obj['toolsSource'] if 'toolsSource' in obj else '')")"

else

__warn "$config_file is invalid JSON. Its settings will be ignored."

fi

else

__warn 'Missing required command: jq or pyton. Could not parse the JSON file. Its settings will be ignored.'

fi

for example.

natemcmaster · 2018-05-02T21:11:14Z

modules/KoreBuild.Tasks/UpgradeDependencies.cs

@@ -210,6 +215,12 @@ private int UpdateDependencies(DependencyVersionsFile localVersionsFile, Depende
                    newValue = KoreBuildVersion.Current;
                    Log.LogMessage(MessageImportance.Low, "Setting InternalAspNetCoreSdkPackageVersion to the current version of KoreBuild");
                }
+                else if (var.Key == "SdkPackageVersion")


Naming what about: DotNetCliVersion?

We should also add update logic for the "msbuild-sdks" section of the global.json file. See https://github.com/aspnet/BuildTools/tree/dev/src/Microsoft.DotNet.GlobalTools.Sdk#usage for example. We will need this more as we migrated Internal.AspNetCore.Sdk and others away from being a PackageReference and towards being proper MSBuild SDKs.

Also, maybe we should have this information stored separately. Dependencies.props was really only mean for PackageReference versions. Maybe use a new file named Tooling.props or something? Just a thought...haven't committed to the idea.

2 should be its own item right? Once I touch 50 repos this is already going to be a messy change, I'd really rather not complicate it further.

If by 2 you mean updating the "msbuild-sdks", then no, we need that before we can take this change. The msbuild-sdks section is required in at least 4 repos for the global tool shim stuff I just added yesterday.

natemcmaster · 2018-05-02T21:13:13Z

modules/KoreBuild.Tasks/Utilities/GlobalJsonFile.cs

+        {
+            var globalJson = JsonConvert.DeserializeObject<Dictionary<string, object>>(File.ReadAllText(_path));
+
+            var sdkDict = (Dictionary<string, object>)globalJson["sdk"];


Double check this section of the file exists first. It may not.

natemcmaster · 2018-05-02T21:13:28Z

modules/KoreBuild.Tasks/Utilities/GlobalJsonFile.cs

+
+        public void SetSdkVersion(string version)
+        {
+            var globalJson = JsonConvert.DeserializeObject<Dictionary<string, object>>(File.ReadAllText(_path));


Handle the case were the file may not exist at all.

natemcmaster · 2018-05-02T23:38:50Z

By the way, it might be good to take the netcoreapp2.2 update and global.json changes separately. I don't believe these need to be done at the same time. The global.json work is less urgent and has deeper implications on how KoreBuild functions.

Some unanswered questions we'll have to address:

How can we check for a min sdk version? If KoreBuild requires a minimum version of the CLI, how can we check and enforce that.
How can we isolate task dependencies? Today, KoreBuild's tasks are tied to the same version of dependencies that ship in MSBuild itself, like NuGet and JSON.NET. We need to break that dependency and isolate tasks in order to make the decoupling work. I suspect we can do this incrementally, but it's something we should have a plan for first.
How can we ensure we install required runtimes for tools? The console tools bundled in KoreBuild depend on a .NET Core runtime. How can we ensure this is installed? (Btw, this is addressed today by only using netcoreapp2.0 and the files/KoreBuild/config/runtime.version file. Updating to netcoreapp2.2 breaks that assumption)

natemcmaster · 2018-05-02T23:44:37Z

Sorry to pile it on, but one more question to consider:

Does aspnet/Universe need to ensure the same CLI version is used in all repos? If so, how will we override global.json? We do this today be re-generating global.json on each build, and overriding the korebuild-lock.txt file in aspnet/Universe.

ryanbrandenburg · 2018-05-03T16:56:05Z

Results from meeting: We're going to punt 1 and 2, our solution for 3 is to only have netcoreapp2.0 projects. As for the other comment we all agreed that Universe should enforce its SDK on all modules via replacing the global.json with its own (the same way we do for korebuild-lock.json)

ryanbrandenburg · 2018-05-17T19:00:51Z

This is stale.

ryanbrandenburg requested a review from natemcmaster May 2, 2018 19:48

ryanbrandenburg changed the title ~~Use global.json and switch to netcoreapp2.2~~ [Design]Use global.json and switch to netcoreapp2.2 May 2, 2018

natemcmaster reviewed May 2, 2018

View reviewed changes

natemcmaster mentioned this pull request May 2, 2018

[Design] Start using global.json and netcoreapp2.2 aspnet/Localization#442

Closed

ryanbrandenburg added 4 commits May 3, 2018 15:03

Use global.json and switch to netcoreapp2.2

736d8d0

Progress

d6aedf8

Progress

df1eddb

PR feedback

4a9f641

ryanbrandenburg force-pushed the rybrande/netcoreapp22 branch from 372f180 to 4a9f641 Compare May 3, 2018 22:04

Progress

49d6709

ryanbrandenburg closed this May 17, 2018

natemcmaster mentioned this pull request May 23, 2018

Need support for getting the SDK version number #679

Closed

natemcmaster mentioned this pull request Aug 9, 2018

Check in the global.json file, use this to control SDK versions #729

Closed

natemcmaster deleted the rybrande/netcoreapp22 branch October 19, 2018 19:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Design]Use global.json and switch to netcoreapp2.2 #661

[Design]Use global.json and switch to netcoreapp2.2 #661

ryanbrandenburg commented May 2, 2018

natemcmaster left a comment

natemcmaster May 2, 2018

natemcmaster May 2, 2018

natemcmaster May 2, 2018

ryanbrandenburg May 2, 2018

natemcmaster May 2, 2018

natemcmaster May 2, 2018

natemcmaster May 2, 2018

natemcmaster commented May 2, 2018

natemcmaster commented May 2, 2018

ryanbrandenburg commented May 3, 2018

ryanbrandenburg commented May 17, 2018

	if jq '.' "$config_file" >/dev/null ; then
	config_channel="$(jq -r 'select(.channel!=null) \| .channel' "$config_file")"
	config_tools_source="$(jq -r 'select(.toolsSource!=null) \| .toolsSource' "$config_file")"
	else
	__warn "$config_file is invalid JSON. Its settings will be ignored."
	fi
	elif __machine_has python ; then
	if python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'))" >/dev/null ; then
	config_channel="$(python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'));print(obj['channel'] if 'channel' in obj else '')")"
	config_tools_source="$(python -c "import json,codecs;obj=json.load(codecs.open('$config_file', 'r', 'utf-8-sig'));print(obj['toolsSource'] if 'toolsSource' in obj else '')")"
	else
	__warn "$config_file is invalid JSON. Its settings will be ignored."
	fi
	else
	__warn 'Missing required command: jq or pyton. Could not parse the JSON file. Its settings will be ignored.'
	fi

[Design]Use global.json and switch to netcoreapp2.2 #661

[Design]Use global.json and switch to netcoreapp2.2 #661

Conversation

ryanbrandenburg commented May 2, 2018

natemcmaster left a comment

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

ryanbrandenburg May 2, 2018

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

natemcmaster May 2, 2018

Choose a reason for hiding this comment

natemcmaster commented May 2, 2018

natemcmaster commented May 2, 2018

ryanbrandenburg commented May 3, 2018

ryanbrandenburg commented May 17, 2018