Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it easier to install the dotnet CLI tools #1139

Merged
merged 28 commits into from
Feb 19, 2019

Conversation

JamesWTruher
Copy link
Member

PR Summary

Sometimes the build of script analyzer fails because version specified in global.json is not installed. This PR makes it easier to install the proper version of the CLI tools with the -bootstrap parameter to build.ps1

PS> ./build.ps1 -bootstrap

will determine which version of the CLI is needed, validate whether the proper version is already present, and if not, will install it.

This does not validate whether the net452 targeting framework is installed, that must be done manually.

PR Checklist

@bergmeister
Copy link
Collaborator

Awesome, I wanted to do that for a while but was never too bothered to actually take the initiative.
Looks ok but the code that you copied from the appveyor.psm1 for this change should be removed then and the new -Bootstrap switch should be used in CI to avoid code duplication.

… dotnet CLI

update build script to handle WMF4 better (theoretically)
Use output of dotnet --version if there are problems with --list-sdks
set failure of finding dotnet as a warning, we will attempt to install it during bootstrap
this handles the case of a blank slate where dotnet has never been installed
@JamesWTruher JamesWTruher force-pushed the bootstrap branch 3 times, most recently from badeae0 to fcf73b6 Compare February 14, 2019 19:19
@JamesWTruher JamesWTruher force-pushed the bootstrap branch 2 times, most recently from 6d7ec14 to f0dab58 Compare February 14, 2019 20:59
Sometimes you can't even invoke dotnet if the version is too low
Throw away stderr of dotnet execution under some case and emit it in others.
Change ToString of PortableVersion to enable better sorts
Sometimes we need the raw version so the installation script works, but sometimes we need to be able to compare it with other versions.
@JamesWTruher JamesWTruher force-pushed the bootstrap branch 2 times, most recently from 4686c98 to 866f35b Compare February 15, 2019 19:15
Also add additional verbose output to improve debuggability
print response from upload
We've had trouble with LANG being set to something that causes problems, so make sure we report what it is.
Removed a bit of the verbosity which was added for debugging
Fix some errors in the module discovered by the tests
Change logic when downloading the dotnet install script
Also enforce en_US.UTF-8 language to ensure that appveyor can see our test results
@JamesWTruher
Copy link
Member Author

@bergmeister - I think this is ready now, let me know if you have additional comments

Copy link
Collaborator

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

A lot of positive changes, even tests for it, I am overall happy with it. Points to consider or think about before merging (but not required to be addressed):

  • If we call bootstrap automatically anyway as part of the build module, why do we need to expose the Bootstrap switch then?
  • Why can we not rely on the dotnet binary doing the logic for testing if the version is suitable? As far as I am aware they do logic and write to stderr in certain cases. It looks a bit like re-inventing the wheel to me but as long as it works, I am fine with it though.
  • Most of the logic seems to be quite generic, I am wondering if it would be worthwhile to later separate the dotnet logic of this out into a separate repo/module on the PSGallery?

@bergmeister bergmeister changed the title WIP: Make it easier to install the dotnet CLI tools Make it easier to install the dotnet CLI tools Feb 19, 2019
@JamesWTruher
Copy link
Member Author

@bergmeister thanks for the feedback and here are answers to questions:

  • I thought it would be useful to run the bootstrapping outside of the build process under some circumstances. I personally want to install sometimes, but not build.
  • I found in my testing that some earlier versions of dotnet don't even let you run if there's a mismatch. I was trying to support any old version where that might happen. I also need to support when dotnet isn't even there.
  • quite possibly - I did try to keep it generic for possible reuse.

@JamesWTruher JamesWTruher merged commit cfeb7d5 into PowerShell:development Feb 19, 2019
bergmeister pushed a commit to bergmeister/PSScriptAnalyzer that referenced this pull request Mar 5, 2019
* Add bootstrapping code to install dotnet

* fix logic for downloading proper dotnet installer.

* add a version checker for installed dotnet

Also harden logic for running the installation script

* Add bootstrapping code to install dotnet

