-
Notifications
You must be signed in to change notification settings - Fork 345
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
Need a common way to reference CMakeLists.txt projects from a csproj #4008
Comments
/cc: @AdamYoblick @zsd4yr |
@hughbe What specifically is the ask from Arcade here? Are you saying that we cannot currently have a reference to a CMakeList.txt in our project references? (i.e. we cannot add a native reference?) |
Or are you saying we can but that it should be a common CMakeList.txt (i.e. the one used by CoreCLR should be the same one we use here) and therefore we need a place to put that common CMakeList? |
No, it is that we can't build c++ projects as part of the existing pipeline. |
@RussKie and @hughbe This request is a bit vague with respect to what would be expected from the Arcade team. There are multiple levels of support implied here so we should be very clear about what is desired. CMake is a meta build system tool. This means it generates the build scripts for actually building. Would Arcade be responsible for install CMake? Which version would be supported? During build generation, CMake makes some assumptions about what kind of scripts to generated based on the system it is running (e.g. On Windows it looks for Visual Studio from newest to oldest - I believe 2010). On non-Windows systems This then brings up a question regarding platform/flavor configuration which would necessitate some canned CMake files be provided by Arcade to properly define the multiple flavors (i.e. RET, CHK, DBG) and ensure that the generated files align with how the managed components will reference them and discover the their outputs. These are only two questions regarding some additional details about expectations for this request. I want to be clear that I would very much welcome Arcade taking care of all the above, I just want everyone to understand the scope of this specific request. |
Thank you for providing the insights. It makes it easier to know what kind of questions we need to ask:) |
Ok. I'll try to explain what we're trying to do in winforms and then I'll detail how I think extracting coreclr's logic to arcade could help with this.
I believe there is some logic e.g. in https://github.com/dotnet/coreclr/blob/master/tests/src/Directory.Build.targets#L113-L197 <Target Name="CopyNativeProjectBinaries" Condition="'$(_CopyNativeProjectBinaries)' == 'true'">
<ItemGroup>
<NativeProjectBinaries Condition="'$(RunningOnUnix)' != 'true'" Include="$(NativeProjectOutputFolder)\**\*.*" />
<!-- ############################################################### -->
<!-- The following is unix only. It is required because the unix test-->
<!-- build, unlike the windows test build, do not place the binaries -->
<!-- built into a separate Debug/Checked/Release directory. Therefore-->
<!-- we must filter the folder to only include dynamic libraries, -->
<!-- static libraries and executables. -->
<!-- -->
<!-- Please take care when modifying the following lines of code. -->
<!-- At a minimum to test any changes to the below, please run a -->
<!-- pri1 test build. -->
<!-- ############################################################### -->
<!-- Include everything and then filter. -->
<NativeProjectBinariesMatched Condition="'$(RunningOnUnix)' == 'true'" Include="$(NativeProjectOutputFolder)\**\*.*" />
<!-- Filter executables on unix -->
<NativeProjectBinariesExeFilter Condition="'$(RunningOnUnix)' == 'true' and $([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `(.*\/)([^.]+)$`))" Include="@(NativeProjectBinariesMatched)" />
<!-- Filter out the *Make* files -->
<NativeProjectBinariesExeFilterRemovedMakeFile Condition="'$(RunningOnUnix)' == 'true' and !$([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `.*Makefile.*`))" Include="@(NativeProjectBinariesExeFilter)" />
<!-- Filter out the CMakeFiles files -->
<NativeProjectBinariesExeFilterRemovedCMakeFile Condition="'$(RunningOnUnix)' == 'true' and !$([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `.*CMakeFiles.*`))" Include="@(NativeProjectBinariesExeFilterRemovedMakeFile)" />
<!-- Filter .dylib files on OSX -->
<NativeProjectBinariesDyLibFilter Condition="'$(__BuildOS)' == 'OSX' and $([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `(.*\/).*\.dylib`))" Include="@(NativeProjectBinariesMatched)" />
<!-- Filter .so files on Linux -->
<NativeProjectBinariesDyLibFilter Condition="'$(__BuildOS)' == 'Linux' and $([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `(.*\/).*\.so`))" Include="@(NativeProjectBinariesMatched)" />
<!-- Filter static lib files on Unix -->
<NativeProjectBinariesStaticLibFilter Condition="'$(RunningOnUnix)' == 'true' and $([System.Text.RegularExpressions.Regex]::IsMatch(['%(Identity)', `(.*\/).*\.a`))" Include="@(NativeProjectBinariesMatched)" />
<!-- Merge the filtered lists -->
<NativeProjectBinaries Condition="'$(RunningOnUnix)' == 'true'" Include="@(NativeProjectBinariesExeFilterRemovedCMakeFile)" />
<NativeProjectBinaries Condition="'$(RunningOnUnix)' == 'true'" Include="@(NativeProjectBinariesDyLibFilter)" />
<NativeProjectBinaries Condition="'$(RunningOnUnix)' == 'true'" Include="@(NativeProjectBinariesStaticLibFilter)" />
</ItemGroup>
<Error Text="The native project files are missing in $(NativeProjectOutputFolder) please run build from the root of the repo at least once"
Condition="'@(NativeProjectBinaries)' == ''" />
<Copy
SourceFiles="@(NativeProjectBinaries)"
DestinationFiles="@(NativeProjectBinaries -> '$(OutDir)%(Filename)%(Extension)')"
SkipUnchangedFiles="$(SkipCopyUnchangedFiles)"
OverwriteReadOnlyFiles="$(OverwriteReadOnlyFiles)"
Retries="$(CopyRetryCount)"
RetryDelayMilliseconds="$(CopyRetryDelayMilliseconds)"
UseHardlinksIfPossible="$(CreateHardLinksForCopyFilesToOutputDirectoryIfPossible)">
<Output TaskParameter="DestinationFiles" ItemName="FileWrites" />
</Copy>
</Target>
<Target Name="ResolveCmakeNativeProjectReference"
Condition="'@(ProjectReference)' != ''"
BeforeTargets="ConsolidateNativeProjectReference;BeforeResolveReferences;BeforeClean" >
<ItemGroup>
<NativeProjectReference Include="%(ProjectReference.Identity)" Condition="$([System.String]::Copy(%(ProjectReference.FileName)).ToUpper()) == 'CMAKELISTS'" />
<ProjectReference Remove="%(NativeProjectReference.Identity)" />
<NativeProjectReferenceNormalized Include="@(NativeProjectReference -> '%(FullPath)')" />
</ItemGroup>
</Target>
<Target Name="ConsolidateNativeProjectReference"
Condition="'$(_CopyNativeProjectBinaries)' == 'true'"
BeforeTargets="Build" >
<ItemGroup Condition="'@(NativeProjectReferenceNormalized)' != ''">
<NativeProjectOutputFoldersToCopy Include="$([System.String]::Copy('%(NativeProjectReferenceNormalized.RelativeDir)').Replace($(SourceDir),$(__NativeTestIntermediatesDir)\src\))" Condition="'$(RunningOnUnix)' == 'true'" />
<NativeProjectOutputFoldersToCopy Include="$([System.String]::Copy('%(NativeProjectReferenceNormalized.RelativeDir)').Replace($(SourceDir),$(__NativeTestIntermediatesDir)\src\))$(Configuration)\" Condition="'$(RunningOnUnix)' != 'true'" />
</ItemGroup>
<Message Text= "Full native project references are :%(NativeProjectReferenceNormalized.Identity)" />
<Message Text= "Native binaries will be copied from :%(NativeProjectOutputFoldersToCopy.Identity)" />
<MSBuild Projects="$(MSBuildProjectFile)" Targets="CopyNativeProjectBinaries" Properties="NativeProjectOutputFolder=%(NativeProjectOutputFoldersToCopy.Identity)" Condition="'@(NativeProjectReference)' != ''" />
</Target>
<Target Name="CopyAllNativeProjectReferenceBinaries" DependsOnTargets="ResolveCmakeNativeProjectReference;ConsolidateNativeProjectReference" />
To answer your specific questions
I think this is possible in the same way that coreclr does (https://github.com/dotnet/coreclr/blob/master/eng/common/native/install-cmake.sh). The version supported should be consistent across .NET Core projects and obviously depends on the requirements of all the projects
I would expect Arcade to use the user's default toolchain (same behaviour as in coreclr). Its the user's responsibility for this. |
Thanks @hughbe ! One of the things I would be interested in, is what (if any) common functionality Arcade should provide across repos. In general, the "rule of thumb" is that if something is shared - then it's likely a good candidate for our shared (go figured) infrastructure - e.g. Arcade. Any chance that after extracting/copying/plagiarising and otherwise stealing the good work from coreclr, we could circle back around and see if (or how) that bit should fit back into Arcade? |
@hughbe Okay seems like something that isn't going to force too much policy into other repos. Here are two additional thing that must also be addressed/considered.
|
Yeah I noticed this. Ideally, yes. Would simplify things greatly
Do we not already have a standard layout |
That is a bit more complicated since there would be complexity with parallel build, but of course would be preferred. It does increase the work and testing burden significantly.
We do indeed. However, that needs to be propagated to all CMake generated projects so that those projects are also aware of the layout. In order to do that CMake needs to know about the additional build flavour ( There is also the IDE support question. If the user loads the |
I've been working locally on making a new SDK to add to Arcade that would drive our CMake builds in the runtime repo. I could definitely add a feature that enables referencing a CMakeLists.txt from another project through this SDK. I'm going to do a little bit of work testing it out with CoreCLR before I put out a PR here. |
@jkoritzinsky - I'm going through and looking at issues w/ no activity for a bit. Is this still something you'd like to do? |
Yes. Unless @hughbe has any additional feedback, I believe this is ready to be merged. |
Well, #4533 is ready to be merged to fix this. |
Ah yes, thanks for reminding me of this PR. Has all the feedback been addressed? Getting signoff from @mmitche , @alexperovich , and @jkotas is sufficient for me to merge. |
I believe I've addressed all the feedback, but I'm ok waiting for explicit signoffs. |
@jkotas are you ok with this? |
See #4533 (comment) |
@jkoritzinsky - looks like there still an open bit of feedback from @jkotas |
It looks like I've addressed all of Jan's feedback from what I can see. There's a little bit of feedback from @mmitche that I'll go address now. |
This is of use in dotnet/winforms#1932
See dotnet/winforms#1932 (comment)
We'd basically like to extract the behavior present in coreclr (https://github.com/dotnet/coreclr/blob/master/Documentation/building/test-configuration.md) where you can write
This is useful to many different .NET Core projects, not just coreclr. I'd like to use it in winforms but also WPF would need to be able to test interop scenarios similarily
/cc @RussKie @AaronRobinsonMSFT @jkoritzinsky
The text was updated successfully, but these errors were encountered: