-
Notifications
You must be signed in to change notification settings - Fork 899
Fix for locating lib folder on netcore #1499
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
Fix for locating lib folder on netcore #1499
Conversation
As I've already mentioned, the lib folder isn't going to be around for .NET Core, so we'll need to come up with some other solution. |
This is aimed to be a hotfix to enable msbuild tasks that leverage Not trying to dictate / interfere with how you chose to address this longer term / in the future once /lib is removed. |
If the decision is not to adopt this fix, I am happy to switch |
The problem is that this isn't even a good enough temporary fix, because it won't work on non-windows platforms at all. I think it makes sense to figure out to handle this MSBuild properly vs. temp fixes and forks. |
Considering that Nerdbank.GitVersioning is working properly with .NET Core without needing changes makes me think the solution isn't to change LibGit2Sharp. |
Nerdbank.Gitversioning works around this issue by adding significant plumbing code to its msbuild tasks for netcore. I would rather see the underyling issues fixed than complicate my msbuild task code. If its fixed it also means we dont need to compile for netcore we can compile for netstandard. You can see all the workaround logic here: https://github.com/AArnott/Nerdbank.GitVersioning/blob/master/src/MSBuildExtensionTask/ContextAwareTask.cs The fix I put in place fixes my issue on windows. You are right I did not also try to fix this for other platforms If libgit2sharp exposed a way for the consumer to configure where it should look for its native assemblies that could be another way to solve this. |
Not sure why the travis build failed.. Seems to be a problem with nuget restore. Don't think it's anything I have permissions to fix. |
Always use Assembly.CodeBase and then Assembly.Location for native search path.
I am closing this, as I think you are right. Fixing this for windows only is not a good enough solution. |
FYI, a primary blocker from doing this is that the library search path can't be altered from within the process on unix like it can be from Windows. Once the process has been started, it's too late to alter it. |
Thanks for the info, that makes sense. I think I have come to the conclusion that the only way to handle this for the msbuild task scenario, accross all platforms, is to have the msbuild task assembly loaded into, and executed from, a custom 'AssemblyLoadContext': Which can then implement that method to resolve native lib's locally to the task assembly. I've raised an issue over on UtilPack: stazz/UtilPack#14 as I think it's something the msbuild task factory can take care of there. |
That does appear to be how Nerdbank.GitVersioning has handled it: https://github.com/AArnott/Nerdbank.GitVersioning/blob/master/src/MSBuildExtensionTask/ContextAwareTask.cs#L72 |
Yup. I just feel strongly that putting this logic in the msbuild task itself is probably the wrong place. MsBuild tasks shouldn't have to be concerned with also being a proxy to themselves imho - they have enough to worry about. I also am not interested in compiling for netcore, I want to have a single compilation for netstandard! Huge respect to AArnott for the solution though - I am not dissing, just expressing my artistic licence :-) |
This fixes #1498