Skip to content

Conversation

@dougbu
Copy link
Contributor

@dougbu dougbu commented Aug 31, 2020

Do not use $(AppRuntimeVersion) or NETCore version for shared Fx in Helix runs

  • remove hard-coded $(AppRuntimeVersion) property entirely
    • use $(SharedFxVersion) instead

nits:

  • make RunTests help more clear about how --runtime option is used
  • remove unused --sdk option on RunTests command line
  • add Microsoft.AspNetCore.App.Ref package to Helix content later, ensuring it exists

… Helix runs

- remove hard-coded `$(AppRuntimeVersion)` property entirely
  - use `$(SharedFxVersion)` instead

nits:
- make `RunTests` help more clear about how `--runtime` option is used
- remove unused `--sdk` option on `RunTests` command line
- add Microsoft.AspNetCore.App.Ref package to Helix content later, ensuring it exists
@dougbu dougbu requested review from a team, BrennanConroy and HaoK August 31, 2020 23:59
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Aug 31, 2020
@dougbu
Copy link
Contributor Author

dougbu commented Sep 1, 2020

@Pilchie I assume this can be tell-mode because it's test-only. Is that correct❔ And, if it doesn't get approved / validated by 10AM tomorrow, I'll need to do the ask-mode dance i.e. that applies for test-only changes too❔

@dougbu
Copy link
Contributor Author

dougbu commented Sep 1, 2020

/fyi Reviewers, this came out of the servicing readiness test. Turns out another change I made for the (still-failing) 6.0 branding work is needed once we stabilize our versions.

@Pilchie
Copy link
Member

Pilchie commented Sep 1, 2020

Definitely tell before 10am tomorrow. I'll confirm the test only bar for after that during tactics tomorrow.

<HelixUseArchive>false</HelixUseArchive>
<LoggingTestingDisableFileLogging Condition="'$(IsHelixJob)' == 'true'">false</LoggingTestingDisableFileLogging>
<NodeVersion>10.15.3</NodeVersion>
<AppRuntimeVersion>5.0.0-ci</AppRuntimeVersion>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wonder if this hard-code version was causing problems when using eng/scripts/RunHelix.ps1 locally, where the version would be 5.0.0-dev even now❔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HaoK what do you thunk❔

Copy link
Member

Choose a reason for hiding this comment

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

Yes for sure, that's what we use for the directory we install to, if you were seeing weirdness with the bits not installed correctly this would be why

@dougbu dougbu added the tell-mode Indicates a PR which is being merged during tell-mode label Sep 1, 2020
<PostCommands>@(HelixPostCommand)</PostCommands>
<Command Condition="$(IsWindowsHelixQueue)">call runtests.cmd $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(AppRuntimeVersion).nupkg Microsoft.AspNetCore.App.Ref.$(AppRuntimeVersion).nupkg $(HelixTimeout)</Command>
<Command Condition="!$(IsWindowsHelixQueue)">./runtests.sh $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(AppRuntimeVersion).nupkg Microsoft.AspNetCore.App.Ref.$(AppRuntimeVersion).nupkg $(HelixTimeout)</Command>
<Command Condition="$(IsWindowsHelixQueue)">call runtests.cmd $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(SharedFxVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(SharedFxVersion).nupkg Microsoft.AspNetCore.App.Ref.$(SharedFxVersion).nupkg $(HelixTimeout)</Command>
Copy link
Member

Choose a reason for hiding this comment

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

Seems worth considering calculating the runtime from the sharedfxversion, would save us one parameter...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. I was originally thinking of parsing the filename i.e. going the other way but dropped that before thinking of creating the filename. But, worthwhile or not, I'd rather get this in if it passes the CI. I'm also unlikely to do the relatively minor follow up but feel free❕

@dougbu dougbu merged commit 48c1261 into release/5.0 Sep 1, 2020
@dougbu dougbu deleted the dougbu/helix.in.servicing branch September 1, 2020 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants