Fix design-time build for Antlr-generated files #1002
Conversation
|
cc @sharwell as he may find this amusing. 😄 |
| @@ -130,7 +130,7 @@ internal ProjectFileInfo(string filePath) | |||
| } | |||
|
|
|||
| var projectInstance = project.CreateProjectInstance(); | |||
| var buildResult = projectInstance.Build(TargetNames.Compile, | |||
| var buildResult = projectInstance.Build(new string[] { TargetNames.Compile, TargetNames.CoreCompile }, | |||
There was a problem hiding this comment.
Is this going to result in unintended behavior changes in other projects?
There was a problem hiding this comment.
It shouldn't. This is also what we do in Visual Studio.
| <PackageReference Include="Antlr4.Runtime" Version="4.6.4" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Antlr4 Include="Grammar.g4"> |
There was a problem hiding this comment.
📝 This block will no longer be necessary with 4.6.5-beta002.
There was a problem hiding this comment.
FWIW, this is a test project directly from a customer. So, I want to ensure we don't regress it.
| { PropertyNames.BuildProjectReferences, "false" }, | ||
| { PropertyNames._ResolveReferenceDependencies, "true" }, | ||
| { PropertyNames.SolutionDir, solutionDirectory + Path.DirectorySeparatorChar }, | ||
|
|
||
| // This properties allow the design-time build to handle the Compile target without actually invoking the compiler. | ||
| // See https://github.com/dotnet/roslyn/pull/4604 for details. | ||
| { PropertyNames.ProvideCommandLineInvocation, "true" }, | ||
| { PropertyNames.ProvideCommandLineArgs, "true" }, |
There was a problem hiding this comment.
💭 I'm somewhat surprised this is needed. Or was that a different issue?
There was a problem hiding this comment.
It's a different issue. Per my PR comment, there a couple of unrelated changes.
| @@ -249,13 +249,14 @@ public bool IsUnityProject() | |||
| var globalProperties = new Dictionary<string, string> | |||
| { | |||
| { PropertyNames.DesignTimeBuild, "true" }, | |||
| { PropertyNames.BuildingInsideVisualStudio, "true" }, | |||
There was a problem hiding this comment.
😆 I don't see any way to avoid this one.
There was a problem hiding this comment.
Yeah, the property name is kind of a misnomer. Originally, it was added for design-time scenarios like code generators in VS.
| <TargetFramework>netcoreapp2.0</TargetFramework> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Antlr4.CodeGenerator" Version="4.6.4" /> |
There was a problem hiding this comment.
💡 Starting with 4.6.5-beta001 (already released), it is possible to eliminate the Java runtime requirement from the ANTLR build. If you don't currently require a Java installation for some other purpose, it might make sense to update the version and use the new code generator as described in the release notes:
https://github.com/tunnelvisionlabs/antlr4cs/releases/tag/v4.6.5-beta001
There was a problem hiding this comment.
FWIW, I haven't seen a Java installation requirement. I was able to get a design-time build of this without having Java installed.
|
CI is failing because I need to update some other tests that are now getting a couple of additional temporary files. Will do that after lunch. 😀 |
|
I've fixed the tests. |
|
OK. After chatting with @sharwell, it sounds like there are some known issues with the Antlr4 code generators that can make it fail on OSX (e.g. tunnelvisionlabs/antlr4cs#244 and tunnelvisionlabs/antlr4cs#243). I'm going to disable this test on non-Windows for now. |
|
I'll work to get a build up that allows the test to be enabled on all platforms. |
No rush. I know you're super-busy. 😄 |
This change may fix several issues, but it particular it fixes files not appearing in the OmniSharpWorkspace that were generated through Antlr. Essentially, we had failed to set the `BuildingInsideVisualStudio` property when running a design-time build in OmniSharp. Antlr uses this property to determine whether the antlr design-time grammar compilation target runs ([link](https://github.com/tunnelvisionlabs/antlr4cs/blob/f62010862bd1e38b3c6e3940931dcde949bdaa91/runtime/CSharp/Antlr4BuildTasks/Antlr4.net40.targets#L95)). In addition, there a couple of other tweaks in this change that are somewhat unrelated: 1. There was a type in the `ProvideCommandLineArgs` property. 2. We should run two targets: `Compile` and `CoreCompile`. This is needed to produce items from `ProvideCommandLineArgs`.
DustinCampbell
force-pushed
the
antlr-generated-files
branch
from
a9983b5 to
fa60b86
Compare
Fixes dotnet/vscode-csharp#1822
This change may fix several issues, but it particular it fixes files not appearing in the OmniSharpWorkspace that were generated through Antlr. Essentially, we had failed to set the
BuildingInsideVisualStudioproperty when running a design-time build in OmniSharp. Antlr uses this property to determine whether the antlr design-time grammar compilation target runs (link).In addition, there a couple of other tweaks in this change that are somewhat unrelated:
ProvideCommandLineArgsproperty.CompileandCoreCompile. This is needed to produce items fromProvideCommandLineArgs.