v6.18.0 breaks workflows in Unity projects #2248
Comments
|
Thank you for the report. What version of .NET are you on? What happens when you pass the |
|
Unity is a pinch awkward in terms of .NET version, I believe I'm currently using .NET Standard 2.1 but could migrate to .NET 4.8 I'll take a look at CSharpier, I'm not particularly married to either as long I can keep using it to apply standards automatically I'm very happy :) |
|
In that case, I believe your original hypothesis was correct, and you were broken by the upgrade to .NET 6. I don’t believe it will be feasible for you to run dotnet-format via recent versions of MegaLinter while on .NET Standard 2.1. I don’t know enough to advise whether migrating to .NET 4.8 would be sufficient, so that might be a question for Stack Overflow if it matters to you. Let us know if you have any feedback regarding running CSharpier via MegaLinter. Hope you find it as helpful as we have! |
|
.NET Standard 2.1 is closer to .NET 6 than .NET Framework 4.8 is to .NET 6. So in the timeline, .NET core were released, versions 1.0, 1.1, 2.0, 2.1, 2.2, 3.0, 3.1, and there was some extra work on .NET framework versions like 4.7 and 4.8 before stopping at 4.8. That seemed like a good idea, until it was just more confusing than anything, and .NET core was really there to stay. At the same time, to prevent any confusion, the release following .NET core 3 was .NET 5, and the .NET core project was renamed to .NET (without changing pas releases). Now, since .NET 5, the concept of .NET Standard is left behind in favour of a new mechanism to assure compatibility with other releases starting with .NET 5. I don't understand that mechanism yet since a couple years. There will be no more .NET Standard releases, but MS and Dotnet as an organization sticks to their old promises and keeps it working as promised.
https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-1-0 https://dotnet.microsoft.com/en-us/platform/dotnet-standard https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core So following this, if the Unity project and code was made for .NET Standard 2.1, and worked with .NET 5 (.NET 5 was the implemented of .NET Standard 2.1 in the previous MegaLinter release 6.17), there is no real reason for it to not work with .NET 6 in this MegaLinter release 6.18. Thus, I suspect something else, or that the dotnet format needs absolutely to load the code and project with the specific version of the dotnet SDK that the project is defined. |
|
Thanks all for the info :) and a great reminder of .NETs history This definitely seems to be a dotnet format issue rather than a Megalinter issue (that said a breaking change version may indicate the ramifications of the .NET 6 upgrade clearer). The only thing I see left that could be an ML issue that might need fixing is that this all gets reported as a warning and thus our workflow continued passing despite unhandled exceptions when running dotnet format For now I'll look at CSharpier and whether I agree with it's heavily opinionated format or if for now I'll stick on megalinter version 6.17.0 for now The final option I can look at is to force the generation of the csproj files before running megalinter, it's possible with Unity to do this but it's slow as you incur the cost of a project import (several hours for big projects) Thanks again all |
|
@echoix that's indeed very interesting, I never did a single line in C# and never used .NET env, so I'm a total newbie in the topic, and i'm very glad to have you and MegaLinter community to provide support on it :) @sm-quasar > @bdovaz is himself a Unity developer, and he's at the origin of CSharper integration within MegaLinter, so switching to CSharpier is probably a good idea :) |
|
Good to know :) Yea I ran a pass of it this morning and wow is it fast! I only did a quick check of it's formatting but it largely looked like I wanted anyway so I probably will switch to, though may still using an .editorconfig to help with naming conventions in IDEs at least |
|
As @nvuillam says I am a professional Unity developer (+10 years) and in our workflow we use dotnet-format but as we already had it before Megalinter, we have it integrated outside of it. Let's see if I find the time and test it from Megalinter to see if it gives us problems or not. In our case the project is Unity 2021.3.x (LTS) + .NET Standard 2.1. |
|
@bdovaz Nice, I too am a professional unity Dev (just under 10 years). I'm curious about how your setup looks, do you commit the csproj files (I tend not to as they generate a lot of noise in my experience), do you force the generation of them as part of the ci flow, are you using the dotnet format version that comes with .NET 6 or the version that shipped prior to its move to ship with .NET? Any advice appreciated My brief testing this morning seemed to indicate that the latest version of dotnet format shipped with .NET 6 doesn't work with a unity project either (with the .sln and .csproj files removed) Also I appreciate this has now divulged from being about a Megalinter issue |
As I say, we have this pipeline before integrating Megalinter, so it does not use the dotnet format linter of Megalinter. I have to test to see if doing it with Megalinter works the same way. |
|
I don't know what I'm speaking about, but... maybe if it is boring to generate thise .csproj files, an option of MegaLinter (activated or not by default) could generate them during the CI before running dotnet format ? |
|
Ahhh okay, yeah I see how you're getting around the restriction I'm facing, the sync solution approach is one I've used in the past though I think last i tried that it still incurred the import cost, that may have changed now though which would be a big win My use case is currently running this on all PRs as a GitHub action using GitHub hosted larger runners, so I think in order to avoid installing Unity as part of that particular workflow I may swap to CSharpier Appreciate all the advice though and I'll check out the sync solution approach for some future automations I'm planning |
The reason this probably isn't viable is because it's highly dependent on having the right version of unity installed |
|
@sm-quasar with PRE_COMMANDS we can configure MegaLinter to install whatever we want before running linters, so we could define something like that (i don't know if the commands are easy to build) PRE_COMMANDS:
- command line to install unity with desired version
- command line to compile the project |
So it's quite easy to grab the unity version from a unity project to help build those commands, the install is pretty large and would take a while maybe 15 minutes or so network speed depending. But once you know the unity version you've two options GameCI maintain docker images with each version&OS or you could install Unity Hub and use the totally undocumented but very useful cli it offers to install the version and required modules (possibly not needed for this purpose but would need testing) |
In our case, everything that has to do with Unity we do it in self-hosted agents because otherwise as you say, it is not very operative to have to download and install Unity in each build... I know it exists: https://game.ci/docs/docker/docker-images But the truth is that with the complexity that Unity has + the N platforms with their particularities (UWP, iOS, WebGL, etc...) we prefer to have "more control" over what happens at all times and to be able to access by remote desktop to the agents in case something strange happens.
Unity's compilation pipeline is "custom" and does not use msbuild, so there is always a double compilation: your IDE (VS, Rider, ...) + Unity IDE. The generation of the *.sln and *.csproj files is the responsibility of Unity + in the case of VS, an extension managed by Microsoft: https://learn.microsoft.com/en-us/visualstudio/gamedev/unity/get-started/visual-studio-tools-for-unity?pivots=windows. By not using Unity msbuild, the *.sln and *.csproj files are ignored with .gitignore because they are autogenerated under certain circumstances (when creating / deleting a script, when creating an asmdef (= assembly), etc...) so these files are "useless" in the sense that you cannot configure properties of the *.csproj files or use NuGet or practically anything with the .NET ecosystem. All these problems come because historically Unity supported 3 languages (Boo, UnityScript and C#) which is why it could not be "coupled" to msbuild. Now they are slowly solving all these problems: https://blog.unity.com/technology/unity-and-net-whats-next But it is something that until at least 2024 I don't think they will solve. |
I 100% agree with this, I used to operate and manage my previous studio's build farm which all ran Jenkins and a boat load of custom logic to keep things moving smoothly Currently, for cost / infra reasons in a new studio, I'm actually using GameCI's github actions in combination with Github's beta Larger Runners which is working okay so far, but I definitely prefer a higher level of control when it comes to Unity
It's gonna be really interesting to re evaluate ci pipelines once this is done and see if there are more standard approaches that can be taken :) |
A big bravo to @belav for writing CSharpier, who I thought might appreciate seeing this thread and knowing that his tool is distributed via MegaLinter. |
|
I can confirm this for .NET 6.0. (sample.csproj & sample.sln) -> sample.zip |
|
Btw I think we can close this issue as it has a couple of options for a solution
@nvuillam Should we open a new issue for the fact that the issues seen here only result in a warning in the action runner and thus could be failing relatively silently for folk not looking at the comment ML leaves and just seeing the workflow passed and moving on? |
|
@sm-quasar well we have the And do you think CSharpier can completely replace dotnet format? |
@jokay Ah my apologies I misunderstood your issue as resolved, that's very mysterious indeed, to confirm the csproj and sln files are in that '/builds/demo' folder?
Completely, no. CSharpier is opinionated and so has very few config options. However it's opinion is by and large inline with ms standards and so it's working pretty well for me :) I adjusted my .editorconfig to match it's rules so I could still use it for things CSharpier doesn't care about like my naming standards (at least in my IDE anyway, nothing actually forces those now |
|
As I don't know dotnet, i can't help a lot :/ |
It runs on a GitLab pipeline. The source code is in a subfolder |
In your error message, it seems to be specifically indicating it's looked at the '/builds/demo' folder not the 'builds/demo/src' folder which is probably why it cannot find the csproj or sln The reason this used to work for you is that prior to be integrated to the dotnet sdk, dotnet format was happy to not need a csproj or sln, and would just recursively look for .cs files. The .NET 6.0 version now requires a .sln or .csproj which is breaking various workflows. You mention the path is changed to that 'src' subfolder but the error message provided doesn't seem to be searching the path so I suspect however you're running megalinter it is not listening to that path change. How are you changing the path? Are you using the DEFAULT_WORKSPACE config value in your '.megalinter.yml' file? @Kurt-von-Laven @echoix @nvuillam |
|
I get your point but we can't release a new MegaLinter version everytime one of the 100 linters adds a breaking change, else we'd release 50 major versions every year :/ We realease a major version when there is a breaking change in .mega-linter.yml configuration But I agree that we need to do something about dotnet issues... if csproj os in a sub folder, maybe search for it and change the current directory before calling dotnet format? |
|
@sm-quasar Since I didn't use dotnet-format when it was separate from the dotnet sdk (or the .net 5 era), could you help me by telling what did that linter do? Was it simply whitespace checks? If before, it was really only that, and not really advanced checks, I tried out and the closest to the old behaviour I found was But not |
|
dotnet format does more than whitespace checks. It is highly configurable and checks a wide range of formatting issues. One approach might be to remove dotnet format altogether. I suspect it is a major contributor to the image size, and it is certainly the slowest linter in the dotnet image by far in our experience, which gives the inaccurate impression that MegaLinter is very slow in general. I am curious what our users want though after giving CSharpier a try. |
I'm not against that for the vast majority of cases. Like I don't think the main Megalinter image benefits from it, from a maintenance side.
But maybe the dotnet specific flavor is still a good candidate to contain it, where the size increase might justify it. As a quick comparaison, I looked up the sizes of the dotnet/sdk and dotnet/runtime for alpine, and the sdk images are 228 MB vs 36.6 MB. I don't know why but I had more like a 700-800 MB difference in head from my previous investigations on the megalinter sizes. |
@Kurt-von-Laven Is it common for people that were using dotnet-format previously, that would maybe have .sln or .csproj or maybe not, but that have some megalinter configuration errors? Also, how does it work when there are multiple .sln files? Does each folder tree get scanned? Do we treat the dotnet-format linter as a linter that "fixes" .sln files, and we call the .sln files as what to lint? |
It's set to variables:
BASE_PATH: "src"
NUGET_PACKAGES_DIRECTORY: ".nuget"
stages:
- lint
before_script:
- cd ${BASE_PATH}
- dotnet restore --packages ${NUGET_PACKAGES_DIRECTORY}
megalinter:
stage: lint
image: megalinter-dotnet:v6.17.0
script: ["true"]
variables:
APPLY_FIXES: "none"
DEFAULT_WORKSPACE: ${CI_PROJECT_DIR}
CSHARP_DOTNET_FORMAT_DISABLE_ERRORS: "false"
FILTER_REGEX_EXCLUDE: .*.gitlab/.*|.*${NUGET_PACKAGES_DIRECTORY}/.*
LINTER_RULES_PATH: .gitlab/linters
LOG_LEVEL: WARN
VALIDATE_ALL_CODEBASE: "true"File structure looks like this: root
\_ .gitlab-ci.yml
\_ src
\_ demo
\_ demo.csproj
\_ demo.tests
\_ demo.tests.csproj
\_ demo.sln |
The main drawback I see to including dotnet format in the dotnet flavor but not the "all" flavor is that losing linters when switching from a flavor to the all flavor seems like a bad and counter-intuitive user experience that could easily go unnoticed in light of how many linters we have. (I admit though that dotnet format specifically is so slow that its absence may be more glaring, and of all the linters to consider excluding from "all," it would be the top candidate.) For these reasons and the fact that we don't get the maintenance savings unless dotnet format is completely removed, I lean towards removing it from both.
My impression of dotnet format's size comes from the fact that the other flavors we use are all around 2 GB, whereas the dotnet flavor is 3. I don't know exactly where that's coming from beyond the packages you investigated, but 700-800 MB total seems more plausible than 265 MB.
Is your question whether it's common for dotnet-format to fail to find the .sln/.csproj when run via MegaLinter v6.18.0?
No, neither the .sln nor .csproj have to be in the repository root.
I think each subtree gets scanned, but if someone happens to know of or have a repo with multiple .sln files that this experiment can be conducted on, I'm all ears. |
So a MegaLinter 7 thing.
Yes, I think it's better worded this way.
That's interesting to document here.
It's not the repo I was thinking of, but that it's a structure that I sometimes see. Some code in a |
Ok, you convinced me. We disabled dotnet format and are using CSharpier only. Amazing how much faster it is. |
|
So... dear dotnet experts... should we disable dotnet format by default? ^^ Note: it is also used for VB .net... but are still people using this language? |
|
cc @bdovaz |
|
As I said I have no experience with 'dotnet format` from Megalinter but I do have experience using it from outside Megalinter.
https://learn.microsoft.com/en-us/visualstudio/extensibility/internals/solution-dot-sln-file https://learn.microsoft.com/en-us/visualstudio/extensibility/internals/projects It does not work like other linters that directly scan source files (in this case In the following script you can see how it looks for And here it talks a bit about different project types ( If we want to integrate the linter in the best possible way and that the speed of the linter is adequate, we must use it as it was originally created, and it is to scan solutions ( Of course we must allow these solutions or projects to be at different levels of the workspace. In the following example you can see how each folder is equivalent to a solution ( https://github.com/dotnet/runtime/tree/main/src/libraries |
|
In fact, the linter I want to introduce #2269 is also based on analyzing solutions and projects so we would have the same problem again. https://github.com/JosefPihrt/Roslynator/blob/main/docs/cli/analyze-command.md |
Not sure if someone still using VB.NET is interested in linting code at all 😉 |
|
@nvuillam @echoix @Kurt-von-Laven what do you think about what I said? #2248 (comment) I need your opinion to make a PR to modify this behavior in the dotnet format linter and to start with the Roslynator PR which will also be affected by this problem. |
|
@bdovaz this is chinese for me as I don't know dotnet, if you agree together, I'll agree with you haha :) Roslynator can run with the files in the repo, no need to have strange files that are not committed ? If yes, that would be great :) |
|
@nvuillam by "strange files" you mean the sln and csproj files we mentioned? They are necessary but the OP wanted to try to analyze the source files without them. As I told him #2248 (comment), there is a method in Unity to force to generate those files and that is the correct way to do it. And about Roslynator, as I have already said, it is necessary to do it through the sln or csproj files.... There is no way to indicate a folder with files or a list of files. |
|
Hi @bdovaz, I’ve been following this thread with interest as we currently are now experiencing random issues and dramatic slow down in our .net repositories when running megalinter over them with dotnet-format enabled. We have a number of small and large legacy repositories that still need to be maintained which are a mixture of C# and VB.bet with different levels of sln and proj files unfortunately. Can I ask how you see it working when you make the changes? Will you only scan each proj file once regardless of how many files have changed in that proj? If I’m understanding how it works correctly it scans all files regardless of which one has changed in that project? How do you suggest we scan sln files? Does it add a big overhead to performance or should we only scan sln files where we have scanned a proj file linked to it?! (Could get messy if you then started going back down to other proj files!) Do we need to consider the VALIDATE_ALL_CODEBASE here as you could, I’m thinking, search all sln and proj files and pass them one at a time if true? I refer to this here when I also made some points on this. #2249 Either way I’m guessing you will need to pass a workspacePath argument as this is obviously an automated process and a user can’t specify manually? Sorry just my thoughts and mumblings if its helps to bounce some ideas off. Sorry if I’ve misunderstood or this has already been explained somewhere. Tom |
|
@TommyE123 the reality is that both dotnet format and Roslynator (not yet integrated) only work through sln/csproj files and not like other linters to which you pass lists of source code files. By this I mean that I see complicated to optimize the fact that it only analyzes the projects/solutions that have changed files and not all of them, since there is no easy way to relate which files belong to which projects, etc... What I don't know is how it would work (@nvuillam ???) with |
Yeah, I agree that removing dotnet format would have to wait until v7.
I suspect so because of this issue and the failed experiment below, but it's difficult to know since we don't have telemetry (not that I'm advocating for it).
It didn't work out of the box at least. I reproduced one of the many similar error messages below.
Good point! Yes, this sounds like the best path forward, particularly since, as @bdovaz pointed out, we will need this feature for Roslynator anyways. It's nice that this can be done in v6, and we can revisit whether to remove dotnet format when upgrading to v7 in my opinion. I think it may be possible to achieve what you are proposing with For the first iteration, I feel we should focus on getting it to work optimally with |
Another common pattern is shared projects, where it is more like a "code bucket" and has no code, but it groups files together that can be included in the compilation of another project. You can see a small one here : Someone correct me, it's either UWP projects or packaging projects that use this pattern all the time. So a csproj is not the root folder of the code files it contains. This shouldn't be a problem if dotnet-format is given only the project files like now. |
Good to know; that seems like a good test case.
I'm not sure, but at least in our UWP project, the |
|
Ok, seeing that we agree that dotnet format and Roslynator linters are designed to scan A project file ( I don't know what you think @Kurt-von-Laven @echoix but the way I see it, at least these 2 linters would have to ignore the value of I also have my doubts that with the current architecture you can ignore or force to different modes depending on the linter, the value of |
|
About linters that can not handle file or list_of_files mode, it's normal that the value of VALIDATE_ALL_CODEBASE is ignored :) |
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
github-actions
bot
added
the
O: stale 🤖
bdovaz
added
nostale
Describe the bug
v6.18.0 fails to locate an MSBuild project file or solution file and is unable to test our C# scripts and thus cannot validate them.
Unhandled exception: System.IO.FileNotFoundException: Could not find a MSBuild project file or solution file in '/github/workspace'
After the upgrade to the .18 release our megalinter workflow began reporting warnings instead of failing/fixing PRs. We currently run it just against changed files in a Unity project ( this potentially puts us in an odd position as we have no .csproj or .sln committed since Unity will auto gen these and regularly stomps over them making for messy commit history). This was all working fine for us in V6.17.0 But something in the updated version has broken the flow (.Net 6 support maybe?)
For now we've returned to the .17 release but that obviously isn't what we'd like to do long term
To Reproduce
Steps to reproduce the behavior:
Unhandled exception: System.IO.FileNotFoundException: Could not find a MSBuild project file or solution file in '/github/workspace'
See attached logs
Expected behavior
Dotnet Format should have ran against the .cs files without error or warning
Additional context
Attached logs from the two versions in question and our yml file
v6.17.0 Log
SUCCESS-CSHARP_DOTNET_FORMAT.log
v6.18.0 Log
WARNING-CSHARP_DOTNET_FORMAT.log
Our .mega-linter.yml file (renamed for github's own sanity)
mega-linter.txt
The text was updated successfully, but these errors were encountered: