You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/06/24 19:21:44 UTC
[GitHub] [arrow] lidavidm opened a new pull request, #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
lidavidm opened a new pull request, #13434:
URL: https://github.com/apache/arrow/pull/13434
Also, enable Flight SQL in Windows CI builds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1167773762
MinGW works now; something about MSVC is not working. I can reproduce the issue, but not sure why it's happening yet. (The suggestions of adding `-DGMOCK_LINKED_AS_SHARED_LIBRARY` lead to multiple symbol definition issues.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1168807577
I made the `client_test`s run only on Unix to just sidestep the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1167609277
Protobuf does not interact very well with dllexport declarations (seemingly on purpose/the team considers it a bad idea: https://groups.google.com/g/protobuf/c/PDR1bqRazts) so hopefully this works to get Flight SQL on Windows (MinGW/MSVC).
The root of the issue here is that client_test.cc uses the Protobuf classes directly, which then means we have to deal with getting dllexport/dllimport annotated in all the right places in the generated Protobuf sources (and the compiler apparently does not do this fully correctly). The tests can't really be rewritten to not use the Protobuf classes directly, but given those tests rely exclusively on mocks, I wonder if they're even worth keeping. CC @jduo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1166274452
Ah, thanks Kou, you're right - I'll fix this up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou merged pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
kou merged PR #13434:
URL: https://github.com/apache/arrow/pull/13434
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1165865075
https://issues.apache.org/jira/browse/ARROW-16902
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1165865088
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jduo commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
jduo commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1167986129
> Protobuf does not interact very well with dllexport declarations (seemingly on purpose/the team considers it a bad idea: https://groups.google.com/g/protobuf/c/PDR1bqRazts) so hopefully this works to get Flight SQL on Windows (MinGW/MSVC).
>
> The root of the issue here is that client_test.cc uses the Protobuf classes directly, which then means we have to deal with getting dllexport/dllimport annotated in all the right places in the generated Protobuf sources (and the compiler apparently does not do this fully correctly). The tests can't really be rewritten to not use the Protobuf classes directly, but given those tests rely exclusively on mocks, I wonder if they're even worth keeping. CC @jduo
I'm OK with removing these tests to deal with this problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #13434: ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13434:
URL: https://github.com/apache/arrow/pull/13434#issuecomment-1177556865
Looks like this builds successfully on Windows now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org