-
Notifications
You must be signed in to change notification settings - Fork 36
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
Resolve dependencies using UnityNuGet scoped registry #9
Resolve dependencies using UnityNuGet scoped registry #9
Conversation
Thank you for your pull request! Let me explain our distribution policy including dependent libraries. First of all, I knew there is a great project called UnityNuGet where you can get dependent packages. However, we decided not to adopt that approach in our project. The first reason is the conflict of assemblies. The second reason is the responsibility for distribution and installation. In enterprise game development companies, for compliance reasons, they may scrutinize the libraries they are using. Initially, we considered having users obtain the dependent libraries from NuGet. While UnityNuGet is convenient and provides a clean solution, it is realistically challenging to ask our customers to use it, from what we have observed with several developers. Therefore, although we are sorry that you took the time to create a PR, we do not currently have any plans to accept methods that rely on unofficial UPMs, not just UnityNuGet. |
@mayuki I understand the concerns but let me argue them and try to partly redirect the purpose of this PR with some adjustments. Conflict resolutionI really missed the point of your explanation because I see that you agree with me that UnityNuGet is the best solution for this case. Security concerns / Enterprise worldWe develop a product which serves as a platform for XR content developers. We solve our dependencies with UnityNuGet / OpenUPM (which does upstream https://openupm.com/nuget/) because this target of developers in our case has very little experience in development and that is why we opted for the simplest way which is the scoped registries and that everything is solved "magically" without any action to be performed by users other than the initial setup of the project by adding the scoped registries through the UI or in some cases not even that because we distribute a "template" project with everything preconfigured. As you can understand, in our case it is crucial that all dependencies can be resolved through packages automatically because our users simply install our package and this in turn brings all the necessary dependencies either ours or third parties. And in successive updates they only have to update our package and nothing else, they don't have to deal with copying dlls by hand, etc... And if we have more advanced users who want to use other assets of OpenUPM, NuGet or Asset Store, the fact of solving everything as packages, facilitates this work enormously. There are more special cases as you mentioned. For these cases you can "defend" how this project (UnityNuGet) is open source, is on GitHub and is nothing "obscure" and is supported by a Unity employee (xoofx). Besides being a Docker container it is very easy to host it on your own without relying on Azure hosting by default (and then it would only be to change the url of the scoped registry in the manifest.json file). On the other hand, for cases where it is "not defensible" or external connections are not allowed, you can embed it, i.e. copy from Therefore, and if you agree, I propose the following modifications in this PR:
This way it would work as it works until now only that the assemblies instead of in It also makes it easier for you to maintain this project because if you need to update the dependencies in the future you don't have to go to each NuGet page in the dependency tree and download them one by one. Just update the top-level packages (System.IO.Pipelines and System.Runtime.CompilerServices.Unsafe) and copy back from |
@mayuki what do you think of my previous message? It would allow to solve the problem this PR is trying to solve while not hurting the use cases you are concerned about. |
Is it possible to use UnityNuGet to get the package dependencies and copy them when creating the .unitypackage (which will be extracted under Plugins)? Also, if we use UnityNuGet to resolve dependencies, can we choose .NET Standard 2.1 as the target framework for the assembly? |
|
Sorry for the delay in response. The idea of using UnityNuGet to resolve dependencies and packaging the result sounds good! |
@mayuki the PR is ready. I have updated the README.md indicating the 2 ways of installation: with UnityNuGet or without it. In the second case it works exactly the same as before, you have to import a *.unitypackage for The nice thing is that the way I set it up makes it infinitely easier to lift the dependencies because you only have to edit the version in the The only thing missing on your side after the merge is to release another release to be able to publish the 2 new *.unitypackage and change the links in the README.md file. |
Thanks for the update! I will review it now. |
It looks very nice and simple.👍 I found that the display name of the package from the unitypackage (built-in Package) looks like I feel a little uncomfortable having the package IDs listed under Packages in the Package Manager and the Project View. |
@mayuki If you clear the UPM cache on your computer and reimport the project from scratch, you will see that packages now have a display name. |
src/YetAnotherHttpHandler.Unity/Assets/Editor/Scripts/PackageExporter.cs
Outdated
Show resolved
Hide resolved
src/YetAnotherHttpHandler.Unity/Assets/Editor/Scripts/PackageExporter.cs
Outdated
Show resolved
Hide resolved
"dependencies": { | ||
"org.nuget.system.io.pipelines": "7.0.0", | ||
"org.nuget.system.runtime.compilerservices.unsafe": "4.5.3" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If install the package by referring to git without using a scoped registry, it seems to cause an error due to these dependencies.
[Package Manager Window] Cannot perform upm operation: Unable to add package [https://github.com/bdovaz/YetAnotherHttpHandler.git?path=src/YetAnotherHttpHandler#feature/resolve-dependencies-using-unity-nuget]:
Package com.cysharp.yetanotherhttphandler@https://github.com/bdovaz/YetAnotherHttpHandler.git?path=src/YetAnotherHttpHandler#feature/resolve-dependencies-using-unity-nuget has invalid dependencies or related test packages:
org.nuget.system.io.pipelines (dependency): Package [[email protected]] cannot be found [NotFound].
UnityEditor.EditorApplication:Internal_CallUpdateFunctions ()
You may need to create a built-in package for dependency reference and change it to be referred from YetAnotherHttpHandler.Unity project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mayuki but of course, this problem occurs if you have not imported before the *.unitypackage which is precisely the one that includes those packages so that then the error does not appear.
If you try to directly install the package without installing the dependencies this will happen.
Precisely for that reason in this PR apart from adding a second installation method (UnityNuGet) in Readme.md I have also inverted the order of the steps, that is to say, in the case of the installation without UnityNuGet I have expressly put that first you have to import the *.unitypackage file of the dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I understand the reason for the problem.
Our policy is to avoid creating dependencies on other packages.
This is because it becomes a problem if the dependency is already managed by another mechanism, such as internal UPM.
In our development environment, libraries are distributed under their own internal package names. For example, if System.Buffers
has already been distributed/installed (e.g. com.example.system.buffers
package), other dependent libraries (Grpc.Net.Client, System.Memory ...) can be moved from Packages
to Plugins
to ignore package dependencies, but YetAnotherHttpHandler will fail to resolve dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But what I don't understand is, this problem I didn't create it already existed.
I mean...
Actually, in the main
branch it says to first import the package with Git and then import the unitypackage with the dependencies. Even if it allows you to install the Git package because the package.json has no dependencies, it will not compile because you have not imported the unitypackage file before.
That is, the order of the instructions should be reversed as I have done in this PR in the Readme.md.
And on the other hand, if a package really has dependencies why shouldn't they be put? I don't understand, that's how this UPM / NPM system works, it's the natural thing to do.
To work with OpenUPM / UnityNuGet it is necessary that the versioned package.json file has them, otherwise this PR loses all sense and all the time invested by me.
I'm really trying hard to make it work "as before" and through this OpenUPM / UnityNuGet flow that has become almost a standard among developers.
What do you suggest to keep this possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. My apologies for misunderstanding.
Given the use of UnityNuGet, it's unavoidable to list dependencies here.
As a result, I understand that if you install via git and without using UnityNuGet, it's necessary to install a .unitypackage that includes the org.nuget.*
packages.
On the other hand, the problem I was having is when a dependency is already installed from elsewhere and the package cannot be installed by UnityNuGet or .unitypackage. Ignore the package when Unity Package Manager cannot resolve the dependency, I wish I could...
While I believe that directing to install from a .unitypackage will solve the problem in most use cases, could there be a solution to address the following case?
flowchart TB
UnityProject-->YetAnotherHttpHandler
UnityProject-->InternalLib
YetAnotherHttpHandler --"org.nuget.system.buffers"--> System.IO.Pipelines
YetAnotherHttpHandler --"org.nuget.system.runtime.compilerservices.unsafe"--> System.Runtime.CompilerServices.Unsafe
subgraph InternalUPM
InternalLib--"com.example.system.buffers"-->System.Buffers
end
subgraph .unitypackage
System.IO.Pipelines --X--> System.Buffers
System.IO.Pipelines --"org.nuget.system.memory"--> System.Memory
System.IO.Pipelines --"org.nuget.system.threading.tasks.extensions"--> System.Threading.Tasks.Exensions
System.Memory --X--> System.Buffers
System.Memory --"org.nuget.system.numerics.vectors"--> System.Numerics.Vectors
System.Memory --"org.nuget.system.runtime.compilerservices.unsafe"--> System.Runtime.CompilerServices.Unsafe
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second package YetAnotherHttpHandler.Unity.Dependencies
is a package solely for dependency resolution for users using UnityNuGet. It is not intended for use with OpenUPM.
The aim is to complete the installation of the package from the two URLs below after configuring UnityNuGet.
https://github.com/Cysharp/YetAnotherHttpHandler.git?path=src/YetAnotherHttpHandler
https://github.com/Cysharp/YetAnotherHttpHandler.git?path=src/YetAnotherHttpHandler.Dependencies
This proposal aims to keep an easy path for UnityNuGet users while there is a unique circumstance of not wanting to specifiy dependencies on YetAnotherHttpHandler
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mayuki okay, and do you have an idea where to create it? Because that folder name you already have it reserved and it has a csproj in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current YetAnotherHttpHandler.Unity.Dependencies is a development helper tool project for copying DLLs.
I think it's okay to replace its contents because its role is ended by using UnityNuGet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mayuki seeing the YetAnotherHttpHandler.Unity.Dependencies.Sync
project loses its sense no? Because it is already going to be solved from UnityNuGet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct, it will become completely unnecessary.
@mayuki check my changes because I think everything we talked about is already resolved. |
To facilitate the installation and above all, to avoid collisions with assemblies, I add in this PR the changes with instructions included to adapt it to use this scoped registry.
By the way, I am mantainer of the mentioned repository: https://github.com/xoofx/unitynuget