Skip to content
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

Migrate to "new" .csproj format. #20305

Closed
Nufflee opened this issue Jul 20, 2018 · 24 comments
Closed

Migrate to "new" .csproj format. #20305

Nufflee opened this issue Jul 20, 2018 · 24 comments

Comments

@Nufflee
Copy link
Contributor

Nufflee commented Jul 20, 2018

New .csproj format supports Include and Exclude globs (like <Compile Include="**\*.cs" Exclude="some other glob"/>). More info here. And it's not .NET Core only according to this.

This would fix the issues discussed in this PR (#12917 and #12415) as we don't have to add <Compile/> tags ourselves and we can let people exclude files that they don't want compiled.

I'm not sure how Mono and MSBuild work with the new format but I'm happy to look into it and open a PR if this is a acceptable solution.

cc @neikeq @Zylann @NathanWarden @Scellow

@mattiascibien
Copy link
Contributor

mattiascibien commented Jul 20, 2018

@Nufflee if I recall well and based on this issue mono/mono#9475 version 5.12 used by Godot should not have problems using the new .csproj format also because Godot documentation recommends using the <PackageReference> tag for referencing nuget packages which is for the new format.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@mattiascibien Thats awesome, still waiting for neikeq's opinion though.

@mattiascibien
Copy link
Contributor

You can try to remove the <Compile Include="File.cs" /> lines on your godot csproj. On dotnetcore at least, every .cs should be included by default (without specifying any compile directive). Not sure about Mono in any case.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

That's not a case with .NET Framework/Mono.

@neikeq
Copy link
Contributor

neikeq commented Jul 20, 2018

You are free to use the method you choose. The idea is not to impose a specific one. By default, Godot will add include files one by one.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@neikeq I'm not sure if I understand what you're saying. Do we want to migrate to this new format or not?

@neikeq
Copy link
Contributor

neikeq commented Jul 20, 2018

I was referring to globs for include items.
Regarding the additions to the format, what would you like to see? I was considering having our own Sdk at some point for simplifying things, but I haven't looked more into it.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@neikeq Hmm, I don't think we're on the same page here. Can we continue discussing this in IRC because this is already getting quite lengthy.

@mattiascibien
Copy link
Contributor

mattiascibien commented Jul 20, 2018

@Nufflee actually I had the possibility to test and using <Compile Include="**\*.cs" /> worked fine. Godot anyway still tries to add the file as expected.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@mattiascibien Test what? New .csproj format?

@mattiascibien
Copy link
Contributor

mattiascibien commented Jul 20, 2018

@Nufflee I tried to use <Compile Include="**\*.cs" /> and it got compiled without problems.

EDIT: did not appear corretly before, edited the post. Sorry.

@neikeq
Copy link
Contributor

neikeq commented Jul 21, 2018

@Nufflee I'm not sure what do you mean by supporting the new format.

You mention include globs, but as far as I know, that's something has existed for a very long time.

Could you explain with more details what you are requesting?

@mattiascibien
Copy link
Contributor

@neikeq I think I means to make Godot sharp use globs instead of adding every CS file to the project on order to fix the problem that happens when deleting the script from Godot editor.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 21, 2018

@neikeq So, Include and Exclude globs weren't a thing for a very long time. They were introduced among with the new/different .csproj format when .NET Core was released but they can be used with .NET Framework as well. You can read more about this new format here.

This format fixes basically all problems we had with deleting, moving and excluding .cs scripts but to make use of it we need to migrate all old .csproj's to this new format which will requires us to write a tool for that or there might be one already available.

@neikeq
Copy link
Contributor

neikeq commented Jul 21, 2018

That's what I understood at first, not sure why the confusion. As I said, the idea is not to impose any style for including files. If you want to use globs, you can use them, but Godot will add files by default.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 21, 2018

@neikeq You're still missing the point. To use Include globs the whole .csproj needs to be changed. We cannot use Include globs with our current .csproj format. Include globs solve all problems we had with including file by file. We either need to completely migrate to this format which has no drawbacks or anything compared to what we're using right now, or we can stay with the current .csproj format and figure out how to solve the issue ourselves or not solve it at all.

The point is that if we completely migrate to this new format, we wont loose anything, it will just solve our issues but it wouldn't make sense to do both.

@PJB3005
Copy link
Contributor

PJB3005 commented Jul 21, 2018

What you're saying about include/exclude globs is wrong. "Include/Exclude globs" (more specifically, wildcards) in item definitions in MSBuild files have existed for ages, and this link from 2008 proves it. You're confusing them with ".NET Core made a default <Compile Include="**\*.cs" /> definition", which is true and definitely useful.

You can already just write your own <Compile Include="**\*.cs" /> in your .csproj file and it will work. That said I've had Visual Studio crap itself when handling that csproj file.

Not that I don't support moving to the new format.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 21, 2018

@PJB3005 That might actually be true, my bad i guess. Though I still don't understand why basically no IDE's use this. Not too sure what should we do here, maybe have a setting somewhere to "turn this on" and make Godot stop adding files to the .csproj

@mattiascibien
Copy link
Contributor

@Nufflee what I was thinking too. I'll try to implement it

@josh18
Copy link

josh18 commented Jan 11, 2019

maybe have a setting somewhere to "turn this on" and make Godot stop adding files to the .csproj

Looks like there is an option now (3.1): #23505

I don't know much about c# but how come we don't use <Compile Include="**\*.cs" /> in the project file by default?

@aaronfranke
Copy link
Member

Does anybody have any information on what version of Mono is needed to support this?

Since this discussion involves using a different format for the csproj file and old projects might need converting, I suggest this be put on the 4.0 milestone, along with #12917.

@neikeq
Copy link
Contributor

neikeq commented Aug 13, 2019

I just re-read this issue and I think I understand now what this was actually about. I got confused with the <Compile Include="**\*.cs" /> talk. What you really want is to use project Sdks, right?
If that's the case then it's going to happen eventually, but I would like to make our own Sdk for Godot instead of using Microsoft.NET.Sdk.

@PJB3005
Copy link
Contributor

PJB3005 commented Aug 13, 2019

@aaronfranke From my own testing I haven't found a Mono version that doesn't support it, so...

@neikeq
Copy link
Contributor

neikeq commented Aug 20, 2020

This was already done for 4.0 in #40595, and #41408 is a back-port for 3.2.

@neikeq neikeq closed this as completed Aug 20, 2020
@akien-mga akien-mga added this to the 4.0 milestone Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants