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

Add NuGet Package Import Support #39

Open
Chizaruu opened this issue May 18, 2023 · 10 comments
Open

Add NuGet Package Import Support #39

Chizaruu opened this issue May 18, 2023 · 10 comments
Labels
enhancement New feature or request
Milestone

Comments

@Chizaruu
Copy link
Owner

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

It should be a simple process of adding names and versions.

What type of pull request would this be?

New Feature

Any links to similar examples or other references we should review?

No.

@Chizaruu Chizaruu added the enhancement New feature or request label May 18, 2023
@NoTuxNoBux
Copy link

NoTuxNoBux commented Jun 7, 2023

Will support for other Roslyn analyzers such as Roslynator tie into this? I'm happy to create a new ticket for it if not, I guess it depends a bit on how you're planning to do this.

Currently with the vscode package from Unity (and the other fork from wackoisgod) Unity looks for Roslyn analyzer assemblies inside the project folder with a specific asset tag, passes them to the VSCode extension, which in turn generates entries like this:

<ItemGroup>
    <Analyzer Include="/home/user/project/Assets/RoslynAnalyzers/roslynator.analyzers/4.2.0/analyzers/dotnet/cs/Roslynator_Analyzers_Roslynator.Core.dll" />
    ...
</ItemGroup>

I have a bunch of those right now, but they don't appear to be added to the generated Scripts.csproj.

If it's not yet implemented: the Unity approach is to ask you to drop your analyzers as DLLs inside the project and tag them, but installing through NuGet is much better since you can effectively version properly and don't need to push dependencies to your repository. This package may be able to generate the entries just for the C# project files and ignore Unity altogether.

The downside to ignoring Unity is that you won't get the analyzer messages inside the Unity editor, but the upside is that analyzer errors won't block play mode (and you can't turn Roslyn analyzers off in Unity 2022 any more, whilst you could in 2021) and the analyzers won't slow down editor iteration times if all you want is to see them with OmniSharp, your IDE, and/or use them in CI.

EDIT: I just realized I can try creating a OriginalName.csproj.user file to add the additional Roslyn analyzers, since the Microsoft Unity analyzer is already in here and works at build time. The Microsoft Unity analyzer is also not working for me with OmniSharp, though; possibly because a reference to it is missing in the C# project file.

@Chizaruu
Copy link
Owner Author

Chizaruu commented Jun 7, 2023

The analyzer works. The problems just aren't showing up :c

I posted a picture in #48

@NoTuxNoBux
Copy link

NoTuxNoBux commented Jun 7, 2023

I'm not sure that's the problem, though (I'm the one that reported it, by the way 😄), since it didn't happen on Linux and is Windows-specific (I'm on Linux).

I'll try to dig in a bit more to see what the problem is 🙂 .

@Chizaruu
Copy link
Owner Author

Chizaruu commented Jun 7, 2023

I'm not sure that's the problem, though (I'm the one that reported it, by the way 😄), since it didn't happen on Linux and is Windows-specific (I'm on Linux).

I'll try to dig in a bit more to see what the problem is 🙂 .

I should really invest some time into creating a new VM for specific Linux edge cases xD

@Chizaruu
Copy link
Owner Author

Chizaruu commented Jun 7, 2023

Btw, if you have time, please let me know if the new dev tool extension works, I'm currently locked behind a path issue 😢

I want to know if it solves the analyzer problems ✌️

@NoTuxNoBux
Copy link

NoTuxNoBux commented Jun 7, 2023

Btw, if you have time, let me know if the new dev tool extension works, I'm currently locked behind a dotnet/vscode-csharp#5713

It's working for me in ordinary .NET projects and in Unity projects (using the wackoisgod fork that generates SDK-style projects). With this package I'm currently hitting #54, but it still works, probably because the projects are already properly generated (not getting any analyzer messages to show up in VSCode, though, but I'll dig into that later).

EDIT: But, as mentioned, I am on Linux, so I might not be suffering from the same bug 😄.

@Chizaruu
Copy link
Owner Author

Chizaruu commented Jun 7, 2023

Btw, if you have time, let me know if the new dev tool extension works, I'm currently locked behind a dotnet/vscode-csharp#5713

It's working for me in ordinary .NET projects and in Unity projects (using the wackoisgod fork that generates SDK-style projects). With this package I'm currently hitting #54, but it still works, probably because the projects are already properly generated (not getting any analyzer messages to show up in VSCode, though, but I'll dig into that later).

EDIT: But, as mentioned, I am on Linux, so I might not be suffering from the same bug 😄.

It will generate the projects as intended, but your issue stops dotnet from running restore/build 😅

Hopefully I can patch it soon

@NoTuxNoBux
Copy link

NoTuxNoBux commented Jun 12, 2023

I fixed my problem with the analyzers not working: it was some shenanigans with indeed not having run dotnet restore in the correct folder 🎉 .

FWIW, a heads-up from someone who looked into getting NuGet packages into Unity using standard .NET tooling and built a mostly-working-home-baked-that-probably-doesn't-work-in-all-situations approach for the company he works for before you start on this 😄 : it's unfortunately not just a matter of dotnet restore-ing some additional packages like in ordinary .NET and having Unity pick those up; the process is made much more complex because of Unity. Here are a few problems you will hit:

  1. dotnet restore installs special 'meta packages' such as System.Buffers to the dependency chain so different NuGet packages can depend on different versions of them and have conflict resolution happen correctly.
    • Most of these packages have special _._ assembly files (i.e. none at all) probably because they are built into .NET itself and don't need installing, and sometimes they don't, e.g. shims for System.Text.Json for older versions of .NET.
    • Unity ships its own custom versions of these assemblies in folders such as Unity/2022.3.1f1/Editor/Data/NetStandard/compat/2.1.0/shims/netstandard and - you guessed it - these have different versions that aren't always compatible. These don't appear often for me, and often you can get away with resolving the conflicts manually in .csproj and it will just "work" during builds, but it's hoping for the best.
  2. Unity validates assembly versions to be compatible. If one package wants 4.1 and another wants 4.2 of package A, Unity will complain about that, even though they are obviously compatible according to semver. You can disable this in the Unity project settings.
  3. .NET packages often package the same assembly DLL multiple times: one for .NET standard 2.0, one for .NET 5, and so on; Unity doesn't care and fails hard because you are trying to install the same assembly multiple times - you will need to filter out the appropriate one based on the Unity version and project settings.
  4. Similarly, .NET packages sometimes package native (unmanaged) DLLs: one for Windows x86-64, one for Windows ARM64 (UWP, HoloLens 2), one for Linux ARM64, and so on. In Unity these need to be marked as included only for the platform they target.
  5. Some .NET packages ship their own Roslyn analyzers. If you want these to work inside the Unity Editor itself, they need to be tagged appropriately (see the documentation) or Unity will not pick them up.

If you look at the codebase of e.g. NuGetForUnity, they also have various workarounds for these things. We started out using that and ended up rolling our own in-house thing because it still fails to install e.g. System.Text.Json and couldn't handle Roslyn analyzers properly.

(Off-topic about item 5: starting with Unity 2022 you can't disable Roslyn analyzers inside the Unity Editor any more, and Roslyn analyzer options specified through .editorconfig (required by e.g. Roslynator) aren't passed along by Unity and they don't plan to fix it, so you will get the wrong errors and slower iteration times in the Unity Editor if you take that path. It might be easier to just do what this package currently does and ignore Unity entirely, so you can just run the analyzers in your IDE and CI/CD, leaving Unity behind.)

@Chizaruu
Copy link
Owner Author

Thanks for the great advice; I'll see what I can do in my free time. 😄

@NoTuxNoBux
Copy link

To answer my own question from before about Roslyn analyzers: I got this working with how this package works right now by making a custom .csproj with the necessary analyzers in, and creating a Scripts.csproj.user (or whatever project you want it in; I have a Unity asmdef for Scripts, hence the name) that just links to that project additionally.

This pulls in the necessary analyzers I want, which works for OmniSharp and dotnet build 🎉 .

@Chizaruu Chizaruu pinned this issue Jun 12, 2023
@Chizaruu Chizaruu added this to the Backlog milestone Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants