Rebase onto .NET Core 1.1 by randomvariable · Pull Request #2737 · PowerShell/PowerShell

Naadir Jeewa (randomvariable) · 2016-11-20T04:59:07Z

Build.psm1 and project.json files modified to use .NET Core 1.1.

.NET Core 1.1 ships with an older dotnet/cli than has currently been used,
so we revert to use case-sensitive directory names for dependencies.

.NET Core 1.1 is a pre-requisite for supporting Fedora 24.

Microsoft Contribution License Agreements (msftclas) · 2016-11-20T04:59:11Z

Hi Naadir Jeewa (@randomvariable), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

sergei (vors) · 2016-11-20T07:26:23Z

.NET Core 1.1 ships with an older dotnet/cli than has currently been used

That should not really matter, although, I think it could be acceptable to revert back for consistency.
Since we are not using nightly dotnet cli anymore.
Last pinned version of dotnet cli was completely random and it's probably better to stick with the one that corresponds to some release.

sergei (vors) · 2016-11-20T07:29:59Z

Please, use separate PRs (or at least separate commits) for such formatting cleanups. Mixing them makes review much harder.

Thanks. Missed when diffing.

Naadir Jeewa (randomvariable) · 2016-11-20T14:02:47Z

Appears the build in #2738 that I've ended up with doesn't allocate a tty, so is pretty unusable as an interactive shell. Not sure why.

Naadir Jeewa (randomvariable) · 2016-11-20T23:05:04Z

TTY bug seems unrelated to .NET 1.1, have filed issue at #2745

Naadir Jeewa (randomvariable) · 2016-11-22T23:28:02Z

Not sure why the DSC tests are failing. Any ideas?

I realise that the package isn't getting installed, but not sure how it's supposed to get installed either.

Modify `Build.psm1` and `project.json` files to use .NET Core 1.1. .NET Core 1.1 ships with an older dotnet/cli than has currently been used, so we revert to use case-sensitive directory names for dependencies. .NET Core 1.1 is a pre-requisite for supporting Fedora 24.

Naadir Jeewa (randomvariable) · 2016-12-04T01:33:14Z

Think this is ready to merge.

Dongbo Wang (daxian-dbw)

This is an awesome PR! Thank you very much!
I left a few comments, can you please take a look?

Dongbo Wang (daxian-dbw) · 2016-12-07T22:33:49Z

        Start-NativeExecution {
            curl -sO $obtainUrl/$installScript
-            bash ./$installScript -c $Channel -v $Version
+            bash ./$installScript -c $Channel


Why is -v $Version removed?

I think I had removed this whilst trying to get the 1.1 install script working, but the defaults are now set correctly elsewhere in build.psm1, so it can be restored.

Dongbo Wang (daxian-dbw) · 2016-12-07T23:05:26Z

        "net451": {},
        "netstandard1.6": {
-            "imports": ["dnxcore50", "portable-net45+win8"],
+            "imports": ["netcoreapp1.1", "portable-net45+win8"],


Why do you change dnxcore50 to netcoreapp1.1 here?

That's an error. Will revert.

Thanks for the quick response. Please add new commits to address comments, it's easier to review the incremental changes this way 😄 I will squash the commits when merging the PR.

Dongbo Wang (daxian-dbw) · 2016-12-07T23:21:58Z

@@ -13,10 +13,10 @@
    },


The dependency to Microsoft.PowerShell.PSReadLine should be "6.0.0-*". It's an oversight from the change that bounced the project versions. Would you mind update it in this PR?

Done. Commits upcoming.

Dongbo Wang (daxian-dbw) · 2016-12-07T23:26:37Z

            "dependencies": {
-                "Microsoft.CodeAnalysis.CSharp": "1.3.1-beta1-20160616-03",
-                "System.Diagnostics.TextWriterTraceListener": "4.3.0-preview1-24530-04"
+                "Microsoft.CodeAnalysis.CSharp": "1.3.2",


Since we are using the latest version on nuget.org for other packages, shall we use the 2.0.0-rc version for Microsoft.CodeAnalysis.CSharp? The 2.0.0-rc was published on Nov 18.

The build and tests succeeded against 2.0.0-rc, so this looks fine to go in.

…t.CodeAnalysis.Csharp

…ead of dnxcore

Naadir Jeewa (randomvariable) · 2016-12-08T00:50:03Z

Commits added for each of the review comments.

Dongbo Wang (daxian-dbw)

LGTM.
It's my bad that I didn't make it clear -- you can use one commit to address any number of comments. Sorry to put extra work on your side 😄

Dongbo Wang (daxian-dbw) · 2016-12-08T01:06:16Z

The Travis CI failure is a known issue, not related to your PR. I have restarted it. I will merge this PR once it succeeds.

Dongbo Wang (daxian-dbw) · 2016-12-08T02:34:03Z

Naadir Jeewa (@randomvariable) Thanks again for your contribution!

* Rebase onto .NET Core 1.1 Modify `Build.psm1` and `project.json` files to use .NET Core 1.1. .NET Core 1.1 ships with an older dotnet/cli than has currently been used, so we revert to use case-sensitive directory names for dependencies. .NET Core 1.1 is a pre-requisite for supporting Fedora 24. * PSReadLine: Bump major version of PSReadline to 6.0.0-* * Microsoft.PowerShell.Commands.Utility: Bump major version of Microsoft.CodeAnalysis.Csharp * Microsoft.PowerShell.SDK: Revert attempt to import netcoreapp1.1 instead of dnxcore * build.psm1: Restore ability to specify version of dotnet-install.sh

Microsoft Contribution License Agreements (msftclas) added the cla-signed label Nov 20, 2016

Steve Lee (SteveL-MSFT) added this to the 6.0.0 milestone Nov 20, 2016

Steve Lee (SteveL-MSFT) added the WG-Maintainers-Build specific to affecting the build label Nov 20, 2016

This was referenced Nov 20, 2016

Initial Support for Fedora 24 #2738

Merged

Remove dotnet pinning #2684

Closed

sergei (vors) reviewed Nov 20, 2016

View reviewed changes

PowerShellTeam added Review - Needed Review - Needed The PR is being reviewed labels Nov 21, 2016

Hemant Mahawar (HemantMahawar) removed the Review-Needed label Dec 1, 2016

Dongbo Wang (daxian-dbw) requested changes Dec 7, 2016

View reviewed changes

Dongbo Wang (daxian-dbw) self-assigned this Dec 7, 2016

Naadir Jeewa (randomvariable) added 4 commits December 8, 2016 00:44

PSReadLine: Bump major version of PSReadline to 6.0.0-*

473df5a

Microsoft.PowerShell.Commands.Utility: Bump major version of Microsof…

418e87c

…t.CodeAnalysis.Csharp

Microsoft.PowerShell.SDK: Revert attempt to import netcoreapp1.1 inst…

dfb1056

…ead of dnxcore

build.psm1: Restore ability to specify version of dotnet-install.sh

bc31f96

Dongbo Wang (daxian-dbw) approved these changes Dec 8, 2016

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit 32a8601 into PowerShell:master Dec 8, 2016

Naadir Jeewa (randomvariable) deleted the dotnet11 branch December 8, 2016 10:08

Ilya (iSazonov) removed the Review - Needed The PR is being reviewed label Mar 8, 2017

Uh oh!

Conversation

Naadir Jeewa (randomvariable) commented Nov 20, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Nov 20, 2016

Uh oh!

sergei (vors) commented Nov 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Naadir Jeewa (randomvariable) commented Nov 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Naadir Jeewa (randomvariable) commented Nov 20, 2016

Uh oh!

Naadir Jeewa (randomvariable) commented Nov 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Naadir Jeewa (randomvariable) commented Dec 4, 2016

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) Dec 8, 2016 • 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!

Dongbo Wang (daxian-dbw) Dec 7, 2016 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Naadir Jeewa (randomvariable) commented Dec 8, 2016

Uh oh!

Dongbo Wang (daxian-dbw) left a comment • 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!

Dongbo Wang (daxian-dbw) commented Dec 8, 2016

Uh oh!

Dongbo Wang (daxian-dbw) commented Dec 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Naadir Jeewa (randomvariable) commented Nov 20, 2016 •

edited

Loading

Naadir Jeewa (randomvariable) commented Nov 22, 2016 •

edited

Loading

Dongbo Wang (daxian-dbw) Dec 8, 2016 •

edited

Loading

Dongbo Wang (daxian-dbw) Dec 7, 2016 •

edited

Loading

Dongbo Wang (daxian-dbw) left a comment •

edited

Loading