You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2020/05/19 09:14:38 UTC
[GitHub] [thrift] zeshuai007 opened a new pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
zeshuai007 opened a new pull request #2148:
URL: https://github.com/apache/thrift/pull/2148
<!-- Explain the changes in the pull request below: -->
This PR fix Visual Studio project build error.
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [x] Did you squash your changes to a single commit? (not required, but preferred)
- [x] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] emmenlau commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
emmenlau commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r429786357
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Dear @zeshuai007 I understand but its quite curious. The previous project did not have it, or did it? Was that an older standard?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] zeshuai007 commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
zeshuai007 commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r429897851
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Dear @emmenlau , The previous project did not have it, it was generated by visual studio when I confirm to chose the `v142 PlatformToolset`. Maybe the correct way is to remove it and use what version of `PlatformToolset` is depending on the developer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] zeshuai007 commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
zeshuai007 commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r430127911
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Dear @emmenlau , I am sorry for not clarify clearly. Visual Studio has the config menu for choose the `PlatformToolset`. Developer can config the correct `PlatformToolset` in they environment. Just like me, I config the `v142 PlatformToolset`, so it build correctly.But if i don't remove the config informoation(`<PlatformToolset>v142</PlatformToolset>`) ,it may not work for others who has the different `PlatformToolset`. So, I think remove it and config by developer again maybe a better way.Even if they occur some problem relate with this, Visual Studio also provide the output information that imply them to config it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stale[bot] closed pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #2148:
URL: https://github.com/apache/thrift/pull/2148
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] emmenlau commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
emmenlau commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r429105704
##########
File path: lib/cpp/src/thrift/transport/THttpClient.cpp
##########
@@ -102,7 +102,7 @@ void THttpClient::flush() {
std::ostringstream h;
h << "POST " << path_ << " HTTP/1.1" << CRLF << "Host: " << host_ << CRLF
<< "Content-Type: application/x-thrift" << CRLF << "Content-Length: " << len << CRLF
- << "Accept: application/x-thrift" << CRLF << "User-Agent: Thrift/" << PACKAGE_VERSION
+ << "Accept: application/x-thrift" << CRLF << "User-Agent: Thrift/" /*<< PACKAGE_VERSION*/
Review comment:
Why is `PACKAGE_VERSION` commented out?
##########
File path: lib/cpp/src/thrift/transport/TSSLSocket.h
##########
@@ -324,7 +324,7 @@ class TSSLSocketFactory {
std::shared_ptr<AccessManager> access_;
static concurrency::Mutex mutex_;
static uint64_t count_;
- THRIFT_EXPORT static bool manualOpenSSLInitialization_;
+ /*THRIFT_EXPORT*/ static bool manualOpenSSLInitialization_;
Review comment:
Why is `THRIFT_EXPORT` commented out?
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -211,7 +267,8 @@
</ImportGroup>
<PropertyGroup Label="UserMacros" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
- <IncludePath>$(ProjectDir)\src\;$(ProjectDir)\src\thrift\windows\;$(BOOST_ROOT)\include;$(BOOST_ROOT)\;$(OPENSSL_ROOT_DIR)\include\;$(IncludePath)</IncludePath>
+ <IncludePath>$(ProjectDir)\src\;$(ProjectDir)\src\thrift\windows\;C:\Users\Administrator\Desktop\HeHaiPing\boost_1_73_0;C:\ruanjianbao\openssl\include;$(IncludePath)</IncludePath>
+ <LibraryPath>$(BOOST_HOME)\stage\lib;C:\ruanjianbao\openssl\lib;$(LibraryPath)</LibraryPath>
Review comment:
This path `C:\ruanjianbao\openssl` is probably an artefact? :-)
##########
File path: lib/cpp/src/thrift/transport/THttpServer.cpp
##########
@@ -140,7 +140,7 @@ void THttpServer::flush() {
std::string THttpServer::getHeader(uint32_t len) {
std::ostringstream h;
h << "HTTP/1.1 200 OK" << CRLF << "Date: " << getTimeRFC1123() << CRLF << "Server: Thrift/"
- << PACKAGE_VERSION << CRLF << "Access-Control-Allow-Origin: *" << CRLF
+ /*<< PACKAGE_VERSION*/ << CRLF << "Access-Control-Allow-Origin: *" << CRLF
Review comment:
Why is `PACKAGE_VERSION` commented out?
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Is `PlatformToolset` required or can Visual Studio find it automatically? Could you try to remove it (maybe with a text editor), then load the project and see if it builds correctly?
The less constraint the project is, the better, because it can remain compatible with more versions of `PlatformToolset`.
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -211,7 +267,8 @@
</ImportGroup>
<PropertyGroup Label="UserMacros" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
- <IncludePath>$(ProjectDir)\src\;$(ProjectDir)\src\thrift\windows\;$(BOOST_ROOT)\include;$(BOOST_ROOT)\;$(OPENSSL_ROOT_DIR)\include\;$(IncludePath)</IncludePath>
Review comment:
Why is `BOOST_ROOT` and `OPENSSL_ROOT` replaced with `BOOST_HOME`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stale[bot] commented on pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#issuecomment-663801553
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] emmenlau commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
emmenlau commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r429913452
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Dear @zeshuai007 This was my suggestion to remove it an let Visual Studio automatically add it for the developer. But I understand you said this did not build correctly? Can you clarify if it can safely be removed or not?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] zeshuai007 commented on a change in pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
zeshuai007 commented on a change in pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#discussion_r429718115
##########
File path: lib/cpp/libthrift.vcxproj
##########
@@ -134,45 +182,53 @@
<ConfigurationType>StaticLibrary</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<CharacterSet>MultiByte</CharacterSet>
+ <PlatformToolset>v142</PlatformToolset>
Review comment:
Hi @emmenlau , I remove it and load the project but it can't builds correctly. The Visual Studio's working mechanism is: it will replace the `PlatformToolset` with your current environment's `PlatformToolset`. If your setting is not match with your current environment, it can't work.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stale[bot] commented on pull request #2148: THRIFT-5212: Thrift Visual Studio project throw errors when build
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2148:
URL: https://github.com/apache/thrift/pull/2148#issuecomment-667502804
This issue has been automatically closed due to inactivity. Thank you for your contributions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org