You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by jeking3 <gi...@git.apache.org> on 2016/09/28 15:13:10 UTC
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/1099
THRIFT-3936: fix compile error on VS2013 and earlier
Submitting to replace https://github.com/apache/thrift/pull/1096.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jeking3/thrift defect/THRIFT-3936
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1099.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1099
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r81119054
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
FInally... the compiler has a windows/config.h that's different than the library one...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r80976940
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
As I said, for windows it's OK, for non-windows without stdint.h, it will fail.
We could improve config.hin and build/cmake/config.h.in to do the same thing as Windows when boost is avaiable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r80977866
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
I see this work was done in windows/config.h so we just need to have TApplicationException.h include the correct header to pull that in on Windows.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1099: THRIFT-3936: fix compile error on VS2013 and earlier
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1099
@CompilerMaster this contains the fix I described.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r80976274
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
Okay then what happens with the "uint32_t" lines at the bottom of this header if someone does not have STDINT_H?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r80975121
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
We have `HAVE_STDINT_H` for this.
For older VC++, uint32_t is still available via windows/config.h, switching between boost and std, which is included via Thrift.h.
It's problematic for any non-windows without stdint.h, though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r81062337
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
Still perplexing me... it looks like _STDINT is already defined by the time we get there but only stdint.h seems to define that... so I'm still looking...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r80994513
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
This is a little more complicated than I anticipated. It looks like when compiling plugin_types.cpp we end up including TApplicationException through some code path that either undefines HAVE_STDINT_H (which is defined in windows/config.h, from thrift/Thrift.h) or something else is happening. Still working on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r81000970
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
Oh... it sounds pretty much caused by THRIFT-2835... I'll look into it within a few days or so too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1099#discussion_r81125635
--- Diff: lib/cpp/src/thrift/TApplicationException.h ---
@@ -21,6 +21,7 @@
#define _THRIFT_TAPPLICATIONEXCEPTION_H_ 1
#include <thrift/Thrift.h>
+#include <stdint.h>
--- End diff --
Figured it out; wasn't able to build with VS2010 due to changes made in THRIFT-2835. I made the necessary changes and ensured I could build on windows and on linux (to a point... I am still running into THRIFT-3937 on my linux system).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1099: THRIFT-3936: fix compile error on VS2013 and earl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1099
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---