openssl 1.0.2j, freetds 1.00.21, and openssl shared lib generation #322
Conversation
| file "ports/#{host}/bin/#{dll}" do |t| | ||
| sh 'x86_64-w64-mingw32-strip', t.name | ||
| end | ||
| end |
There was a problem hiding this comment.
I removed this stuff so I could use the symbols while debugging and it seemed like some symbols which looked useful for relocation may also have been removed... but I couldn't confirm so I left it removed...
| @@ -157,7 +157,7 @@ def configure | |||
| end | |||
| args = [ "CFLAGS=-DDSO_WIN32", | |||
| "./Configure", | |||
| "no-shared", | |||
| "shared", | |||
There was a problem hiding this comment.
I'm really not sure about the implications of this change. The resulting binaries have different exports, different sizes, and different symbols, and use a different build process than the old dllwrap. Given the comment below I'm not sure how safe this is. Any ideas?
| @@ -39,5 +39,8 @@ environment: | |||
| secure: fYKSKV4v+36OFQp2nZdX4DfUpgmy5cm0wuR73cgdmEk= | |||
| matrix: | |||
| - ruby_version: "23-x64" | |||
| - ruby_version: "23" | |||
| - ruby_version: "22-x64" | |||
| - ruby_version: "22" | |||
There was a problem hiding this comment.
I added these back because for a while I was getting failures in one or two, but not all.
|
This is amazing work. I honestly need to rely on you to make the calls on these because this area of TinyTDS is most unfamiliar to me. Is there anything else you want to do before I merge this? Are there any next steps after this? I am hitting Rails v5 in the adapter hard, could you help with moving the adapter to the new miniportile docker work so we can build and push new Windows gems? |
|
@coderjoe I added you as a collaborator. Feel free to merge when ready. Super appreciate the help! |
|
Thank you very much for your incredible responsiveness in helping me resolve this issue especially around the holidays. I'm glad it has been helpful. I've got some changes I want to make before committing, so I'm going to close this PR and continue to work my forked branch so I can make changes without fear of them accidentally being merged. I'll submit a new PR once it's cleaned up, re-tested, and ready to go. :) I don't have much availability tomorrow to work on opensource stuff, but I hope to be able to put get everything together ASAP probably this weekend at the latest. |
|
Either works fine, you could have kept this open and made commits to see the builds run. Leaving it open would also mean others could find it more easily and comment. Don't sweat the churn to this branch either. Once you are ready, we can do a squash merge to keep it clean. Either way, whatever works best for you. |
|
Oh that's a really good point. I'll re-open for visibility's sake. Thanks for the advice. 😄 |
|
@metaskills do you know of any way to convince appveyor to rebuild a commit without re-committing? It failed on my most recent commit but it looks like it might be a network error and nothing more. |
Sadly not without logging into Appveyor and clicking re-build. But logging in is limited to the #rails-sqlserver account where I set that up. So no-op commits are best. The state of this PR looks fine. Can I merge this work in? |
|
@metaskills presuming this last test run passes and you can't duplicate any failures on Azure (I haven't had time to test it yet myself) then I'm good to merge. :) |
|
Hit the button whenever you are ready :) I recommend a squash merge with a wrapped up commit message. I'm starting work in the adapter now. Trying to fix the forgetting assignment bug. |
- Fix openssl being out of date. (Bumped to 1.0.2j) - Fix freetds being out of date. (Bumped to 1.00.21) - Fix the invalid w32 application error which occurs on x64 platforms occasionally due to problems with DLLs generated via dllwrap after being relocated. Relates to: rails-sqlserver#290 and rails-sqlserver#310
So with respect to issue #310 I was running into 3 distinct problems
998: invalid memory accessin windowsAll three of these problems seemed to boil down to the
dllwrapdll creation in the native build process. In my searches I found numerous references todllwrapoccasionally causing problems when dll relocation occurs.To test I removed the custom DLL generation, removed the symbol stripping (for debugging purposes) and rebuilt. Upon rebuild all tests pass, and when used on an instance experiencing
invalid win32 applicationit proceeds as normal.Now I'm not entirely sure why tinytds is building its own openssl shared libraries but letting openssl generated the libraries seems to have solved my problems. Thoughts?