* Handle missing dotnet, and install it.

* Add logic for checking the appropriate version of the Cli tools before starting to build

* Update appveyor.psm1 to use build script bootstrapping for installing dotnet CLI

update build script to handle WMF4 better (theoretically)

* make the hunt for the dotnet executable more generic and try harder to find the exe

* Fix typo when checking for usable versions of dotnet

* use -version rather than ambiguous -v when installing dotnet

* Improve error message when hunting for cli version

Use output of dotnet --version if there are problems with --list-sdks

* ignore errors when first attempting to find dotnet executable

set failure of finding dotnet as a warning, we will attempt to install it during bootstrap
this handles the case of a blank slate where dotnet has never been installed

* Improve logic for handling a system where dotnet has never been installed

* fine tune messages emitted during the hunt for dotnet

* additional logic for finding dotnet executable

* harden search for dotnet

Sometimes you can't even invoke dotnet if the version is too low

* Attempt to quieten build output.

Throw away stderr of dotnet execution under some case and emit it in others.
Change ToString of PortableVersion to enable better sorts

* Fix tostring method for portable version

* add -Raw flag to Get-GlobalJsonSdkVersion

Sometimes we need the raw version so the installation script works, but sometimes we need to be able to compare it with other versions.

* Attempt to harden the upload code

Also add additional verbose output to improve debuggability
print response from upload

* upgrade pester version to 4.4.4

* Force the testsuite TestFixture to be named 'Pester' to get through appveyors test result recognition

* Emit env:LANG to output before executing tests

We've had trouble with LANG being set to something that causes problems, so make sure we report what it is.
Removed a bit of the verbosity which was added for debugging

* Create tests for build module

Fix some errors in the module discovered by the tests

* Add additional tests

Change logic when downloading the dotnet install script
Also enforce en_US.UTF-8 language to ensure that appveyor can see our test results
bergmeister pushed a commit to bergmeister/PSScriptAnalyzer that referenced this pull request Mar 22, 2019
* Add bootstrapping code to install dotnet

* fix logic for downloading proper dotnet installer.

* add a version checker for installed dotnet

Also harden logic for running the installation script

* Add bootstrapping code to install dotnet

* Handle missing dotnet, and install it.

* Add logic for checking the appropriate version of the Cli tools before starting to build

* Update appveyor.psm1 to use build script bootstrapping for installing dotnet CLI

update build script to handle WMF4 better (theoretically)

* make the hunt for the dotnet executable more generic and try harder to find the exe

* Fix typo when checking for usable versions of dotnet

* use -version rather than ambiguous -v when installing dotnet

* Improve error message when hunting for cli version

Use output of dotnet --version if there are problems with --list-sdks

* ignore errors when first attempting to find dotnet executable

set failure of finding dotnet as a warning, we will attempt to install it during bootstrap
this handles the case of a blank slate where dotnet has never been installed

* Improve logic for handling a system where dotnet has never been installed

* fine tune messages emitted during the hunt for dotnet

* additional logic for finding dotnet executable

* harden search for dotnet

Sometimes you can't even invoke dotnet if the version is too low

* Attempt to quieten build output.

Throw away stderr of dotnet execution under some case and emit it in others.
Change ToString of PortableVersion to enable better sorts

* Fix tostring method for portable version

* add -Raw flag to Get-GlobalJsonSdkVersion

Sometimes we need the raw version so the installation script works, but sometimes we need to be able to compare it with other versions.

* Attempt to harden the upload code

Also add additional verbose output to improve debuggability
print response from upload

* upgrade pester version to 4.4.4

* Force the testsuite TestFixture to be named 'Pester' to get through appveyors test result recognition

* Emit env:LANG to output before executing tests

We've had trouble with LANG being set to something that causes problems, so make sure we report what it is.
Removed a bit of the verbosity which was added for debugging

* Create tests for build module

Fix some errors in the module discovered by the tests

* Add additional tests

Change logic when downloading the dotnet install script
Also enforce en_US.UTF-8 language to ensure that appveyor can see our test results
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants