Skip to content
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 e_sqlite3 not found for vsto apps. #553

Merged
merged 4 commits into from
Nov 7, 2023

Conversation

farzonl
Copy link
Contributor

@farzonl farzonl commented Aug 31, 2023

Problem:
In issue #552 I identified that VSTO is copying SQLitePCLRaw.core.DLL to C:\Users\username\AppData\Local\assembly\dl3\<HASH_STRINGS>\ SQLitePCLRaw.core.DLL but was not copying e_sqlite3.dll with it. SQLitePCLRaw.core.DLL is expecting to find e_sqlite3.dll in the same directory level as itself or a few levels down from it at <same_path>\runtimes\{os-arch}\native\e_sqlite3.dll For VSTO apps nothing is copying the native dependency over. so e_sqlite3.dll isn't propogated.

The Fix:

using System.Reflection.Assembly.GetExecutingAssembly().CodeBase we can get the original path of the copied over binary. This will be the same path where e_sqlite.dll lives. Using Path.GetDirectoryName We get the path without the base name and build out a path using Path.Combine and the libname.

We define a new flag in NativeLibrary called WHERE_CODEBASE that will let us have a third option for lookup of e_sqlite3 providers.

I'm limiting to just e_sqlite3 Because that is the library used by the App Center C# sdk.

@@ -370,6 +370,12 @@ LibSuffix suffix
a.Add(Path.Combine(dir, libname));
}

if ((flags & WHERE_CODEBASE) != 0)
{
var dir = Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().CodeBase);
Copy link

Choose a reason for hiding this comment

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

Assembly.CodeBase is obsolete and throws on .NET 5 and later version.

Copy link
Contributor Author

@farzonl farzonl Sep 4, 2023

Choose a reason for hiding this comment

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

Vsto apps are preserved in amber on the 4.0 runtime, but i get your point. I could limit this to .net framework with System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Few things. First this isn’t a build failure issue, so i think some proper guards should be sufficient to satisfy everyone. Second if you are a 5.0 or higher .net app you are likely going to resolve a path via WHERE_RUNTIME_RID, WHERE_ARCH, or WHERE_ADJACENT. Third, SQLitePCL.raw bundle today is already throwing two win32 exceptions when e_sqlite3 is not found. Checking for dot net framework or catching one more exception doesn’t seem unreasonable. Also until the dll shadow copy has happened i don't know the path and further IT policy limits me from copying dlls at runtime so I can’t use the recommended Assembly.Location see remarks:

.NET Framework only: If the loaded file was [shadow-copied](https://learn.microsoft.com/en-us/dotnet/framework/app-domains/shadow-copy-assemblies), the location is that of the file after being shadow-copied. To get the location before the file has been shadow-copied, use the [CodeBase](https://learn.microsoft.com/en-us/dotnet/api/system.reflection.assembly.codebase?view=net-7.0) property.

@ericsink
Copy link
Owner

ericsink commented Sep 6, 2023

We've had problems with VSTO for a very long time, and I've never known what was going wrong or how to fix it. Now we have answers, and that's great news.

But now we need to figure how to incorporate the fix without breaking anything else. I'll need to find some time soon to look at this a bit more closely.

@farzonl
Copy link
Contributor Author

farzonl commented Sep 6, 2023

We've had problems with VSTO for a very long time, and I've never known what was going wrong or how to fix it. Now we have answers, and that's great news.

But now we need to figure how to incorporate the fix without breaking anything else. I'll need to find some time soon to look at this a bit more closely.

No worries. I want to do this right. I was looking for ways to handle different versions of .net, and to my suprise I didn't find any usage of Environment.Version and RuntimeInformation.FrameworkDescription in this repo before mine. Thats pretty impressive given the shear number of platforms this projects suports between runtimes, oses, etc.

In anycase I put up the fix I mentioned to @tibel. Pleae let me know how you want to proceed.

@farzonl
Copy link
Contributor Author

farzonl commented Sep 21, 2023

@ericsink Is the use of RuntimeInformation.FrameworkDescription in revision 4 sufficent to satify the concerns here?

@ericsink
Copy link
Owner

Sorry, I haven't found time to look at this closely yet. But thanks for the nag. I'll try to get it done soon.

@farzonl
Copy link
Contributor Author

farzonl commented Oct 15, 2023

#552 (comment)

No worries Eric.

I wanted to show another way we could do this:

https://github.com/libgit2/libgit2sharp/blob/8c32b6160a49890e26100148e73c1558b0daa9af/LibGit2Sharp/GlobalSettings.cs#L43C1-L68C10

libgit2sharp is creating preprocessor blocks for .netframework. I tested it and it works in VSTO apps aswell.

@farzonl
Copy link
Contributor Author

farzonl commented Nov 6, 2023

@ericsink if it is possible to make a decision on this pr this month that would be ideal for me. I'm not sure what the next semester plan will be for the app I've built after November so can't promise to devote any time to this going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants