-
Notifications
You must be signed in to change notification settings - Fork 237
first build.ps1 #623
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
first build.ps1 #623
Conversation
build.ps1
Outdated
|
||
function needsDotNet451TargetingPack () { | ||
# how could we check for this? | ||
return $false |
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.
Get-CimInstance Win32_Product | Where-Object Name -match '\.NET Framework 4\.5\.1 Multi-Targeting Pack'
However, this call is slow - ~30 secs on my laptop. We might want to do this check only during bootstrap. Or we find a quicker way to test for this requirement.
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.
That sounds good. I have it running on only Bootstrap and also write to the console a small message that says the check wasn't made when on windows and building.
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.
@tylerl0706 part of why it's slow is it verifies and repairs all installed MSI packages, potentially breaking installed applications. Here's a kb on it. We should definitely find an alternative, maybe searching HKLM:\SOFTWARE
instead.
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 did my best to look around for something that could indicate the targeting pack in HKLM:\SOFTWARE
but I couldn't find anything :(
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 have any other hints/pointers, I'm happy to try.
That's evil. Yeah, we should find a different way. |
P.S @rkeithhill + @SeeminglyScience I'm fine leaving the Targeting Pack check out and leaving it in the README. FWIW, Visual Studio installs this targeting pack so maybe we could settle for checking if they have Visual Studio and if they don't, tell them they may need the targeting pack. |
@tylerl0706 I vote to just leave it out. I haven't actually seen any other build scripts include checking for it presumably for these reasons, it's just a huge pain. If only we could ditch it entirely and just use the core CLI 😒 |
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 after targeting pack check is removed
I was talking to @joeyaiello about this the other day... It might be possible if the proper shim was in place between .NET Core (or Standard) -> .NET Framework. We'd have to ditch 3/4 support IIRC. |
So I changed the logic a bit based off of the build.ps1 in the PowerShellStandard repo: The changes are:
I also changed BootstrapBuildEnv to just Bootstrap because it's less verbose. |
In scope:
build.ps1
checks for missing tools and runsInvoke-Build Build
build.ps1 -Clean
checks for missing tools and runsInvoke-Build Clean
and thenInvoke-Build Build
build.ps1 -Test
checks for missing tools and runsInvoke-Build Build
and thenInvoke-Build Test
build.ps1 -Bootstrap
informs you of what tools you are missingOut of scope:
The script is a bit verbose on purpose so that we can easily add additional dependencies.
Resolves #586