GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — new build scripts for release pipeline by JamesWTruher · Pull Request #1442 · PowerShell/PSScriptAnalyzer

Conversation

James Truher (JamesWTruher) commented Apr 15, 2020

Copy link

Copy Markdown

Contributor

PR Summary

The previous build scripts for release used Docker which is no longer available for MS build/signing pipelines. These needed to be rewritten them to not use docker and I created some additional configuration scripts for signing. There shouldn't be any affect on the product. I made a slight improvement to one of the tests so when it fails we get better diagnosability.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) added the Area - Build label

Apr 15, 2020

James Truher (JamesWTruher) requested a review from Christoph Bergmeister (bergmeister)

April 15, 2020 16:51

James Truher (JamesWTruher) self-assigned this

Apr 15, 2020

Rob Holt (rjmholt) reviewed

Apr 15, 2020

View reviewed changes

Comment thread

build.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

build.psm1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

build.psm1


		function New-Catalog
		{
		[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseCompatibleCommands', '')]

Rob Holt (rjmholt) Apr 15, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is there a specific diagnostic here that was appearing?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Apr 17, 2020

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

One of the stages in the sign build was to run all the tests, and one of the tests found new-filecatalog not being present in all versions, but I hadn't picked up the latest master. Also, I removed the test phase from the build since it happens in CI. I can pull this line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rjmholt reacted with thumbs up emoji

All reactions

👍 1 reaction

James Truher (JamesWTruher) Apr 21, 2020

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

nope - I need to put it back - now it's failing in the CI build

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Rob Holt (rjmholt) Apr 28, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No worries -- perhaps we can open an issue to track removing this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

build.psm1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

tools/releaseBuild/AssemblySignConfig.xml Resolved

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) reviewed

Apr 15, 2020

View reviewed changes

Comment thread

build.psm1 Outdated

		$framework,
		"--configuration",
		"$buildConfiguration"
		if ( $env:InVSTS ) {

Christoph Bergmeister (bergmeister) Apr 15, 2020

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The TF_BUILD environment variable is defined in Azure DevOps and could be used for that instead of defining a custom variable on the release build. Or do you need to specifically know if you are running in the release build? If so, why do we need to build differently with a different output directory when we copy it to the module directory anyway at the end?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rjmholt reacted with thumbs up emoji

All reactions

👍 1 reaction

James Truher (JamesWTruher) Apr 17, 2020

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'll try that - thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Apr 17, 2020

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The more important question to understand for me is why we cannot keep the existing directory structure?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) reviewed

Apr 15, 2020

View reviewed changes

Comment thread

build.psm1

		$foundVersion = Get-InstalledCLIVersion
		Write-Warning "No suitable dotnet CLI found, requires version '$requiredVersion' found only '$foundVersion'"
		}
		$verboseWanted = $false

Christoph Bergmeister (bergmeister) Apr 15, 2020

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why is this needed? PowerShell has the $VerbosePreference variable for that and should automatically propagate the usage of -Verbose to down to the other called cmdlets (splatting is the only exception where this does not work as far as I am aware)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) Apr 17, 2020

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

because I want finer grained control, I don't want verbose everywhere. I tried it without and the logs are super large and I would rather be more targeted (no reason to be verbose on import-module for example).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) mentioned this pull request

Apr 16, 2020

False positive: "is implicitly aliasing 'Get-..\..\Target.ps1'" on file path? #1369

Closed

James Truher (JamesWTruher) force-pushed the NewBuild branch from cf16500 to e73fa56 Compare

April 17, 2020 19:35

James Truher (JamesWTruher) added 14 commits

April 21, 2020 15:12


          change build location when in VSTS

c3c7fdd


          copy newtonsoft assembly from proper location

272039b


          Add support for verbose output during build

67cc341


          update verbose behavior

db8bed0

incorporate catalog building into module


          better code to handle verbose

2243b62


          file for signing analyzer files

8ed8171


          tweaks to catalog creation

5ff3bff


          Add suppression of UseCompatibleCommands rule for function that build…

eacda3b

…s file catalog


          Improve message when there are failures in use compatible commands

c391c7a


          change logic for whether catalog can be created based on whether new-…

dc4653b

…filecatalog exists rather than IsWindows variable which doesn't exist on PS5


          Add catalog signing configuration

52e5818


          fix typo in catalog name

5d5ae33


          use 400 for signing

2bbd2d5


          Address PR feedback

1462e1b

Remove comment code
change logic to use existing environment variable when building in VSTS
Remove extraneous suppression of New-FileCatalog

James Truher (JamesWTruher) force-pushed the NewBuild branch from b3cff98 to 1462e1b Compare

April 21, 2020 22:13

James Truher (JamesWTruher) added 2 commits

April 21, 2020 15:53


          add the suppression back to the function which creates the filecatalog

2fb32d3


          Create suppression file for CredScan so it does not get triggered.

f04b6a1

Rob Holt (rjmholt) approved these changes

Apr 28, 2020

View reviewed changes

Comment thread

build.psm1


		function New-Catalog
		{
		[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseCompatibleCommands', '')]

Rob Holt (rjmholt) Apr 28, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No worries -- perhaps we can open an issue to track removing this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

James Truher (JamesWTruher) merged commit 7bb24d2 into PowerShell:master

Apr 29, 2020

Reviewers

Christoph Bergmeister (bergmeister)

bergmeister left review comments

+1 more reviewer

Rob Holt (rjmholt)

rjmholt approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

James Truher (JamesWTruher)

Labels

Area - Build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.