-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
Feature: Add support for Nuget dependency resolution when testing packages #266
Comments
I guess thinking about it some more, another option would be to take the idea of having NuGetForUnity resolving the packages for me, and build an action for it. It could create a project, sets up the manifest with NuGetForUnity and define Then, ultimately all the game-ci action would need to change is the ability to receive a "ProjectDirectory" for package testing, which is a significantly smaller change with no added dependencies on any systems like nuget. In addition this could help resolve, although not directly, the problem of no caching support for package testing mentioned in this issue: #226 Kind of just riffing here, would appreciate any input on what sounds the most logical and reasonable. |
Context
When testing a package that relies on a nuget package, resolving the dependency is painful.
Currently the best work-around I've found is to simply include the
.dll
/s in the repo, which is not ideal for a number of reason.These include:
.dll
/s will clash.It would be nice if during the creation of a TempProject for adding the package under test to, that any defined nuget packages could also be resolved and included before importing the package. This is because the
.dll
/s need to be present before a package can be added to the project, otherwise the editor will fail to compile and as a result fail the CI.This is really only a problem for packages, as it's likely that any project under test has the nuget
.dll
/s in the project already. Tools exist like NuGetForUnity which provide a nice mechanism for maintaining these files in a project.Suggested solution
I believe we could simply leverage the dotnet cli, to download any required
.dll
/s and add them to the project before importing the package.src/main.ts
, add an argument toDocker.run
calledpackageNugetDependencies
.action.yml
to have an optional configuration forpackageNugetDependencies
that can be used when in package mode.run_tests.sh
, once the TempProject has been created, but before the manifest is modified, check to see if any nuget dependencies are defined..dll
/s, and move them into the TempProject "Assets" directoryConsidered alternatives
Another option would be to possibly use git-submodules and store the binaries in a separate repo. The package repo could then depend on the submodule and only resolve it when needed. This still does not really alleviate the pains of binaries now manually being modified and stored in a repo.
In addition, I've also already tried to use the aforementioned NuGetForUnity package. By adding it as a dependency to the
package.json
and trying to react to compilation, based on a suggestion from the maintainer there on this issue. Unfortunately the problem with this approach is that Unity detects the compile errors in the package before any of that code can execute, before the project even opens.This does highlight another possible approach of, when the temp project is built, instead of using the dotnet cli to resolve the packages, the NuGetForUnity package could be added to the manifest of the temp project, along with it's required
packages.config
file that determines which Nuget packages and versions to resolve. I'm less partial to this idea though as it would require the engine to compile multiple times and places a direct dependency on NuGetForUnity.Additional details
I'm really opening the issue to gauge any interest in this idea, and to possibly get some feedback on the general idea. I realize it may be a niche requirement and so thought it was best to open an issue first. If this idea is agreeable to the maintainers, it's something I'm willing to put some effort into implementing.
The text was updated successfully, but these errors were encountered: