build: Update build.sh to run fully manual build by nadiasvertex · Pull Request #2832 · PowerShell/PowerShell

nadiasvertex · Dec 2, 2016

Also, add ubuntu-16.10-x64 to the list of architectures to build for.

Also, add ubuntu-16.10-x64 to the list of architectures to build for.w

msftclas · Dec 2, 2016

Hi @nadiasvertex, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

vors · Dec 3, 2016

+   cmake -DCMAKE_BUILD_TYPE=Debug .
+   make -j
+   make test
+   popd


The biggest step is missing: dotnet build from src/powershell-unix

vors · Dec 3, 2016

    powershell -noprofile -c "Import-Module ./build.psm1; Start-PSBuild"
 else
-    echo 'No `powershell`, see docs/building/linux.md or macos.md to build PowerShell!'
+   echo 'Continuing with full manual build'


Maybe a separate script (i.e. build-new-platform.sh or something) would be better. The reason is the build system is already pretty complicated. Putting more logic into the main build path would increase the probability of future errors and time for trouble-shooting.

… of build

vors

When onboarding new unix distros, it's desirable to have docker build as well https://github.com/PowerShell/PowerShell/tree/master/docker

For example see https://github.com/PowerShell/PowerShell/pull/2738/files#diff-7f8105a00850a7f7fea1de45bca96f8e

vors · Dec 4, 2016

 else
-    echo 'No `powershell`, see docs/building/linux.md or macos.md to build PowerShell!'
+   echo 'Continuing with full manual build'
+   ./build-from-scratch.sh


Again, I don't think it should be run automatically.
The error message should suggest to use this script, but not run it.

Why? This makes no sense. The user has typed "build.sh". They expect the build to run. The script has proved to itself that powershell is not available. So now it should fail and tell the user what to do next? That seems to be the height of silly, especially when it knows what to do next.

You right, it's silly.
I was thinking we are using this script for the CI build and didn't want to complicate the CI troubleshooting.
But turns out that script is not used and purely for the convenience here.

Please, ignore my comment about the separating logic into a different file: since we call it anyway, there is no need to do it. Sorry for going back and forth on it.

Couple more things:

build.sh will never be as fully-featured as build.psm1 file. I.e. build.psm1 can find dotnet, if it's not in the path, can run dotnet build or dotnet publish, etc. I think it's better to avoid duplicating all such logic and use build.sh precisely for the bootstrapping on the new platforms. If we agree about this purpose, then my suggestion would be to replace

pushd src/powershell-unix dotnet build --configuration Linux popd

by

dotnet publish --configuration Linux ./src/powershell-unix/ --output bin

And add an echo message about the build location, something like You can run powershell from bin/.

publish will create a self-contained build of powershell.

Putting it in bin/ will allow to avoid complicated logic about figuring out where the build was made

this version could be used for the build with .psm1 and would not interfere with the standard location (avoid "file in use" problem).

build.sh doesn't perform analog of Restore-PSModule (adding PS modules from myget and psgallery). It's worth to have a message with this information at the end of the build to avoid confusion.

vors · Dec 4, 2016

@@ -0,0 +1,26 @@
+#!/usr/bin/env bash


I don't think build-from-scratch is a good name. build.sh also build everything from scratch.
Maybe build-without-powershell.sh? I think name should emphasise what's different about this build script.

nadiasvertex · Dec 8, 2016

That sounds great. I will make th or changes.

…

On Mon, Dec 5, 2016, 2:05 PM Sergei Vorobev ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In build.sh <#2832>: > if hash powershell 2>/dev/null; then echo 'Continuing with `powershell -noprofile -c Start-PSBuild`' powershell -noprofile -c "Import-Module ./build.psm1; Start-PSBuild" else - echo 'No `powershell`, see docs/building/linux.md or macos.md to build PowerShell!' + echo 'Continuing with full manual build' + ./build-from-scratch.sh You right, it's silly. I was thinking we are using this script for the CI build and didn't want to complicate the CI troubleshooting. But turns out that script is not used and purely for the convenience here. Please, ignore my comment about the separating logic into a different file: since we call it anyway, there is no need to do it. Sorry for going back and forth on it. Couple more things: - build.sh will never be as fully-featured as build.psm1 file. I.e. build.psm1 can find dotnet, if it's not in the path, can run dotnet build or dotnet publish, etc. I think it's better to avoid duplicating all such logic and use build.sh precisely for the bootstrapping on the new platforms. If we agree about this purpose, then my suggestion would be to replace pushd src/powershell-unix dotnet build --configuration Linux popd by dotnet publish --configuration Linux ./src/powershell-unix/ --output bin And add an echo message about the build location, something like You can run powershell from bin/. 1. publish will create a self-contained build of powershell. 2. Putting it in bin/ will allow to avoid complicated logic about figuring out where the build was made 3. this version could be used for the build with .psm1 and would not interfere with the standard location (avoid "file in use" problem). - build.sh doesn't perform analog of Restore-PSModule (adding PS modules from myget and psgallery). It's worth to have a message with this information at the end of the build to avoid confusion. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2832>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8JvCcph1OdwgySUuL4jaQkQbE6FVSyks5rFGB3gaJpZM4LCEBA> .

andyleejordan · Dec 16, 2016

I like this change a lot 😄

SteveL-MSFT · Dec 19, 2016

@nadiasvertex your build issue with #2735 complaining about Microsoft.CodeAnalysis.dll is due to src/TypeCatalogParser/Main.cs the call x.Name.ToLower() making the filenames in lowercase and thus later on not matching the case-sensitive paths. Not sure why this was there explicitly, but removing that call (changing to just x.Name) will get you past this.

SteveL-MSFT · Dec 19, 2016

    powershell -noprofile -c "Import-Module ./build.psm1; Start-PSBuild"
 else
-    echo 'No `powershell`, see docs/building/linux.md or macos.md to build PowerShell!'
+   echo 'Continuing with full manual build'


Seems like we should include install dotnet in build.sh
Also cmake

andyleejordan · Dec 19, 2016

@SteveL-MSFT x.Name.ToLower() is there explicitly because of .NET CLI's behavior, per #2162. We verified this was intended change on their end in https://github.com/dotnet/cli/issues/4141.

If CamelCasePaths are being created for .NET CLI restored packages, then one of three things is ocurring: either an old version of dotnet is being used (a build before 3546), or the latest builds changed behavior (again), or dotnet has disparate behavior among platforms.

SteveL-MSFT · Dec 19, 2016

@andschwa well, this is awkward since dotnet restore on my Ubuntu 16.10 system has packages with Pascal casing...

removing the ToLower() I can get a build on 16.10 using this change, but getting an exception starting powershell

andyleejordan · Dec 20, 2016

@SteveL-MSFT file a bug with .NET CLI, see if they purposefully changed it back 😄

SteveL-MSFT · Dec 20, 2016

@andschwa I think the difference is a breaking change they made with preview3 vs preview2. On my Windows box and Ubuntu16.10, I have preview2 (as far as I can tell, preview3 isn't available on 16.10). My Ubuntu16.04 box has preview3. This PR may have to wait until we have preview3.

andyleejordan · Dec 20, 2016

@SteveL-MSFT @nadiasvertex:

I installed the exact build that Start-PSBootstrap is currently pinned to on an Ubuntu 16.10 box, did a dotnet restore, and confirmed that these packages are not restored lower-cased:

docker run -it ubuntu:16.10 bash
apt-get update
apt-get install libunwind8 libicu57 libcurl3 git curl
curl -sO https://raw.githubusercontent.com/dotnet/cli/v1.0.0-preview2-1-3177/scripts/obtain/dotnet-install.sh
bash dotnet-install.sh -c preview -v 1.0.0-preview2-1-00317
git clone https://github.com/PowerShell/PowerShell.git
cd PowerShell/
~/.dotnet/dotnet --version
~/.dotnet/dotnet restore
ls ~/.nuget/packages/

And got:

1.0.0-preview2-1-003177.dotnetSentinel
Libuv
Microsoft.Build
Microsoft.Build.Framework
Microsoft.Build.Utilities.Core
Microsoft.CSharp
Microsoft.CodeAnalysis.Analyzers
Microsoft.CodeAnalysis.CSharp
Microsoft.CodeAnalysis.Common
Microsoft.CodeAnalysis.VisualBasic
Microsoft.DiaSymReader
Microsoft.DiaSymReader.Native
Microsoft.DotNet.Cli.Utils
...

So I dug a bit further, and in 32a8601 the ToLower() was removed because:

.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.

This commit was made December 8, so I'd recommend @nadiasvertex you pull PowerShell and get the latest changes, as the bug you hit appears fixed.

andyleejordan · Dec 20, 2016

(This corresponds to @SteveL-MSFT finding the change is in preview3, but not preview2; and I didn't realize we'd reverted the tooling back that far.)

* build: Update build.sh to run fully manual build * add ubuntu-16.10-x64 to the list of architectures to build for.w

build: Update build.sh to run fully manual build

563a311

Also, add ubuntu-16.10-x64 to the list of architectures to build for.w

msftclas added the cla-required label Dec 2, 2016

PowerShellTeam added the Review - Needed The PR is being reviewed label Dec 2, 2016

vors suggested changes Dec 3, 2016

View reviewed changes

build: Move from-scratch build into a separate script, add final part…

258932c

… of build

vors suggested changes Dec 4, 2016

View reviewed changes

build: Rename build-from-scratch

f82de8e

SteveL-MSFT mentioned this pull request Dec 8, 2016

powershell won't compile on ubuntu 16.10 #2735

Closed

build.sh: Update manual build steps

56df4ab

SteveL-MSFT reviewed Dec 19, 2016

View reviewed changes

vors merged commit 7388ec9 into PowerShell:master Dec 28, 2016

rjmholt pushed a commit to rjmholt/PowerShell that referenced this pull request Jan 9, 2017

build: Update build.sh to run fully manual build (PowerShell#2832)

cbf7e4c

* build: Update build.sh to run fully manual build * add ubuntu-16.10-x64 to the list of architectures to build for.w

Search code, repositories, users, issues, pull requests...

Conversation

nadiasvertex commented Dec 2, 2016

Uh oh!

msftclas commented Dec 2, 2016

Uh oh!

vors Dec 3, 2016

Choose a reason for hiding this comment

Uh oh!

vors Dec 3, 2016

Choose a reason for hiding this comment

Uh oh!

vors left a comment

Choose a reason for hiding this comment

Uh oh!

vors Dec 4, 2016

Choose a reason for hiding this comment

Uh oh!

nadiasvertex Dec 5, 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!

vors Dec 5, 2016

Choose a reason for hiding this comment

Uh oh!

vors Dec 4, 2016

Choose a reason for hiding this comment

Uh oh!

nadiasvertex Dec 5, 2016

Choose a reason for hiding this comment

Uh oh!

nadiasvertex commented Dec 8, 2016 via email

Uh oh!

andyleejordan commented Dec 16, 2016

Uh oh!

SteveL-MSFT commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SteveL-MSFT Dec 19, 2016

Choose a reason for hiding this comment

Uh oh!

andyleejordan commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SteveL-MSFT commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyleejordan commented Dec 20, 2016

Uh oh!

SteveL-MSFT commented Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyleejordan commented Dec 20, 2016

Uh oh!

andyleejordan commented Dec 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nadiasvertex Dec 5, 2016 •

edited

Loading

SteveL-MSFT commented Dec 19, 2016 •

edited

Loading

andyleejordan commented Dec 19, 2016 •

edited

Loading

SteveL-MSFT commented Dec 19, 2016 •

edited

Loading

SteveL-MSFT commented Dec 20, 2016 •

edited

Loading