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

Simplify sqlite authoring now that we're 64bit only #57836

Merged
merged 3 commits into from
Nov 20, 2021

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Nov 18, 2021

Fixes #57827

This also addresses the issue where on CoreCLR calling into sqlite woudl result in a dll-not-found exception happening. Effectively, we just place e_sqlite.dll next to all the SQLitePCLRaw.xxx dlls we include. Being right next to them seems to always work for dynamic loading for our two supported scenarios.

I'm hoping this simple approach is acceptable.

I have manually validated this both for normal OOP and CoreCLR oop.

Note: i'm not sure if this impacts VS4Mac.

@CyrusNajmabadi
Copy link
Member Author

Also tagging @Therzok . Need to make sure this won't be an issue for VS4Mac.

@genlu
Copy link
Member

genlu commented Nov 18, 2021

i'm not sure if this impacts VS4Mac.

This change only impact our vsix, so VS4Mac should be fine (or rather, won't be broken because of this)

@jinujoseph
Copy link
Contributor

cc @davidwengier for the VSMac awareness

Copy link
Contributor

@Therzok Therzok left a comment

Choose a reason for hiding this comment

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

Checked sqlitepcl.raw and it seems like probing is allowed for adjacent or RID. So we're safe.

LE: Just checked, we are bundling the native library ourselves, so we won't be impacted here, since Roslyn isn't responsible for loading the assemblies.

@CyrusNajmabadi
Copy link
Member Author

Thanks very much @Therzok . @davidwengier there should be no concerns here for you. Thanks all!

@davidwengier
Copy link
Contributor

if @Therzok is happy, i'm happy.

<Content Include="$(NuGetPackageRoot)\sqlitepclraw.lib.e_sqlite3\$(SQLitePCLRawbundle_greenVersion)\runtimes\win-x64\native\e_sqlite3.dll">
<Link>runtimes\win-x64\native\e_sqlite3.dll</Link>
<Link>Core\e_sqlite3.dll</Link>
Copy link
Member

Choose a reason for hiding this comment

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

❔ Can we not place it in Core\runtimes\win-x64\native\e_sqlite3.dll?

@CyrusNajmabadi CyrusNajmabadi merged commit fa2e3ca into dotnet:main Nov 20, 2021
@CyrusNajmabadi CyrusNajmabadi deleted the sqliteAuthoring branch November 20, 2021 22:58
@ghost ghost added this to the Next milestone Nov 20, 2021
@allisonchou allisonchou modified the milestones: Next, 17.1.P2 Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ServiceHub CoreCLR is failing to load sqlite dlls.
8 participants