search

chucknorris / roundhouse

RoundhousE is a Database Migration Utility for .NET using sql files and versioning based on source control
http://projectroundhouse.org
917 stars 249 forks source link

Added missing resource file required for MySql transactions #318

Closed mmalik76 closed 5 years ago

mmalik76 commented 6 years ago

Fixed issue #317 - MySql NullReferenceException when running with transaction

BiggerNoise commented 6 years ago

First and foremost, I really want to thank you for submitting pull requests for this and the other two issues. That really helps keep things moving along.

Second. It's easier as a maintainer when each P/R is submitted on a separate branch from the master (rather than an accumulating set of commits from other P/Rs). Don't sweat it for these three; it's easy enough to understand what's going on, so I can tease them apart really easily.

On this P/R. I'd really like to alias the resource so it comes out correctly. I have no idea what the legal issues involved with incorporating any of Oracle's source code into our project would be, but I expect they, should Oracle decide to care, are ugly.

@ferventcoder I expect this is surfacing from the ilmerge that we may not have quite gotten correct. Is there a setting that would allow us to surface this resource at a different named point?

I'm also willing to pull in the resource file as is if you're OK with that.

erikbra commented 5 years ago

@mmalik76 I'm trying to pick up this PR, sorry for the long wait. I don't exactly understand what @BiggerNoise is referring to when talking about Oracle's source code. Are any of the files included in the PR from Oracle's source code?

mmalik76 commented 5 years ago

@erikbra check out issue #317 if you haven't already it has more details on the issue, but yes the source code that @BiggerNoise is referring to is the keywords.txt file is a copy of a the resource file compiled in Oracle's MySql driver dll.

BiggerNoise commented 5 years ago

My concern was that Oracle (owners of MySQL) would do a douchey Oracle thing and assert a copyright claim on the resources.

On Wed, Jan 2, 2019 at 5:52 PM Matt Malik notifications@github.com wrote:

@erikbra https://github.com/erikbra check out issue #317 https://github.com/chucknorris/roundhouse/issues/317 if you haven't already it has more details on the issue, but yes the source code that @BiggerNoise https://github.com/BiggerNoise is referring to is the keywords.txt file is a copy of a the resource file compiled in Oracle's MySql driver dll.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/chucknorris/roundhouse/pull/318#issuecomment-451021252, or mute the thread https://github.com/notifications/unsubscribe-auth/AApSEqq60ZAK-IsQBoFC-A3NwqXv3yw9ks5u_UYjgaJpZM4TKIsq .

erikbra commented 5 years ago

Thanks a lot for your comments, guys. As I see it, there are two issues, the one is that Rh gets the wrong filename for the embedded resource. But, is the keywords.txt embedded in the MySql.Data DLL as distributet on NuGet? If so, I think refining how we construct the embedded resource name when in the merged assembly would maybe be a better solution.

Concerning the list of reserved words, it is already public information, available from e.g. https://dev.mysql.com/doc/refman/8.0/en/keywords.html#keywords-in-current-series (just the words with (R) behind them), so we could easily make our own, if necessary.

But, if we could read the embedded resource from the MySql dll itself, would that be preferred?

erikbra commented 5 years ago

Ah, sorry, I see this now, that it is MySql.Data that is reading the embedded file... That makes my suggestion difficult, yes... I don't think it would be a problem to check in the file, but maybe we could be smarter, and extract it from the MySql.Data.dll as part of the build process, before the merge. I haven't done anything like that before, but I guess it's not impossible..

erikbra commented 5 years ago

@mmalik76 this PR is now feature-wise merged into master. I solved the keywords.txt issue in a little different way, I extracted it from MySql.Data.dll before build (happens in build.ps1). That way, we'lll not have any copyright issues, at least (even though I don't think we would have anyway, but IANAL).

Could you please try to verify that master now solved the problem with keywords.txt? We have some problems with builds off the build server, as AppVeyor has moved to netcore2.2 (I will solve it soonish), so until this is fixed, I guess you have to build it yourself to try.

Again, thanks a lot for contributing, and I'm sorry that the PR has been left hanging for so long.