-
Notifications
You must be signed in to change notification settings - Fork 114
Support $(ProjectName).NativeMethods.txt pattern for single-file-app projects #1550
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
base: main
Are you sure you want to change the base?
Conversation
| <ItemGroup> | ||
| <CsWin32NativeMethodsTxt Include="%(AdditionalFiles.FullPath)" Condition="'%(FileName)%(Extension)'=='NativeMethods.txt'" /> | ||
| <CsWin32NativeMethodsJson Include="%(AdditionalFiles.FullPath)" Condition="'%(FileName)%(Extension)'=='NativeMethods.json'" /> | ||
| <CsWin32NativeMethodsTxt Include="%(AdditionalFiles.FullPath)" Condition="$([System.String]::new('%(FileName)%(Extension)').EndsWith('NativeMethods.txt'))" /> |
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.
Are you sure you need this string::new syntax? Doesn't this work? It's much simpler.
| <CsWin32NativeMethodsTxt Include="%(AdditionalFiles.FullPath)" Condition="$([System.String]::new('%(FileName)%(Extension)').EndsWith('NativeMethods.txt'))" /> | |
| <CsWin32NativeMethodsTxt Include="%(AdditionalFiles.FullPath)" Condition="'%(FileName)%(Extension)'.EndsWith('NativeMethods.txt')" /> |
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 could swear this didn't work in MSBuild in certain situations. It does work here so I am able to simplify but are you sure there aren't versions of MSBuild we need to support where this is broken?
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.
Docs seem to indicate calling string instance methods on property values is fine?
https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#string-property-functions
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.
Yeah ok, like I thought, the pipeline failed with:
D:\a\1\s\src\Microsoft.Windows.CsWin32\build\Microsoft.Windows.CsWin32.targets(48,70): error MSB4092: An unexpected token "." was found at character position 26 in condition "'%(FileName)%(Extension)'.EndsWith('NativeMethods.txt')". [D:\a\1\s\test\GenerationSandbox.BuildTask.Tests\GenerationSandbox.BuildTask.Tests.csproj::TargetFramework=net9.0-windows10.0.22621.0]
It's weird because it worked locally. I'll go back to the previous version. We've had to do this on item metadata conditions as long as I can remember in MSBuild.
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.
Rules must be different for item metadata references vs. property functions.
Per suggestion in #1546 , added additional items to pick up $(ProjectName).NativeMethods.txt and .json.
Also updated the SourceGenerator to look for AdditionalFile items that end with NativeMethods.txt and .json, and did the same for the equivalent build task logic to pick up those items.
Added tests in the integration-tests to cover this logic.