Skip to content

Move to .NET 5 preview.1 - #12140

Merged
Travis Plunk (TravisEz13) merged 12 commits into
PowerShell:masterfrom
SteveL-MSFT:dotnet-5
Mar 19, 2020
Merged

Move to .NET 5 preview.1#12140
Travis Plunk (TravisEz13) merged 12 commits into
PowerShell:masterfrom
SteveL-MSFT:dotnet-5

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented Mar 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Updated associated build files to netcoreapp5.0 and ref assemblies to 5.0-preview.1 where applicable. Updated files.wxs to changes.

PR Checklist

PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
Closed
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Mar 17, 2020

Copy link
Copy Markdown
Member Author

It looks like there's an issue in .NET 5 preview1 where we set the main thread name and that somehow gets propagated as the process name on Linux (only) and so tests that check if it's pwsh ends up failing. I'll see about putting in some reasonable workarounds for now and add a pending test to verify the process name.

PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
TravisEz13
Travis Plunk (TravisEz13) reviewed Mar 17, 2020
View reviewed changes

@TravisEz13 Travis Plunk (TravisEz13) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've looked at the changes so far

@TravisEz13 Travis Plunk (TravisEz13) added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Mar 17, 2020
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 17, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Mar 17, 2020

Copy link
Copy Markdown
Member Author

.NET 5 specifically has improvements to regex, so needed to increase the size of the DOS test

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Mar 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

On Windows, pwsh is crashing in a native API call. Opened dotnet/runtime#33703

@danmoseley

Dan Moseley (danmoseley) commented Mar 18, 2020

Copy link
Copy Markdown

.NET 5 specifically has improvements to regex, so needed to increase the size of the DOS test

cc Stephen Toub (@stephentoub) 👏

@danmoseley

Dan Moseley (danmoseley) commented Mar 18, 2020

Copy link
Copy Markdown

It looks like there's an issue in .NET 5 preview1 where we set the main thread name and that somehow gets propagated as the process name on Linux (only)

Steve Lee (@SteveL-MSFT) is that a bug in .NET?

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Mar 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

@danmosemsft The main thread name becoming the process name is a bug in .NET and is understood (dotnet/runtime#33673). That issue isn't blocking as it's currently only affecting tests and I made changes to work around it. The crashing issue (dotnet/runtime#33703) is preventing us from moving to .NET 5. Thanks!

@TravisEz13

Travis Plunk (TravisEz13) commented Mar 18, 2020

Copy link
Copy Markdown
Member

There is already an issues filed for regex DOS tests: #12138

PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 19, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 19, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
@SteveL-MSFT
Readjust webcmdlet regex test to account for perf improvement
0f8568a 
Remove assembly from startup validation
PoshChan
PoshChan-Bot (PoshChan) reviewed Mar 19, 2020
View reviewed changes
Comment thread .devcontainer/Dockerfile
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Mar 19, 2020

Copy link
Copy Markdown
Member Author

PoshChan-Bot (@PoshChan) retry macos

@PoshChan

PoshChan-Bot (PoshChan) commented Mar 19, 2020

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT), successfully started retry of PowerShell-CI-macOS

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 19, 2020
@TravisEz13

Travis Plunk (TravisEz13) commented Mar 19, 2020

Copy link
Copy Markdown
Member

.NET originally said that the perf improvement was due to optimization, but it's actually due to a bug. I filed an issue to add it back to avoid delaying this PR.
#12155

@stephentoub

Stephen Toub (stephentoub) commented Mar 19, 2020

Copy link
Copy Markdown
Contributor

.NET originally said that the perf improvement was due to optimization, but it's actually due to a bug

It's half and half :) The algorithmic complexity improved, in part due to perf improvements, and in part due to a bug fixed in Preview 2 (dotnet/runtime#33409).

@TravisEz13 Travis Plunk (TravisEz13) changed the title Move to .NET 5 preview.1 Move to .NET 5 preview.1 Mar 19, 2020
@TravisEz13
Travis Plunk (TravisEz13) merged commit 495dea0 into PowerShell:master Mar 19, 2020
@ghost

Deleted user (ghost) commented Mar 26, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

This was referenced Apr 7, 2020
Closed
Merged
@SteveL-MSFT
Steve Lee (SteveL-MSFT) deleted the dotnet-5 branch June 6, 2020 02:32
@xtqqczze xtqqczze mentioned this pull request Dec 9, 2020
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@SteveL-MSFT
Move to .NET 5 preview.1 (PowerShell#12140)
43a3405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 approved these changes
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
+1 more reviewer
@PoshChan PoshChan-Bot (PoshChan) PoshChan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@anmenaga Andrew (anmenaga)

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept

Projects

None yet

Milestone

7.1.0-preview.1

Development

Successfully merging this pull request may close these issues.

6 participants

@SteveL-MSFT @danmoseley @TravisEz13 @PoshChan @stephentoub @anmenaga