GeezerVison

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

GeezerVison is showing github.com — Enable install-powershell.ps1 to work from powershell-daily folder by SteveL-MSFT · Pull Request #5429 · PowerShell/PowerShell

Conversation

Steve Lee (SteveL-MSFT) commented Nov 13, 2017

Copy link

Copy Markdown

Member

To make it easier to selfhost daily:

removed [validate] attribute from parameter so this script can be invoked directly from web.
This doesn't have any negative impact as the $Destination parameter will have default value if null or empty.
moved removal of destination folder later as installing the package requires packagemanagement module and archive module (on Windows)
on Windows, because files have open handles when run from existing powershell-daily install, I rename the existing files and copy over the new ones. User needs to exit and restart pwsh to take effect (similar to macOS/Linux where you have to exit and restart anyways)

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) added 3 commits

November 11, 2017 21:15


          remove [validate] attribute so script can be called directly from inv…

e58ec0e

…oke-expression


          enable installing daily build using $env:PSCoreDaily

402eb03

enable running from installed daily by using temp folder


          if using from same folder as being installed for daily, rename existi…

5c1c8e2

…ng files as they are in use

and copy over the new ones

Steve Lee (SteveL-MSFT) assigned Dongbo Wang (daxian-dbw)

Nov 13, 2017

Steve Lee (SteveL-MSFT) requested a review from Dongbo Wang (daxian-dbw)

November 13, 2017 06:45

Steve Lee (SteveL-MSFT) requested review from Travis Plunk (TravisEz13) and Aditya Patwardhan (adityapatwardhan) as code owners

November 13, 2017 06:45


          remove $env:pscoredaily usage, can use: iex "& { $(irm 'url') } -daily"

391ac68

Dongbo Wang (daxian-dbw) reviewed

Nov 13, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) left a comment

Copy link

Copy Markdown

Member

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.

Still reviewing.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

tools/install-powershell.ps1

		$Destination = "${Destination}-daily"
		}
		}
		Write-Verbose "Destination: $Destination" -Verbose

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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.

Should get the unresolved absolute provider path for $Destination. Otherwise, the $Destination -eq $PSHome check below may fail when it should succeed.
You can use

$Destination = $PSCmdlet.SessionState.Path.GetUnresolvedProviderPathFromPSPath($Destination)

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

Will update

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

tools/install-powershell.ps1 Outdated

		$packagePath = Join-Path -Path $tempDir -ChildPath $packageName
		Invoke-WebRequest -Uri $downloadURL -OutFile $packagePath

		New-Item -ItemType Directory -Path "$Destination.new" > $null

Dongbo Wang (daxian-dbw) Nov 13, 2017 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

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.

Please use the $tempDir for anything that is temporary, it's cleaner and also prevent the corner case that $Destination.new already exists.
For example

$packageContent = Join-Path -Path $tempDir -ChildPath content
New-Item -ItemType Directory -Path $packageContent > $null
...
if ($IsWinEnv) {
    Expand-Archive ... -DestinationPath $packageContent
} else {
    tar zxf  ... -C $packageContent
}

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

Will update

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

tools/install-powershell.ps1 Outdated

		}

		Remove-Destination $Destination
		Move-Item -Path "$Destination.new" -Destination $Destination

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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.

If you can use Move-Item here, I think you can do the same in the daily code path without having to differentiate between $Destination exists or not (line 127).

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

The differentiation is needed when $Destination is in use (because install-powershell was run from $Destination) so I can't use the Move-Item optimization.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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.

When installing a release package here, the Destination could also be $PSHOME, right?

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

Yes, that's true. Will fix.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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 just tried install-powershell.ps1 -Destination $PSHOME on Windows, and new files are not moved to $PSHOME after the run.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) reviewed

Nov 13, 2017

View reviewed changes

Comment thread

tools/install-powershell.ps1

		if (Test-Path -Path "$Destination.old") {
		Remove-Item "$Destination.old" -Recurse -Force
		}
		if ($IsWinEnv -and ($Destination -eq $PSHome)) {

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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 didn't know that on Linux you can delete $PSHome from a running powershell.
Can you please add some comment in Remove-Destination about the different scenarios of the remove operation?

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

Sure

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

tools/install-powershell.ps1 Outdated

		}

		Remove-Destination $Destination
		Move-Item -Path "$Destination.new" -Destination $Destination

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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 just tried install-powershell.ps1 -Destination $PSHOME on Windows, and new files are not moved to $PSHOME after the run.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) reviewed

