Use path for loading native dependencies #10347
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brettfo
suggested changes
Oct 28, 2020
|
|
||
| Environment.SetEnvironmentVariable("PATH", path.Replace(probePath + ";", "")) | ||
| let mutable _p = null | ||
| addedPaths.TryTake(ref _p) |> ignore |
Member
There was a problem hiding this comment.
I'm having a hard time tracking what these new bits do. Can you refactor to something like let addToPath(existingPath: string, extraDir: string) -> string/let removeFromPath(existingPath: string, removeDir: string) -> string and add unit tests? Then these locations simply pipe in to and out of %PATH%.
Contributor
Author
There was a problem hiding this comment.
addedPaths is the collection of paths that have been added. Collected, so that they can be removed in dispose. But I can refactor some stuff yes.
KevinRansom
force-pushed
the
usepathfornativebinding
branch
from
1174a5c to
ba8a100
Compare
cartermp
approved these changes
Oct 29, 2020
brettfo
approved these changes
Oct 29, 2020
nosami
pushed a commit
to xamarin/visualfsharp
that referenced
this pull request
Feb 23, 2021
* Use path for loading native dependencies * feedback * Update Microsoft.DotNet.DependencyManager.fsproj * Update VisualFSharpFull.csproj
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So we still have some issues loading native dll's in scenarios where the Coreclr does not raise a ResolvingUnmanagedDll event notification.
So ... what we do with this PR, is for any native dll that msbuild identifies as should be copied to the output directory, we add the directory containing that dll to the Path environment variable for the process. This should increase the range of scenarios where native .dll loading currently fails.
I also have made the code netstandard2.0, since fcs is currently netstandard2.0 only.
/cc @jonsequitur --- this should fix the sqllite notebook issue.