Skip to content

Conversation

@lauxjpn
Copy link
Member

@lauxjpn lauxjpn commented Mar 9, 2020

This is a work-in-progress PR to upgrade the provider code (without the tests as of now) to EF Core 3.1.x. That means, that this PR can still change/be rebased/force-pushed without warning.

I removed most SQL Server-only code that had no corresponding implementation in Jet. Some smaller parts might still exist and should be cleaned up over the next iterations.

I also kept the codebase relatively similar to its EF Core 2.2.x origin, though a couple of method call translations should be corrected. They should come up again in a couple of test cases.

I also added some TODO: and CHECK: flags in the codebase, that should be revisited before going GA.

This PR is tracked by #34.

@lauxjpn lauxjpn added this to the 3.1.x milestone Mar 9, 2020
@lauxjpn lauxjpn self-assigned this Mar 9, 2020
@lauxjpn lauxjpn linked an issue Mar 9, 2020 that may be closed by this pull request
@FreddyDgh
Copy link
Contributor

@lauxjpn Looks like you might want to consider changing the following line in NuGet.config

<add key="dotnet-core (nightly)" value="https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json" />

to be

<add key="dotnet-core (nightly)" value="https://dnceng.pkgs.visualstudio.com/public/_packaging/dotnet5/nuget/v3/index.json" />

I'm not sure what the difference is, but someone at Microsoft just linked the second one in one of the issues, so I'm guessing that the first one is an older (but still working) version of the nightly feed. The dnceng.pkgs.visualstudio.com feed seemed to work much better/more reliably for me when using the Visual Studio package manager UI, which was a bit buggy for me with the other one.

@lauxjpn
Copy link
Member Author

lauxjpn commented Mar 10, 2020

@FreddyD-GH Thanks! I was wondering today, why your dotnet/runtime#32573 fix wasn't yet part of the nightly-builds and implemented a dummy transaction class to ignore the error for the time being.
But with the feed you proposed, the fix is part of the latest nightly preview and I can confirm that transactions do work again.

BTW, I will push the test projects tomorrow. They should be compiling. I fixed a couple of tests and provider code issues that came up because of them.

@bubibubi
Copy link
Member

I did not read about the OleDb issue. If the transaction get closed (commit/rollback) also with the issue (and so we can just ignore the error), the whole OleDb provider is wrapped by System.Data.Jet (the only thing that is not wrapped is the Connection Builder). If you think that is better to use the "stable" version of OleDb we can work in JetTransaction.Commit.

@lauxjpn lauxjpn changed the title [WIP] Upgrade provider code to 3.1.x Upgrade provider and test code to 3.1.x Mar 23, 2020
@lauxjpn
Copy link
Member Author

lauxjpn commented Mar 23, 2020

From my point-of-view, the PR is good enough to be merged. I will leave it open for about 48 hours for anybody to review the code and will merge after that, if there are not objections.

@lauxjpn lauxjpn force-pushed the upgrade branch 2 times, most recently from ac58b4a to bd708a3 Compare March 23, 2020 19:57
lauxjpn added 2 commits March 25, 2020 05:25
Use `ReleaseComObject()` instead of `FinalReleaseComObject` to avoid creating stale references to the RCW of a potential singleton COM object.
@lauxjpn lauxjpn removed a link to an issue Mar 27, 2020
@lauxjpn lauxjpn merged commit 253ca16 into CirrusRedOrg:3.1-preview Mar 27, 2020
@lauxjpn lauxjpn deleted the upgrade branch March 27, 2020 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants