Port fix to make satellite generation work for VB to release/2.1.1xx#1997
Merged
livarcocc merged 1 commit intodotnet:release/2.1.1xxfrom Mar 14, 2018
Merged
Conversation
Fixes dotnet#1748. The `CoreGenerateSatelliteAssemblies` task first calls the `WriteCodeFragment` task to write a bunch of attributes to a .cs file, and then calls the `Csc` task to compile it (along with the .resource files, of course). However, the call to `WriteCodeFragment` will use the project's language to generate the source file. This means that in a VB project we'll end up generating VB code into a .cs file which we then try to compile with csc.exe. That doesn't end well. The fix here is to always pass "C#" as the language to `WriteCodeFragment`.
Contributor
Author
|
cc @weshaggard |
Contributor
Author
|
@MattGertz for servicing approval. This would go into the next 2.1.1xx / 15.6.x servicing release, not the current release in progress. However, source builds for the current release will have this patch applied. |
|
@nguerrera Approving this, but @livarcocc we should have some sort of VSO bug tracking this is it's going into a servicing release of VS assigned to 15.6.1 milestone. |
Contributor
|
@nguerrera go ahead and file the VSO issue. We will need to fill out the ASK MODE template there and figure out which vehicle will carry this for 15.6. The MSRC is locked down, so, it will have to be a servicing after that. |
Contributor
Author
|
Holding merge until VSO issues is filed, but dealing with some other priorities first. |
Contributor
Author
|
VSO bug filed: https://devdiv.visualstudio.com/DevDiv/_queries?id=574901 |
livarcocc
approved these changes
Mar 13, 2018
dseefeld
added a commit
to dseefeld/source-build
that referenced
this pull request
Apr 9, 2018
dseefeld
added a commit
to dotnet/source-build
that referenced
this pull request
Apr 10, 2018
* Move roslyn repo to branch without patches * Patch tools for SDK issue: dotnet/sdk#1997 * Add updaters to update global.json and Tools.props * Change UpdateXml to write output properly * Add patches for roslyn * Update roslyn.proj to build the source-build solution. Remove unneeded NuSpec. * Add roslyn to known-good.proj * Move roslyn sha to commit that corrects issue with SourceBuild.sln and remove temporary sln. Remove xml updater since it isn't needed. Move patch to tools-local dir and update git apply to not use --directory. * Remove UpdateJson task because it is causing a conflict with newtonsoft.json versions. Adding #409 to re-add this later.
MichaelSimons
pushed a commit
to dotnet/source-build-externals
that referenced
this pull request
Feb 8, 2022
* Move roslyn repo to branch without patches * Patch tools for SDK issue: dotnet/sdk#1997 * Add updaters to update global.json and Tools.props * Change UpdateXml to write output properly * Add patches for roslyn * Update roslyn.proj to build the source-build solution. Remove unneeded NuSpec. * Add roslyn to known-good.proj * Move roslyn sha to commit that corrects issue with SourceBuild.sln and remove temporary sln. Remove xml updater since it isn't needed. Move patch to tools-local dir and update git apply to not use --directory. * Remove UpdateJson task because it is causing a conflict with newtonsoft.json versions. Adding dotnet/source-build#409 to re-add this later.
JL03-Yue
pushed a commit
that referenced
this pull request
Mar 19, 2024
…024.1 (#1997) [main] Update dependencies from dotnet/arcade - Coherency Updates: - Microsoft.DotNet.XliffTasks: from 1.0.0-beta.23516.1 to 1.0.0-beta.23523.1 (parent: Microsoft.DotNet.Arcade.Sdk)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Customer scenario
VB project using SDK and having satellite assemblies. Build breaks trying to compile the Assembly info for the satellite that is written out in VB instead of C# even though satellite builds in SDK always invoke the C# compiler.
2.1.1xx and 2.1.2xx SDKs cannot build themselves because Roslyn in these release branches depends on having this fix.
Bugs this fixes:
#1748 (Already fixed, but only for 2.1.300+ and this is a request to have it included in the next 2.1.1xx servicing release as well as 2.1.200.)
Workarounds, if any
Wait for 2.1.300 to be released (which already has the fix).
Copy entire target with fix into Directory.Build.targets. This is very fragile because it would shut off any future fixes to the target and tie the customer project to internal implementation details of the SDK. I would be very hesitant to recommend this.
Source build can apply a patch, but this keeps a discrepancy betweeen source and non-source-builds and adds debt. By putting this low risk fix into the next servicing release, we eliminate that debt sooner.
Risk
Low
Performance impact
None
Is this a regression from a previous update?
No. It was broken since the first release that supported VB (1.1 I believe), but Roslyn was the first customer to report it. It seems the intersection of VB sdk projects and satellite assemblies is small.
Root cause analysis:
Insufficient VB coverage.
Insufficient understanding / communication that a Roslyn branch effectively cannot depend on a CLI newer than the CLI release train it is on. Otherwise, source build will break. Moving forward, any fix we take to unblock Roslyn will need to consider the correlation between Roslyn branches and CLI branches.
How was the bug found?
Dogfooding