Skip to content

Conversation

@mateoatr
Copy link
Contributor

Fixes #42772. I took the easy path here - whenever the app is single-file, add the exe's dir to NATIVE_DLL_SEARCH_DIRECTORIES. Additionally, if the user sets -p:IncludeNativeLibrariesForSelfExtract=true, add the extraction path to the search directories.

@ghost
Copy link

ghost commented Sep 29, 2020

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

@vitek-karas
Copy link
Member

I think the logic should be:

  • If this is a single-file - add the bundle's directory to the native search paths
  • If we extracted anything - add the extraction directory to the native search paths

This should specifically cover the 3.1 backward compat mode (I know it works today on Windows, but it won't work on Linux ).

We obviously need tests for this - we can start with validating the content of the NATIVE_DLL_SEARCH_DIRECTORIES as that is easy enough to do in the tests.

Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests?

@elinor-fung
Copy link
Member

This should specifically cover the 3.1 backward compat mode (I know it works today on Windows, but it won't work on Linux ).

What is broken with 3.1 compat mode on Linux for this? It should (with the fix for actually extracting) be able to search for native libraries based on the managed assembly's path?

@vitek-karas
Copy link
Member

You're right that it will basically work - but as you pointed out - the design called for setting the extraction path as one of the native search directories - which this should fix. There are still potential scenarios (plugins) where not setting it will break the app (mostly on Linux), but I agree that they're not that common.

@mateoatr
Copy link
Contributor Author

I removed needs_native_libraries_extraction from the manifest flags and changed the logic so that whenever the extraction dir exists, we add it to the native search paths.
For testing I'm just validating that we put the bundle's dir and the extraction dir in NATIVE_DLL_SEARCH_DIRECTORIES.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to make sure the bundle actually has something to extract so that it creates/adds the extraction directory? By default, we won't have anything to extract on Linux, right?

@agocke
Copy link
Member

agocke commented Oct 1, 2020

@mateoatr I don't think these changes are strictly necessary, they're more test hygiene suggestions. If you think it would be faster to deal with these separately, that's fine. We can merge this change, and take the modifications in another PR.

@mateoatr
Copy link
Contributor Author

mateoatr commented Oct 1, 2020

@agocke I had some issues testing locally, but I just pushed your suggestions. I think we can merge after all tests run in the CI.

@agocke
Copy link
Member

agocke commented Oct 1, 2020

Yeah, I've been trying to work on stuff locally and mostly got stuff to work by building everything, then runnign the apphost test by doing ~/runtime/dotnet.sh build then ~/runtime/dotnet.sh build -t:test

.And.HaveStdOutContaining("ExecutingAssembly.Location: " + extractionDir) // Should point to the app's dll
.And.HaveStdOutContaining("AppContext.BaseDirectory: " + extractionDir) // Should point to the extraction directory
// In extraction mode, we should have both dirs
.And.HaveStdOutMatching($"NATIVE_DLL_SEARCH_DIRECTORIES: .*{extractionDir}.*{bundleDir}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I assume the Matching is turning this into a regex, meaning that the \s now need to be escaped on Windows?

@agocke
Copy link
Member

agocke commented Oct 2, 2020

Moving us back to the previous commit, since that passed tests, so we can get this in today

@agocke agocke merged commit 0403018 into dotnet:master Oct 2, 2020
agocke pushed a commit to agocke/runtime that referenced this pull request Oct 2, 2020
)

Add two directories to NATIVE_DLL_SEARCH_DIRECTORIES to single-file bundles:

1. The bundle exe directory
2. If the bundle extracts any files, the extraction directory

Fixes dotnet#42772

(cherry picked from commit 0403018)
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Single-File] DllImport fails to find native library

4 participants