Skip to content

Conversation

@KevinRansom
Copy link
Contributor

So .. at the end of the build it does some versioning validation, it takes a while and only really needs doing on the ci and official builds. It used the product build version of fsi.exe, which is not always built.

  1. Only do version validation
  2. Add fsi.proj to proto build and Bootstrapper
  3. Don't do a partial clean at the end of the proto-build it's pointless.

@KevinRansom KevinRansom requested a review from brettfo March 28, 2019 05:02
@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2019

Hey @brettfo told me to remove these checks in #6365

@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2019

(@KevinRansom Just to say I've always wanted to avoid FSI in the proto build, since I can imagine a future where FSI is a separate tool in a separate repository. But it's no big deal, and maybe these version checks can be removed altogether anyway)

@KevinRansom
Copy link
Contributor Author

@dsyme, I need to talk to @brettfo to find about where the validation is actually done, because I was able to successfully build with an invalid version number and I think the CI needs to ensure that doesn't happen.

Copy-Item "$ArtifactsDir\bin\fsc\$bootstrapConfiguration\$bootstrapTfm\*" -Destination $dir

Write-Host "Cleaning Bootstrap compiler artifacts"
Run-MSBuild $projectPath "/t:Clean" -logFileName "BootstrapClean" -configuration $bootstrapConfiguration
Copy link
Member

Choose a reason for hiding this comment

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

Why is the clean step getting removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it just deletes a few files from fsc\Proto\net46, and looks odd.

@KevinRansom
Copy link
Contributor Author

@dsyme, I need to talk to @brettfo to find about where the validation is actually done, because I was able to successfully build with an invalid version number and I think the CI needs to ensure that doesn't happen.

@KevinRansom
Copy link
Contributor Author

@dsyme, The proto compiler is really an artifact of this repo, we just need one that we can rely on for build tasks. We could either use the LKG or build and execute one in the build. For now it seems as if building one during the proto build phase is a reasonable choice, and gives us better coverage.

@KevinRansom KevinRansom reopened this Mar 28, 2019
@KevinRansom
Copy link
Contributor Author

@dsyme, @brettfo and I chatted about this. and we decided this validation is still needed, choosing to only do it during CI is a decent compromise.

@KevinRansom KevinRansom merged commit 4d33579 into dotnet:master Mar 29, 2019
@KevinRansom KevinRansom deleted the validation branch March 29, 2019 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants