cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.89k stars 724 forks source link

Project Parser does not support Wildcards. #2183

Open rpriest1260 opened 6 years ago

rpriest1260 commented 6 years ago

The following is a valid ItemGroup entry in a Visual Studio Project:

   <ItemGroup>
        <None Include="WildCardIncludes\**\*.txt">
            <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
        </None>
    </ItemGroup>

However, the project parser will fail if it encounters such an entry.

rpriest1260 commented 6 years ago

I've updated the project parser and I am willing to submit a PR for review.

kcamp commented 6 years ago

Will be interested in seeing the PR in that it may also address the pattern(s) identified in GH-1299

gep13 commented 6 years ago

@rpriest1260 @kcamp have both of you tried the Project Parser that lives in Cake.Incubator? While things were churning in the csproj format, the parser logic was being updated over there. At some point, I think soon, we need to move the logic from Cake.Incubator into Cake proper.

kcamp commented 6 years ago

I haven't attempted the incubator implementation in production implementations; I wound up working around the issue in a different way. I knocked together a quick test case against the incubator implementation. While it avoids the unhandled exception, it still doesn't quite meet expectations.

Given a csproj containing this

<ItemGroup>
    <Compile Include="**\*.cs" />
  </ItemGroup>
  <ItemGroup>
    <None Include="packages.config" />
  </ItemGroup>

My, perhaps wrong, expectations would be for the parser to unfold the glob similar to the MSBuild/VS runtime. As it stands, that sample (above) fails with

.... to not have any items matching x.FilePath.Path.Contains("**"), but found {Cake.Incubator.CustomProjectFile
   {
      Compile = True
      FilePath = Cake.Incubator.ProjectPath
      {
         IsFile = True
         Path = "**\*.cs"
      }
      RelativePath = "**\*.cs"
   }

I had originally introduced the Globber into the mix via PR-1529 as a potential solution, but that branch/PR is quite outdated at this point.

It may be worth resurrecting some of that code for integration into the incubator's implementation to fully address this? Or at least discussing what the expected return values should be for a wildcarded inclusion?

gep13 commented 6 years ago

Did you have any thoughts on this @wwwlicious

rpriest1260 commented 6 years ago

I've handled the globbing in my revised code. I will submit a pull request shortly.

gep13 commented 6 years ago

@rpriest1260 which PR? I would suggest a PR to Cake.Incubator would make the most sense, as we will likely take everythign from there into Cake.

rpriest1260 commented 6 years ago

I was just about to submit one. I did the work against the current project parser code, not the incubator. I will create a branch against the incubator, apply my changes there, and submit the PR using that. I will respond back when complete.

rpriest1260 commented 6 years ago

The current incubator project doesn't build. Seems like the cake version isn't pinned?

Preparing to run build script...
Running build script...
Analyzing build script...
Processing build script...
Installing addins...
Error: The assembly 'Cake.Codecov, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'
is referencing an older version of Cake.Core (0.22.0).
This assembly need to reference at least Cake.Core version 0.26.0.
Another option is to downgrade Cake to an earlier version.
It's not recommended, but you can explicitly opt-out of assembly verification
by configuring the Skip Verification setting to true
(i.e. command line parameter "--settings_skipverification=true",
environment variable "CAKE_SETTINGS_SKIPVERIFICATION=true",
read more about configuration at https://cakebuild.net/docs/fundamentals/configuration)

I'm pretty sure you don't want me to commit without first building everything :)

wwwlicious commented 6 years ago

@rpriest1260 use --settings_skipverification=true to build. This is fine.

rpriest1260 commented 6 years ago

thanks @wwwlicious , I thought of that. Setting CAKE_SETTINGS_SKIPVERIFICATION=true gets you passed the first error. But then the "DupFinder" throws an error. I guess it would just help if someone ran a more recent build of the incubator stuff.:

========================================
DupFinder
========================================
Executing task: DupFinder
Deleting file Z:/test/cake/forks/cake_incubator_rpriest1260/92338360-91be-46bf-8320-574ab4e5d07e.cake
JetBrains Duplicates Finder 2017.2
Running in 64-bit mode, .NET runtime 4.0.30319.42000 under Microsoft Windows NT 6.2.9200.0
Collecting files to analyze:
  from project 'Z:\test\cake\forks\cake_incubator_rpriest1260\src\Cake.Incubator.sln' excluding: Z:/test/cake/forks/cake_incubator_rpriest1260/src/Cake.Incubator/**/*.AssemblyInfo.cs;Z:/test/cake/forks/cake_incubator_rpriest1260/src/Cake.Incubator.Tests/*.cs;Z:/test/cake/forks/cake_incubator_rpriest1260/src/Cake.Incubator/CustomProjectParser.cs;Z:/test/cake/forks/cake_incubator_rpriest1260/src/Cake.Incubator/DotNetCoreTestExtensions.cs.
32 files found to analyze.
Duplicates report was written to Z:\test\cake\forks\cake_incubator_rpriest1260\BuildArtifacts\TestResults\DupFinder\dupfinder.xml
Total time: 00:00:08.3906250
User time: 00:00:06.8281250
Peak virtual memory: 4983MB
Peak working set: 279MB
Duplicate Located with a cost of 142, across 2 Fragments
File Name: src\Cake.Incubator.Tests\obj\Debug\net462\Cake.Incubator.Tests.AssemblyInfo.cs Line Numbers: 1 - 24
File Name: src\Cake.Incubator.Tests\obj\Debug\netcoreapp2.0\Cake.Incubator.Tests.AssemblyInfo.cs Line Numbers: 1 - 24
An error occurred when executing task 'DupFinder'.
Deleting file Z:/test/cake/forks/cake_incubator_rpriest1260/8faf14c9-9236-4c7f-b3d5-c143c42089f7.cake
Duplicates were found in your codebase, creating HTML report...
XSL Transformation complete

I've attached a copy of the dupfinder report from my build. FYI - the attach didn't accept .html files so I renamed it to .txt.

dupfinder.txt

rpriest1260 commented 6 years ago

I found the issue with the dup finder. It appears there is a missing exclusion. Here is the diff:

diff --git a/setup.cake b/setup.cake
index a83d396..b761757 100644
--- a/setup.cake
+++ b/setup.cake
@@ -17,6 +17,7 @@ BuildParameters.PrintParameters(Context);
 ToolSettings.SetToolSettings(context: Context,
                             dupFinderExcludePattern: new string[] {
                                                                BuildParameters.RootDirectoryPath + "/src/Cake.Incubator/**/*.AssemblyInfo.cs",

+                                BuildParameters.RootDirectoryPath + "/src/Cake.Incubator.Tests/**/*.AssemblyInfo.cs",
                                 BuildParameters.RootDirectoryPath + "/src/Cake.Incubator.Tests/*.cs",
                                 BuildParameters.RootDirectoryPath + "/src/Cake.Incubator/CustomProjectParser.cs",
                                 BuildParameters.RootDirectoryPath + "/src/Cake.Incubator/DotNetCoreTestExtensions.cs" },

I will leave it for now until I finish the task at hand. If no one else gets to it, I will check it in.

rpriest1260 commented 6 years ago

Hello. Any thoughts on my pull request to satisfy this issue? I submitted it yesterday.

wwwlicious commented 6 years ago

@rpriest1260 apologies for the radio silence, I'm away at the moment with limited connectivity. I'll catchup on the PR towards the end of next week

rpriest1260 commented 6 years ago

@wwwlicious sounds good. Thanks for the update.