Solve .NET SDK version check failure when building PowerShell. by Fragmachine · Pull Request #17203 · PowerShell/PowerShell

Fragmachine · 2022-04-27T13:16:35Z

PR Summary

PowerShell fail to build even when the correct .NET SDK version is installed.

PR Context

There are two issues with how the .NET SDK version check works,

The regex parsing fail on certain versions due to unescaped dots.
When the supported SDK isn't the last entry in the generated list of installed SDK's it fails.

PR Checklist

…on was installed.

pull-request-quantifier-deprecated · 2022-04-27T13:16:40Z

This PR has 26 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Small
Size       : +16 -10
Percentile : 10.4%

Total files changed: 1

Change summary by file extension:
.psm1 : +16 -10

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

ghost · 2022-04-27T13:16:48Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ Fragmachine sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

daxian-dbw · 2022-04-27T22:42:19Z

@Fragmachine I guess changes here can be superseded by #17198, right?

Fragmachine · 2022-04-28T06:51:04Z

@daxian-dbw Yes, I've already talked to @powercode about it (we work at the same company). He solved the first issue I ran into but then I hit this other case with the SDK not being in the order the build script expected.

daxian-dbw · 2022-04-28T17:28:51Z

@Fragmachine So you mean this PR should supersede @powercode's #17198 then?

He solved the first issue I ran into but then I hit this other case with the SDK not being in the order the build script expected.

#17198 fixes the parsing, and also sort the results to get the highest version. I presume that resolves the other issue you ran into, right?

Fragmachine · 2022-04-28T19:44:19Z

Yes, this supersedes #17198. The reason being that 'Get-LatestInstalledSDK' indeed returns the latest which doesn't work if you have an SDK installed that's newer than the required. Essentially, there's a time gap between a new SDK version being released and PowerShell being updated to use it (I assume the intention is to stay close to the latest).

daxian-dbw · 2022-04-28T23:29:03Z

which doesn't work if you have an SDK installed that's newer than the required.

I believe that is intentional, because we want to make sure the exact version defined in global.json is used for the building.
It's not uncommon to run into issues when moving to a new .NET SDK, and many of them are regressions that cause PowerShell to malfunction.

If you are fine taking the risk for a private build to use a newer SDK, then you should just update the sdk version in global.json
in your local build to the version you are using.

Fragmachine · 2022-04-29T06:45:21Z

You misunderstand, this is not about building against another SDK older or newer. It's about being able to build with the appropriate SDK while having the others installed as well. As long as the correct version has priority i.e. the first one in PATH it works as it should.

daxian-dbw · 2022-04-29T16:44:23Z

Understood. Your change is basically to make sure the SDK with desired version can be found first from the path.

daxian-dbw · 2022-04-29T16:58:29Z

build.psm1

+function Get-DotNetVersionInPath {
    Start-NativeExecution -sb {
-        dotnet --list-sdks | Select-String -Pattern '\d*.\d*.\d*(-\w*\.\d*.\d*.\d*)?' | ForEach-Object { [System.Management.Automation.SemanticVersion]::new($_.matches.value) } | Sort-Object -Descending | Select-Object -First 1
+        dotnet --version


I don't think this will work. When running within the repo folder, global.json will be honored, so dotnet --version will returns messages like the following when the dotnet version doesn't match:

PS:15> dotnet --version Could not execute because the application was not found or a compatible .NET SDK is not installed. Possible reasons for this include: * You intended to execute a .NET program: The application '--version' does not exist. * You intended to execute a .NET SDK command: A compatible installed .NET SDK for global.json version [7.0.100-preview.2.22153.17] from [C:\arena\source\PowerShell\global.json] was not found. Install the [7.0.100-preview.2.22153.17] .NET SDK or update [C:\arena\source\PowerShell\global.json] with an installed .NET SDK: 5.0.402 [C:\Program Files\dotnet\sdk] 6.0.202 [C:\Program Files\dotnet\sdk]

I'm afraid we still need dotnet --list-sdks, but instead of returning the highest version, we need to check if the returned versions contain the required version. So, I think part of the changes in #17198 that fixed the parsing is still needed.

daxian-dbw · 2022-05-10T02:01:04Z

Superseded by #17299

Fragmachine added 2 commits April 26, 2022 10:18

Fix 'Start-PSBootstrap' error when processing .net sdk paths.

feebf35

Fix problem with failing to build although the correct .NET SDK versi…

db50f20

…on was installed.

Fragmachine requested review from PaulHigin, TravisEz13, adityapatwardhan, anmenaga and daxian-dbw as code owners April 27, 2022 13:16

ghost assigned TravisEz13 Apr 27, 2022

pull-request-quantifier-deprecated bot added the Extra Small label Apr 27, 2022

daxian-dbw reviewed Apr 29, 2022

View reviewed changes

daxian-dbw added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label May 4, 2022

daxian-dbw mentioned this pull request May 10, 2022

Fix build.psm1 to find the required .NET SDK version when a higher version is installed #17299

Merged

13 tasks

daxian-dbw closed this May 10, 2022

Conversation

Fragmachine commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

pull-request-quantifier-deprecated bot commented Apr 27, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

ghost commented Apr 27, 2022

Uh oh!

daxian-dbw commented Apr 27, 2022

Uh oh!

Fragmachine commented Apr 28, 2022

Uh oh!

daxian-dbw commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fragmachine commented Apr 28, 2022

Uh oh!

daxian-dbw commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fragmachine commented Apr 29, 2022

Uh oh!

daxian-dbw commented Apr 29, 2022

Uh oh!

daxian-dbw Apr 29, 2022

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxian-dbw commented May 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fragmachine commented Apr 27, 2022 •

edited

Loading

daxian-dbw commented Apr 28, 2022 •

edited

Loading

daxian-dbw commented Apr 28, 2022 •

edited

Loading

daxian-dbw Apr 29, 2022 •

edited

Loading