cake-build / cake

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

ParseProject doesn't support new csproj-format #1662

Open bjorkstromm opened 7 years ago

bjorkstromm commented 7 years ago

ParseProject doesn't support new csproj-format. One essential thing missing is support for multiple target frameworks.

bjorkstromm commented 7 years ago

@cake-build/cake-team is there any other thing we should support as well?

Also, design stuff. In ProjectParserResult I would go for adding a new ISet<string> TargetFrameworks and maybe obsoleting the old string TargetFrameworkVersion?

devlead commented 7 years ago

Actually TargetFrameworkVersion is used by default if it's one framework specified.

bjorkstromm commented 7 years ago

Worse yet. I thought Cake.Common handled new csproj format. But it doesn't. Getting Error: Failed to parse project properties when parsing new csproj format. Cake.Recipe handles this by using ProjectParser in Cake.Incubator. Maybe we should get this into Cake.Common soon?

bjorkstromm commented 7 years ago

Raised issue in Cake.incubator https://github.com/cake-contrib/Cake.Incubator/issues/37 regarding multiple target frameworks.

But I'd strongly suggest that we should add support for new csproj format in Cake.Common as well.

gep13 commented 7 years ago

@mholo65 Cake.Incubator is intended to be the project that tests out nee functionality before pulling into Cake proper. I would suggest we get it fully baked over there, and then pull into Cake.

matkoch commented 7 years ago

If I'm not mistaken, the current implementation will fail to parse for projects that utilize includes. And what about conditions?

gep13 commented 7 years ago

@matkoch that is certainly a possibility. Could I get you to raise an issue kn the Cake.Incubator repository?

kcamp commented 7 years ago

@matkoch

Yes, there is at least one known issue in the way the parser handles includes. See #1299 as resolved by #1529.

Conditions are also something that are not supported fully; general Configuration|Platform conditions are handled, but not all permutations. I've developed an ExtendedProjectParser based on a reference to Microsoft.Build that allows for a more exacting evaluation of the project. If either of those would be deemed useful or more appropriate for Cake.Incubator, I can see about fitting them in.

matkoch commented 7 years ago

@kcamp

Is there any reason why you use XML parsing? The possibilities of mutating in MSBuild are numerous, so why not utilizing MSBuild itself to get the values, like here. It's not optimal though.

evil-shrike commented 7 years ago

+1. Please.

Also PackageReference info should be added (it's missing in ProjectParserResult) Currently References and ProjectReferences are supported.

BTW in the documentation it's unclear what type these collections have:

ProjectReferences | ICollection<T> | Gets the references to other projects.
References | ICollection<T> | Gets the references.

ICollection of what?

gep13 commented 7 years ago

@evil-shrike just to confirm, are you using the Cake.Incubator addin?

evil-shrike commented 7 years ago

@gep13 no, I just wrote ParseProject and it worked (failed), I didn't use any directives.

gep13 commented 7 years ago

Have a look at the Cake.Incubator addin then. It has support for parsing the new project format.

evil-shrike commented 7 years ago

@gep13 I've tried Incubar's version and it works fine, thanks. Bit it works fine if I use an overloaded method which is absent in Cake:

    var proj = ParseProject("./awesome.csproj", configuration: "Debug");

This fails:

    var proj = ParseProject("./awesome.csproj");

How to tell Cake to use a method from Addin if the same method exists in Cake itself?

As I can see you wrap all new project properties for SDK-projects into NetCoreProject class instance (in NetCore prop): proj.NetCore.PackageReferences.First().Name Let me put your attention that SDK-project is not always .net core projects. It's just a new project system. They can target full .net fw as well. Maybe it'd make sense to choose another name?

wexman commented 6 years ago

So Cake - a C# build tool - is not able to parse C# project files?

Are you freaking kidding me?

gep13 commented 6 years ago

@wexman have you taken the time to read this issue? If you had, you would understand that we are currently using the Cake.Incubator addin to provide this functionality.

Have you tried using this?

daveaglick commented 6 years ago

@wexman Parsing MSBuild project files is actually very complicated. Also note that project files are more a function of MSBuild than the C# language. Between the different types of projects, different SDKs and project file formats, and different MSBuild tasks and targets, there’s a lot to them under the hood. There’s also a difference between “parsing” the files to determine the structure of the file (as Cake does) and “evaluating” them to figure out how MSBuild will interpret them (as @matkoch suggests).

Can we please leave the attitude out of this, especially without understanding all the facets of the problem. We’re all volunteers here.

wwwlicious commented 6 years ago

If you take a look at the MSBuild code you can get a sense of how much is going on where the project system is concerned. Quick version is 'a lot'

The parsing in cake.incubator is a 'best effort' at reading and providing the information available without being concerned with build environments / imports / variable binding and evalutions and conditionals.

Instead it takes a few fundamental parameters (config and platform) and an awareness of the default values in the project system to try and output information which is hopefully useful in helping the routing of projects to various tasks during cake builds.

Using the MSBuild project system directly (along with the full MSBuild arguments you would intend to use) is obviously the more complete solution.

You could even get the msbuild /pp option to inline imports in the correct order or output a merged project file using the MSBuildEmitSolution=1 environment variable which outputs the .metaproj file msbuild actually uses to build a solution. msbuild tips

wexman commented 6 years ago

So now with version 0.26.0 of cake, the incubator addin (1.7.2) doesn't work anymore (because it was built against cake.core 0.22.0). How am I supposed to get the version info I need now? Can you please stop breaking things between every 2nd release?

devlead commented 6 years ago

@wexman we've had 7 intentional breaking changes in 87 releases. Most of those have been because dependencies have introduces breaking changes. I.e. switching from .NET Standard 1.6 to 2.0 is a pretty big change tooling wise. We do try hard to not introduce breaking changes, and that we break every second release simply isn't true.

0.25.0 still works, so if you pin to that your build should just continue work as before.

gep13 commented 6 years ago

@wexman said... Can you please stop breaking things between every 2nd release?

Introducing breaking changes is not something that we do lightly. We only do this when we feel it is absolutely necessary. We do however go out of our way to try to let the Cake community know about these upcoming breaking changes ahead of time, so that they can be prepared, and we also recommend pinning to a specific version of Cake, so that your builds don't get affected by these changes. Have you pinned your Cake and Addin/Tools versions? Are you seeing the notifications about upcoming breaking changes?

wexman commented 6 years ago

Sorry if I'm touching a sore point here, but in other words it is not currently possibly to parse a c# project file using the latest version of Cake?

bjorkstromm commented 6 years ago

@wexman oh, it sure is possible. Just pass --settings_skipverification=true to cake.exe, that will disable the verification check, and hopefully Cake.Incubator addin will just work. More info here

gep13 commented 6 years ago

@wexman said... Sorry if I'm touching a sore point here, but in other words it is not currently possibly to parse a c# project file using the latest version of Cake?

No, not a sore point at all.

What you said was factually incorrect, and @devlead wanted to ensure that you understood that.

Quite frankly, the way that you said it came across and rude and entitled, and I also wanted to make sure that you understood that we don't go into a breaking change lightly, however, these things do happen, and there are documented mechanisms to do what you want.

The solution that @mholo65 pointed out was listed in the release notes associated with the release of 0.26.0. Did you happen to read those?

https://cakebuild.net/blog/2018/02/cake-v0.26.0-released

lonix1 commented 2 years ago

I'm using the "incubator" version of ParseProject; thanks to @gep13 for pointing me in the right direction.

Some feedback:

  1. It supports this traditional style:

    <Project Sdk="Microsoft.NET.Sdk">
    </Project>

    But not this form:

    <Project>
      <Sdk Name="Microsoft.NET.Sdk" />
    </Project>

    ...Which is common in large monorepos, because lots of reusable config is hidden away in Directory.Build.props (etc.) files. It errors with "Error: Failed to parse pre VS2017 project properties".

  2. It does not support nuget's new "Central Package Management" feature. So when versions are kept in a Directory.Package.props file instead of the Foo.csproj file, then the ProjectParserResult.References has null for the versions.

gep13 commented 2 years ago

@lonix1 the best place to capture these things would be on the repository for that addin: https://github.com/cake-contrib/Cake.Incubator/issues. And if possible, we welcome PR's to happen address these issues. Once things are fully working in Cake.Incubator, we can then look to bring it into Cake proper.

lonix1 commented 2 years ago

Sorry gep13! There are so many repos I was not sure what goes where. I'll recreate the issue over there.

daverogersvald commented 1 year ago

This was exactly what I needed, but even the Incubator project does not support it (if using v 3.0.0).

Is there any other way of getting data out of the csproj file dynamically? Or do I need to manually type in things like Description (which is not really a good solution, especially when you are iterating your projects in the build step).

hotchkj commented 6 months ago

I realise this is an old issue, but since the last comment is a query on how to do this and there are related open issues on this in Incubator, I'd offer the following thoughts having done this myself:

See MS docs on the subject for more details.

Hope that helps someone.