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 2022/10/01 21:50:08 UTC
[GitHub] [thrift] cfriedt opened a new pull request, #2689: lib: cpp: TTransportException: create thrift::numeric_cast
cfriedt opened a new pull request, #2689:
URL: https://github.com/apache/thrift/pull/2689
<!-- Explain the changes in the pull request below: -->
This adds an equivalent implementation of `boost::numeric_cast` written purely in standard c++.
The implementation is relatively trivial and reduces the dependency on `boost`.
Adapted from
https://stackoverflow.com/a/49658950/5636218
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
No
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
No
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
Yes
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
Yes
--
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: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323407160
> @cfriedt I guess this broke sth.
> https://app.travis-ci.com/github/apache/thrift/jobs/589120686
That's weird - I'll take a look. Was that an autotools build or CMake build?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323408624
CMake I'd say
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G merged pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
Jens-G merged PR #2689:
URL: https://github.com/apache/thrift/pull/2689
--
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: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323520474
> > We can fix it or revert it. Your decision.
> > BTW this also looks strange but don't know yet if it is related, may be caused by another patch.
> > https://app.travis-ci.com/github/apache/thrift/jobs/589120685
>
> I've seen this before and it's due to header reordering. I've been careful to avoid that FWIR.
>
> There is that one header that defines `THRIFT_IOCTL` etc macros and because it does not itself include unistd, ioctl, fcntl, etc, it's ordering is fragile.
Ah, this one is because `<thrift/config.h>` is not included
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323441592
> CMake I'd say
I see the issue. `thrift/numeric_cast.h` needs to be installed (of course). `make-dist.sh` tests builds from the distributed tarball.
Should be a quick fix.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1291953304
Currently seeing a bunch of MSVC errors like the one below:
```
C:\projects\thrift\lib\cpp\src\thrift/numeric_cast.h(47): warning C4003: not enough arguments for function-like macro invocation 'max' [C:\projects\build\MSVC2017\x64\lib\cpp\thrift.vcxproj]
C:\projects\thrift\lib\cpp\src\thrift/numeric_cast.h(50): warning C4003: not enough arguments for function-like macro invocation 'max' [C:\projects\build\MSVC2017\x64\lib\cpp\thrift.vcxproj]
ThreadManager.cpp
TimerManager.cpp
PeekProcessor.cpp
```
Will take a look shortly.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1292794195
A number of travis build failures. Not due to this PR, but I know what this fix is and will submit in a separate PR.
```
src/thrift/transport/TFileTransport.cpp -fPIC -DPIC -o src/thrift/transport/.libs/TFileTransport.o
In file included from ./src/thrift/Thrift.h:23:0,
from ./src/thrift/transport/TTransport.h:23,
from ./src/thrift/transport/TFileTransport.h:23,
from src/thrift/transport/TFileTransport.cpp:22:
./src/thrift/concurrency/FunctionRunner.h: In member function 'virtual void apache::thrift::concurrency::FunctionRunner::run()':
./src/thrift/transport/PlatformSocket.h:118:29: error: 'usleep' was not declared in this scope
# define THRIFT_SLEEP_USEC usleep
^
./src/thrift/concurrency/FunctionRunner.h:104:9: note: in expansion of macro 'THRIFT_SLEEP_USEC'
THRIFT_SLEEP_USEC(intervalMs_ * 1000);
^~~~~~~~~~~~~~~~~
./src/thrift/transport/PlatformSocket.h:118:29: note: suggested alternative: 'fseek'
# define THRIFT_SLEEP_USEC usleep
^
./src/thrift/concurrency/FunctionRunner.h:104:9: note: in expansion of macro 'THRIFT_SLEEP_USEC'
THRIFT_SLEEP_USEC(intervalMs_ * 1000);
^~~~~~~~~~~~~~~~~
```
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323415540
> We can fix it or revert it. Your decision.
>
> BTW this also looks strange but don't know yet if it is related, may be caused by another patch.
>
> https://app.travis-ci.com/github/apache/thrift/jobs/589120685
I've seen this before and it's due to header reordering. I've been careful to avoid that FWIR.
There is that one header that defines `THRIFT_IOCTL` etc macros and because it does not itself include unistd, ioctl, fcntl, etc, it's ordering is fragile.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323401889
@cfriedt I guess this broke sth.
https://app.travis-ci.com/github/apache/thrift/jobs/589120686
[4/354] Building CXX object lib/cpp/CM...ansport/TNonblockingServerSocket.cpp.o
FAILED: lib/cpp/CMakeFiles/thriftnb.dir/src/thrift/transport/TNonblockingServerSocket.cpp.o
/usr/bin/g++ -DBOOST_ALL_DYN_LINK -DBOOST_TEST_DYN_LINK -DTHRIFT_STATIC_DEFINE -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/cpp -I../lib/cpp -I. -I../lib/cpp/src -O2 -g -DNDEBUG -std=c++11 -MD -MT lib/cpp/CMakeFiles/thriftnb.dir/src/thrift/transport/TNonblockingServerSocket.cpp.o -MF lib/cpp/CMakeFiles/thriftnb.dir/src/thrift/transport/TNonblockingServerSocket.cpp.o.d -o lib/cpp/CMakeFiles/thriftnb.dir/src/thrift/transport/TNonblockingServerSocket.cpp.o -c ../lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp
In file included from ../lib/cpp/src/thrift/transport/TTransport.h:25:0,
from ../lib/cpp/src/thrift/transport/TSocket.h:25,
from ../lib/cpp/src/thrift/transport/TNonblockingServerTransport.h:23,
from ../lib/cpp/src/thrift/transport/TNonblockingServerSocket.h:23,
from ../lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp:54:
../lib/cpp/src/thrift/transport/TTransportException.h:25:10: fatal error: thrift/numeric_cast.h: No such file or directory
#include <thrift/numeric_cast.h>
^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
ninja: build stopped: subcommand failed.
The command "build/docker/run.sh" exited with 1.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323403094
We can fix it or revert it. Your decision.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] cfriedt commented on pull request #2689: THRIFT-5660: lib: cpp: TTransportException: create thrift::numeric_cast
Posted by GitBox <gi...@apache.org>.
cfriedt commented on PR #2689:
URL: https://github.com/apache/thrift/pull/2689#issuecomment-1323524387
@Jens-G fixes are in https://github.com/apache/thrift/pull/2733
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org