GeezerVison

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

GeezerVison is showing github.com — Fix Start-PSBuild on WSL if repository was already built on Windows by bergmeister · Pull Request #5346 · PowerShell/PowerShell

Conversation

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

To fix #5194: Use the first 3 characters of the passed -Runtime parameter for the .inc file (the full runtime name could not be used since the separation characters ike e.g. the the hyphen would make dotnet crash when building).

A separate issue is that the runtime does not get determined if is not being passed (which is OK since dotnet takes then care of choosing the right one) but maybe given this scenario would it make sense to determine and set it as a convenience?

Sorry, something went wrong.

Uh oh!

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

rkeithhill reacted with thumbs up emoji

All reactions

👍 1 reaction


          Fix build on WSL if repository was already built on Windows.

e4fa4e8

Note that the '-Runtime' parameter needs to be passed in this case.

Christoph Bergmeister (bergmeister) requested review from Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) as code owners

November 5, 2017 18:29

Keith Hill (rkeithhill) commented Nov 5, 2017

Copy link

Copy Markdown

Collaborator

This should slot in nicely with my tweak to build.psm1 #5341

All reactions

Sorry, something went wrong.

Uh oh!

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

Keith Hill (rkeithhill) commented Nov 5, 2017

Copy link

Copy Markdown

Collaborator

I think you need to update this .gitignore file to make it handle the platform specific inc files:

40:121ms> gc .\src\TypeCatalogGen\.gitignore
powershell.inc

I was noticing that Git thought I had a new file with powershell_win.inc.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Update .gitignore to match powershell**.inc instead of powershell.inc…

8d4a8af

… because the file can now optionally end in the runtime prefix.

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor Author

Thanks. Well spotted, I updated it to ignore now powershell**.inc. I find it a bit odd that there is a special .gitignore file in this folder and am wondering if we can move this to the top level .gitignore file?
I see that Andy Jordan (@andschwa) moved out ignoring the .inc file from the top level .gitignore file in this commit. What was the reasoning for this?

All reactions

Sorry, something went wrong.

Uh oh!

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

Andy Jordan (andyleejordan) commented Nov 5, 2017

Copy link

Copy Markdown

Member

Christoph Bergmeister (@bergmeister) Because this ignore rule doesn't apply to anywhere else in the repository, just keeping it simple 😄

All reactions

Sorry, something went wrong.

Uh oh!

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

Christoph Bergmeister (bergmeister) commented Nov 5, 2017

Copy link

Copy Markdown

Contributor Author

Andy Jordan (@andschwa) My thinking is rather that if there will never be any other .inc files somewhere else, it would be nice to put it into the top one to have a more central place, especially since there is only one entry in it. But this is probably more of a stylistic preference with no 'right' answer depending on how big the central place can become (to me the output of gci -Recurse .gitignore | % {cat $_} did not seem to be too long)... But I'll leave it then as it is if that's your preference.

All reactions

Sorry, something went wrong.

Uh oh!

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

Travis Plunk (TravisEz13) reviewed

Nov 6, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/src/System.Management.Automation/CoreCLR/CorePsTypeCatalog.cs")) {
		$incFileName = 'powershell'
		if (-not [string]::IsNullOrEmpty($Runtime)) {
		$incFileName = "$($incFileName)_$($Runtime.Substring(0,3))" # use the first 3 characters only because otherwise dotnet will crash due to the separation characters

Travis Plunk (TravisEz13) Nov 6, 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 believe our guidance is to put the comment on a separate line.

Sorry, something went wrong.

Uh oh!

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

All reactions

Travis Plunk (TravisEz13) Nov 6, 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.

you could replace the seperation character.... not sure it's needed though.

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 6, 2017

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.

You're right about the comment. I noticed that only after the commit but I will fix that. The separation character is only for readability because to me powershell_win or powerhell_lin is easier and faster to read than powershellwin or powershelllin

Sorry, something went wrong.

Uh oh!

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

All reactions

Travis Plunk (TravisEz13) approved these changes

Nov 6, 2017

View reviewed changes

Travis Plunk (TravisEz13) 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.

Looks good with minor comments.

Dongbo Wang (@daxian-dbw) and/or Ilya (@iSazonov) should review as well.

Sorry, something went wrong.

Uh oh!

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

All reactions


          Put comment on single line as per style guide and add more comment de…

af094e3

…tails

Christoph Bergmeister (bergmeister) commented Nov 6, 2017 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor Author

NB: The build was red because NuGet has a global outage (see here)

All reactions

Sorry, something went wrong.

Uh oh!

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

Ilya (iSazonov) reviewed

Nov 7, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

		# Append the first 3 characters fo the Runtime only because otherwise dotnet will crash due to the separation characters. The underscore is only for better readability.
		$incFileName = "$($incFileName)_$($Runtime.Substring(0,3))"
		}
		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/TypeCatalogGen/$($incFileName).inc")) {

Ilya (iSazonov) Nov 7, 2017

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.

We use $incFileName).inc so many times so make sense add:

$incFileName = ($incFileName).inc

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 7, 2017

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.

Ok, I have addressed that.

Sorry, something went wrong.

Uh oh!

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

All reactions

Ilya (iSazonov) commented Nov 7, 2017

Copy link

Copy Markdown

Collaborator

I'd prefer to fix the issue more natively like #3870.

Also I'm not sure that's the only runtime addiction. Should we clean up if our files contains "#if UNIX"?

All reactions

Sorry, something went wrong.

Uh oh!

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


          Include file extension in $incFileName variable to reduce '.inc' dupl…

e1fde8f

…ication as suggested in PR.

Tested again that it still works when building on Windows first using 'Start-PSBuild' and then 'Start-PSBuild -Runtime linux-x64' on WSL (ubuntu16)

Dongbo Wang (daxian-dbw) requested changes

Nov 7, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

		# handle TypeGen
		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/src/System.Management.Automation/CoreCLR/CorePsTypeCatalog.cs")) {
		$incFileName = 'powershell.inc'
		if (-not [string]::IsNullOrEmpty($Runtime)) {

Dongbo Wang (daxian-dbw) Nov 7, 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.

Can you use $Options.Runtime? When -Runtime is not specified, we will figure out the current RID via dotnet --info, which will be captured by $Options.Runtime

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 7, 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.

The $Options.Runtime will always be 'NotNullOrEmpty', so maybe you can have:

$incFileName = "powershell_$($Runtime.Substring(0,3)).inc"

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017

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.

Good idea. Yes, we can use $Options.Runtime indeed instead. I was not aware that you already figure it out automatically when it is not being provided. Shall I maybe add this information as a comment to the $Runtime parameter?

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 14, 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.

You mean the -Runtime parameter of Start-PSBuild? Sure you can add a comment there.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		$incFileName = 'powershell.inc'
		if (-not [string]::IsNullOrEmpty($Runtime)) {
		# File name must be different for Windows and Linux to allow build on Windows and WSL.
		# Append the first 3 characters fo the Runtime only because otherwise dotnet will crash due to the separation characters. The underscore is only for better readability.

Dongbo Wang (daxian-dbw) Nov 7, 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.

characters fo the Runtime

A typo.

otherwise dotnet will crash due to the separation characters

Could you please share more information on this?

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017 •

edited

Loading

Uh oh!

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

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.

Thanks for spotting the typo and thanks for coming back about the crash. When I started out I was experimenting with different approaches (one of was to put the inc file into a runtime specific folder) and I actually cannot repro the crash anymore and will therefore use the full $Runtime string instead of the first 3 characters.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		if (-not [string]::IsNullOrEmpty($Runtime)) {
		# File name must be different for Windows and Linux to allow build on Windows and WSL.
		# Append the first 3 characters fo the Runtime only because otherwise dotnet will crash due to the separation characters. The underscore is only for better readability.
		$incFileName = "$($incFileName)_$($Runtime.Substring(0,3)).inc"

Dongbo Wang (daxian-dbw) Nov 7, 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.

Maybe just use powershell_$($Runtime.Substring(0,3)).inc? Otherwise it would beome powershell.inc_xxx.inc

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017

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.

This change has now been undone since we do not need that logic any more due to the usage of $Options.Runtime

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		# Append the first 3 characters fo the Runtime only because otherwise dotnet will crash due to the separation characters. The underscore is only for better readability.
		$incFileName = "$($incFileName)_$($Runtime.Substring(0,3)).inc"
		}
		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/TypeCatalogGen/$($incFileName)")) {

Dongbo Wang (daxian-dbw) Nov 7, 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.

$($incFileName) can be replaced by $incFileName

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017

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.

No, this is one of the gotchas about PowerShell string interpolation because PowerShell allows underscores for variable names. Therefore PowerShell would then look for a variable named incFileName_ instead of incFileName. As a general rule, I always wrap variables in a string inside $() because it also safeguards against cases like calling a property on an object...

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 14, 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.

In the case of "$($incFileName)_xxx.inc", you can replace it with "${incFileName}_xxx.inc". PowerShell support the varialbe syntax ${var name}

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		(
		[Parameter(Mandatory=$true)]
		[ValidateNotNullOrEmpty()]
		$IncFileName

Dongbo Wang (daxian-dbw) Nov 7, 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.

Maybe we should have 'powershell.inc' as the default value for this parameter, so that Start-TypeGen without parameter will continue to work.

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017 •

edited

Loading

Uh oh!

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

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 can do that if people actually call Start-TypeGen? I always use the -ResGen swich on Start-PSBuild because the reason why I need update ResGen is because I need to build anyway. Instead of having the default, why not call New-PSOptions and grab the determined Runtime, which is what Start-PSBuild does as well.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 14, 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.

It's not commonly needed, but when debugging anything about the type catalog, it's handy to just run Start-TypeGen without building the rest of the projects. For that purpose, a simple default value like powershell.inc would be sufficient, no need to call New-Options to figure out the runtime.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 14, 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.

And a minor comment: the variable name you used in the rest of this function is incFileName, maybe it can be changed to IncFileName.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		Push-Location "$PSScriptRoot/src/Microsoft.PowerShell.SDK"
		try {
		$ps_inc_file = "$PSScriptRoot/src/TypeCatalogGen/powershell.inc"
		$ps_inc_file = "$PSScriptRoot/src/TypeCatalogGen/$($incFileName)"

Dongbo Wang (daxian-dbw) Nov 7, 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.

$($incFileName) can be replaced by $incFileName

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) Nov 13, 2017

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.

In this case we can do that but what do you think about my comment above where this is not always possible?

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) Nov 14, 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.

PowerShell support this syntax for variable: ${var name}, so you can use "${varname}_xxx.inc" in that case.

Sorry, something went wrong.

Uh oh!

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

All reactions

Christoph Bergmeister (bergmeister) commented Nov 9, 2017

Copy link

Copy Markdown

Contributor Author

I'll address the comments next Monday when I'm back from travelling.

All reactions

Sorry, something went wrong.

Uh oh!

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

Christoph Bergmeister (bergmeister) added 2 commits

November 13, 2017 20:16


          Fix typo and do not trim the $Runtime string since this does not seem…

adcf857

… to be necessary.


          Use $Options.Runtime instead of $Runtime because the former gets dete…

a78e0c0

…rmined if the $Runtime is not being provided.

Christoph Bergmeister (bergmeister) commented Nov 13, 2017

Copy link

Copy Markdown

Contributor Author

Dongbo Wang (@daxian-dbw) Thanks for your useful feedback. I have addressed all issues and left comments there. But basically the 2 remaining questions are:

Should we be strict about string interpolation (i.e. use $()) since there are gotchas as I pointed out?
Should Start-TypeGen have a default value for the new $IncFileName parameter or should we automatically determine it in there similarly as in Start-PSBuild?

All reactions

Sorry, something went wrong.

Uh oh!

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

Dongbo Wang (daxian-dbw) reviewed

Nov 14, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

		# Handle TypeGen
		# .inc file name must be different for Windows and Linux to allow build on Windows and WSL.
		$incFileName = "powershell_$($Options.Runtime).inc"
		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/TypeCatalogGen/$($incFileName)")) {

Dongbo Wang (daxian-dbw) Nov 14, 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.

$($incFileName)

Subexpression is not needed here, you can just use $incFileName

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		# Append the first 3 characters fo the Runtime only because otherwise dotnet will crash due to the separation characters. The underscore is only for better readability.
		$incFileName = "$($incFileName)_$($Runtime.Substring(0,3)).inc"
		}
		if ($TypeGen -or -not (Test-Path "$PSScriptRoot/TypeCatalogGen/$($incFileName)")) {

Dongbo Wang (daxian-dbw) Nov 14, 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.

In the case of "$($incFileName)_xxx.inc", you can replace it with "${incFileName}_xxx.inc". PowerShell support the varialbe syntax ${var name}

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) requested changes

Nov 14, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

		(
		[Parameter(Mandatory=$true)]
		[ValidateNotNullOrEmpty()]
		$IncFileName

Dongbo Wang (daxian-dbw) Nov 14, 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.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

build.psm1 Outdated

		Push-Location "$PSScriptRoot/src/Microsoft.PowerShell.SDK"
		try {
		$ps_inc_file = "$PSScriptRoot/src/TypeCatalogGen/powershell.inc"
		$ps_inc_file = "$PSScriptRoot/src/TypeCatalogGen/$($incFileName)"

Dongbo Wang (daxian-dbw) Nov 14, 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.

PowerShell support this syntax for variable: ${var name}, so you can use "${varname}_xxx.inc" in that case.

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) commented Nov 14, 2017

Copy link

Copy Markdown

Member

Christoph Bergmeister (@bergmeister) For the two questions, I answered them in the corresponding comment threads: #5346 (comment) and #5346 (comment)

All reactions

Sorry, something went wrong.

Uh oh!

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


          Addressed minor suggestions made in PR 5346.

4625d83

Use default value for Start-TypeGen for convenience and minor syntax changes.
Tested again that it still works when building on Windows first and then on WSL

Christoph Bergmeister (bergmeister) commented Nov 14, 2017

Copy link

Copy Markdown

Contributor Author

Dongbo Wang (@daxian-dbw) Thanks for the review. I have addressed your comments.

All reactions

Sorry, something went wrong.

Uh oh!

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

Dongbo Wang (daxian-dbw) approved these changes

Nov 15, 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.

LGTM. Thanks Christoph Bergmeister (@bergmeister)!

Sorry, something went wrong.

Uh oh!

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

All reactions

Dongbo Wang (daxian-dbw) self-assigned this

Nov 15, 2017

Travis Plunk (TravisEz13) approved these changes

Nov 15, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit 019cfee into PowerShell:master

Nov 15, 2017

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

Aug 6, 2026


          Fix Start-PSBuild on WSL if repository was already built on Windows (P…

953422f

…owerShell#5346)

Replace the static `powershell.inc` filename with a name based on the current RID, so that the build doesn't confuse when running in the same local repository on WSL.

Reviewers

Ilya (iSazonov)

iSazonov left review comments

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.

Building PowerShell Core on WSL generates error

Uh oh!

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

6 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 01402520 — 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

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Keith Hill (rkeithhill) commented Nov 5, 2017

Uh oh!

Keith Hill (rkeithhill) commented Nov 5, 2017

Uh oh!

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andy Jordan (andyleejordan) commented Nov 5, 2017

Uh oh!

Christoph Bergmeister (bergmeister) commented Nov 5, 2017

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) left a comment

Choose a reason for hiding this comment

Uh oh!

Christoph Bergmeister (bergmeister) commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Nov 7, 2017

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!

Christoph Bergmeister (bergmeister) 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!

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

Christoph Bergmeister (bergmeister) 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!

Christoph Bergmeister (bergmeister) commented Nov 9, 2017

Uh oh!

Christoph Bergmeister (bergmeister) commented Nov 13, 2017

Uh oh!

Choose a reason for hiding this comment

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 •

edited

Loading

Uh oh!

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

Christoph Bergmeister (bergmeister) commented Nov 5, 2017 •

edited

Loading

Uh oh!

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

Christoph Bergmeister (bergmeister) commented Nov 6, 2017 •

edited

Loading

Uh oh!

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

Christoph Bergmeister (bergmeister) Nov 13, 2017 •

edited

Loading

Uh oh!

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

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

edited

Loading

Uh oh!

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

Christoph Bergmeister (bergmeister) Nov 13, 2017 •

edited

Loading

Uh oh!

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

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

edited

Loading

Uh oh!

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