Nov 13, 2017

View reviewed changes

Comment thread

tools/install-powershell.ps1 Outdated

		Copy-Item -Path $contentPath\* -Destination $Destination -Recurse -Force
		if (Test-Path $Destination) {
		Write-Verbose "Copying files" -Verbose
		Copy-Item -Recurse -Path "$contentPath\*" -Destination $Destination -ErrorAction Ignore

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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.

We shouldn't specify -ErrorAction Ignore, if it fails to copy a file then the installation fails. And maybe add -Force.

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

The failure is due to it copying folders, I can change this to filter to just files and letting it fail

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

tools/install-powershell.ps1 Outdated

		$contentPath = [System.IO.Path]::Combine($tempDir, $packageName, "content")
		Copy-Item -Path $contentPath\* -Destination $Destination -Recurse -Force
		if (Test-Path $Destination) {
		Write-Verbose "Copying files" -Verbose

Dongbo Wang (daxian-dbw) Nov 13, 2017

Copy link

Copy Markdown

Member

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 think this log message should be moved before if (Test-Path $Destination) since both copy-item and move-item would take time. Maybe change the message to Deploying files ...

Sorry, something went wrong.

Uh oh!

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

All reactions

Steve Lee (SteveL-MSFT) Nov 13, 2017

Copy link

Copy Markdown

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

Move-Item is just updating a single directory pointer so shouldn't take any time

Sorry, something went wrong.

Uh oh!

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

All reactions

Aditya Patwardhan (adityapatwardhan) mentioned this pull request

Nov 13, 2017

Try fixing the 'brew update' issue in bootstrap script #5400

Merged

Travis Plunk (TravisEz13) commented Nov 13, 2017

Copy link

Copy Markdown

Member

restarted macOS ci

All reactions

Sorry, something went wrong.

Uh oh!

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


          address PR feedback

117427b

Dongbo Wang (daxian-dbw) approved these changes

Nov 14, 2017

View reviewed changes

Travis Plunk (TravisEz13) approved these changes

Nov 14, 2017

View reviewed changes

Steve Lee (SteveL-MSFT) commented Nov 14, 2017

Copy link

Copy Markdown

Member Author

Still testing, don't merge yet

All reactions

Sorry, something went wrong.

Uh oh!

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


          fix copying of files to correct destination folder

c1dc92f

Steve Lee (SteveL-MSFT) commented Nov 14, 2017

Copy link

Copy Markdown

Member Author

Working for me. I think it's ready to go.

All reactions

Sorry, something went wrong.

Uh oh!

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

Dongbo Wang (daxian-dbw) merged commit 83e1d64 into PowerShell:master

Nov 14, 2017

Steve Lee (SteveL-MSFT) deleted the install-powershell branch

November 15, 2017 01:08

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Enable install-powershell.ps1 to update the current running powershell (

a316070

PowerShell#5429)

1. Remove `[validate]` attribute from parameter so this script can be invoked directly from the web. This doesn't have any negative impact as the $Destination parameter will have a default value if null or empty.
2. Move removal of destination folder later as installing the package requires package management module and archive module (on Windows)
3. On Windows, because files have open handles when run from existing powershell-daily install, I rename the existing files and copy over the new ones.  User needs to exit and restart `pwsh` to take effect (similar to macOS/Linux where you have to exit and restart anyways)

Reviewers

Dongbo Wang (daxian-dbw)

daxian-dbw approved these changes

Travis Plunk (TravisEz13)

TravisEz13 approved these changes

Aditya Patwardhan (adityapatwardhan)

Awaiting requested review from adityapatwardhan

Uh oh!

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

Assignees

Dongbo Wang (daxian-dbw)

Labels

None yet

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.

Conversation

Steve Lee (SteveL-MSFT) commented Nov 13, 2017

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!

Dongbo Wang (daxian-dbw) Nov 13, 2017 • 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!

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!

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!

Travis Plunk (TravisEz13) commented Nov 13, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 14, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 14, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Dongbo Wang (daxian-dbw) Nov 13, 2017 •

edited

Loading

Uh oh!

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