You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by ozymaxx <gi...@git.apache.org> on 2018/02/19 15:08:20 UTC

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

GitHub user ozymaxx opened a pull request:

    https://github.com/apache/thrift/pull/1496

    THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python)

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ozymaxx/thrift THRIFT-4476

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1496.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 #1496
    
----
commit f5a6337d4448a30c284a3033307865dfc992ac7d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-19T13:55:09Z

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++

----


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    The current master is fairly clean except for a recurring dlang issue.  Squash and rebase on master.  Squash to one commit please.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx closed the pull request at:

    https://github.com/apache/thrift/pull/1496


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    The build on the `CYGWIN x86` (see https://ci.appveyor.com/project/ozymaxx/thrift/build/job/09xqx9b54efog55o for more details) machine has broken. Here is the error that I got:
    ```
    3504CMake Error at lib/java/cmake_install.cmake:39 (file):
    3505  file INSTALL cannot find
    3506  "/cygdrive/c/projects/build/CYGWIN/x86/lib/java/build/docs/javadoc".
    3507Call Stack (most recent call first):
    3508  cmake_install.cmake:36 (include)
    ```
    
    Some of the Travis builds have timed out (more details at https://travis-ci.org/ozymaxx/thrift). A build has also failed with the following error:
    ```
    Failed examples:
    
    rspec ./spec/server_spec.rb:103 # Server Thrift::ThreadPoolServer should serve inside a thread
    rake aborted!
    /usr/bin/ruby2.3 -S rspec ./spec/base_protocol_spec.rb ./spec/base_transport_spec.rb ./spec/binary_protocol_accelerated_spec.rb ./spec/binary_protocol_spec.rb ./spec/bytes_spec.rb ./spec/client_spec.rb ./spec/compact_protocol_spec.rb ./spec/exception_spec.rb ./spec/flat_spec.rb ./spec/http_client_spec.rb ./spec/json_protocol_spec.rb ./spec/namespaced_spec.rb ./spec/nonblocking_server_spec.rb ./spec/processor_spec.rb ./spec/serializer_spec.rb ./spec/server_socket_spec.rb ./spec/server_spec.rb ./spec/socket_spec.rb ./spec/ssl_socket_spec.rb ./spec/struct_nested_containers_spec.rb ./spec/struct_spec.rb ./spec/thin_http_server_spec.rb ./spec/types_spec.rb ./spec/union_spec.rb ./spec/unix_socket_spec.rb --color --format d failed
    /var/lib/gems/2.3.0/gems/rspec-core-2.13.1/lib/rspec/core/rake_task.rb:156:in `run_task'
    /var/lib/gems/2.3.0/gems/rspec-core-2.13.1/lib/rspec/core/rake_task.rb:124:in `block (2 levels) in initialize'
    /var/lib/gems/2.3.0/gems/rspec-core-2.13.1/lib/rspec/core/rake_task.rb:122:in `block in initialize'
    Tasks: TOP => default => gem => spec => realspec
    (See full trace by running task with --trace)
    Makefile:655: recipe for target 'check-local' failed
    make[3]: *** [check-local] Error 1
    make[3]: Leaving directory '/thrift/src/lib/rb'
    Makefile:515: recipe for target 'check-am' failed
    make[2]: *** [check-am] Error 2
    make[2]: Leaving directory '/thrift/src/lib/rb'
    Makefile:580: recipe for target 'check-recursive' failed
    make[1]: *** [check-recursive] Error 1
    make[1]: Leaving directory '/thrift/src/lib'
    Makefile:662: recipe for target 'check-recursive' failed
    make: *** [check-recursive] Error 1
    ```
    
    @jeking3 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    The AppVeyor builds stay in the queue for a long time. Is there any possibility to run the builds more quickly?


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174495089
  
    --- Diff: test/DoubleConstantsTest.thrift ---
    @@ -0,0 +1,17 @@
    +namespace java thrift.test
    +namespace cpp thrift.test
    +
    +// more tests on double constants (precision and type checks)
    --- End diff --
    
    Further evidence that disabling the test in Java is a bad idea: why don't you need to disable Javascript if the compiler is the cause?


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Two errors at the initial steps of my builds:
    * The one in Travis (happening in multiple builds):
    ```
    Resolving master.dl.sourceforge.net (master.dl.sourceforge.net)... 216.105.38.12
    Connecting to master.dl.sourceforge.net (master.dl.sourceforge.net)|216.105.38.12|:80... failed: Connection refused.
    The command '/bin/sh -c apt-key adv --keyserver keyserver.ubuntu.com --recv-keys EBCF975E5BA24D5E &&         wget http://master.dl.sourceforge.net/project/d-apt/files/d-apt.list -O /etc/apt/sources.list.d/d-apt.list &&         wget -qO - https://dlang.org/d-keyring.gpg | apt-key add -' returned a non-zero code: 4
    The command "if [[ `uname` == "Linux" ]]; then build/docker/refresh.sh; fi" failed and exited with 4 during .
    ```
    
    * The one in AppVeyor (at the initial steps):
    ```
    Downloading winflexbison3 
      from 'https://sourceforge.net/projects/winflexbison/files/old_versions/win_flex_bison-2.5.10.zip'
    WARNING: C:\projects\thrift\buildcache\winflexbison3\2.5.10.20170725\win_flex_bison-2.5.10.zip is of content type text/html
    Progress: 100% - Completed download of C:\projects\thrift\buildcache\winflexbison3\2.5.10.20170725\win_flex_bison-2.5.10.zip (652 B).
    Download of win_flex_bison-2.5.10.zip (652 B) completed.
    Error - hashes do not match. Actual value was '7A07D3F7CCA5C0B38CA811984EF8DA536DA32932D68C1A6CCE33EC2462B930BF'.
    ERROR: Checksum for 'C:\projects\thrift\buildcache\winflexbison3\2.5.10.20170725\win_flex_bison-2.5.10.zip' did not meet '565967D6F6D021B617144698DEDD055DF9B91825CB61A7E1F29BFD22219F550C' for checksum type 'sha256'. Consider passing the actual checksums through with --checksum --checksum64 once you validate the checksums are appropriate. A less secure option is to pass --ignore-checksums if necessary.
    The install of winflexbison3 was NOT successful.
    Error while running 'C:\ProgramData\chocolatey\lib\winflexbison3\tools\chocolateyInstall.ps1'.
     See log for details.
    Chocolatey installed 0/1 packages. 1 packages failed.
     See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
    Failures
     - winflexbison3 (exited -1) - Error while running 'C:\ProgramData\chocolatey\lib\winflexbison3\tools\chocolateyInstall.ps1'.
     See log for details.
    Command exited with code -1
    ```
    
    I think these errors are build configration-related. These also showed up in the builders running on my forked repo.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    You can fork the project and set up your own Travis CI and Appveyor builds, and submit pull requests into your fork to kick off builds, and you will not contend with the rest of the apache arsenal of projects.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
GitHub user ozymaxx reopened a pull request:

    https://github.com/apache/thrift/pull/1496

    THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python)

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++
    
    see the related issue: https://issues.apache.org/jira/browse/THRIFT-4476

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ozymaxx/thrift THRIFT-4476

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1496.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 #1496
    
----
commit f5a6337d4448a30c284a3033307865dfc992ac7d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-19T13:55:09Z

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++

commit 6dbc35f74fe53a9bacf06867036c21f2761c823e
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-22T09:47:27Z

    - a comment added (about setprecision)
    - BOOST_TEST -> BOOST_CHECK

commit 55e3371ed9ea7eadc5826a7f17d3e432e9b99858
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:10:15Z

    double rendering in erlang, fixed (some additional tests added)

commit 36cc6f9dfe5b508a531ad70958ca3209c3b65e84
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:44:31Z

    less than operator - correction in erlang test

commit a22d72f024bbf463895022792a31d04054a79445
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:17:16Z

    style changes, some fixes in erlang test

commit a615812729096a7fa106f8259eb2f41d61ac99ce
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:51:24Z

    some fixes in erlang test

commit 50b9a3fc69b82a9af2e78602a2e60b9a649874ee
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T07:22:41Z

    some fixes in erlang test - no need for including lists library

commit b4e9ce56aecf75beffbf77864f02891808072204
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T09:31:46Z

    more style changes - something overlooked

commit 39bb546d0ccbec7177866a1a78d9b5f4de0d9ec1
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T10:24:45Z

    expected 2 lines before main - fixed (not seen locally)

commit 98db81883af06d10e9277fb32988edfd3f044d1d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T11:24:05Z

    noticed that test cases should be added manually in Python

commit 64aae330efac4ee23ed01694bc7dceb5a7a9164b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-27T06:48:48Z

    DoubleConstantTest thrift compilation directive is now given in generate.cmake

commit decb436f40990e0a04be9730accd3becab951c39
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-28T06:13:34Z

    render_double_to_string -> emit_double_as_string

commit e9b2e389b1184efa4d8ad1d6424ec52a108b80c9
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-02T12:45:54Z

    added assertion error messages to see the problems inside Java tests in more detail

commit 248e8a4c208e518da30f761a2d836ff790cd9b0d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T06:59:59Z

    RDP access to AppVeyor build workers

commit dd9b65113f143f13e6524917add6c216b16f86e5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:01:07Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

commit 65a6588d049dcbbef7d3df5d7c3ba9a7b141917a
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:03:20Z

    duplicate key problem in appveyor settings

commit 6305384d9203bd532983d18a992515db802b29a5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T09:31:22Z

    ignore the test cases with (-+)1.7e+308 in Java tests

commit 74667acf55c435c661c5fc19de729eb5f82758dc
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T10:00:31Z

    ignore the test cases with (-+)1.7e+308 in Java tests - 2

commit 85b872bac39078446aa27336ca57001022524fc0
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T12:18:14Z

    pyflakes8 style changes

commit af11ef8c7475c5f57f2d6619bedea23856a0fd17
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:21:51Z

    disabling the test named 'TestRenderedDoubleConstants' on machines with the MSVC2010 configuration

commit 8ed1997141dce6c4b4505b09d2d6843344dfa04b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:58:25Z

    msvc.profile parameter existence check

commit beb12744f990ed0890648b07d81f19c121303c59
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:05:04Z

    passing "msvc profile" on build stage

commit 1a0756aa8d48b693ae39dd860bdbeb7de5252cae
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:50:10Z

    "MSVC_PROFILE" as a cache variable

commit b40fc4e06083f2787db3f805756abe7e6c2c704d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:01:48Z

    wrong usage of cache variables

commit ef43030fd69f1dee060c3cf447acfcceefb4be71
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:19:42Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit ec32218d28d6054f6fa5148c0d5d24236660afcf
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:23:39Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit bd1207f29b0662022e46371d1b1cc7be37013151
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:03:27Z

    let's see whether TestRenderedDoubleConstants is excluded or not

commit 6c3e243c2afc09d726113ae578b5236e673640ad
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:49:41Z

    msvc.profile access - fixed

commit 201fcb541ae3a5774b509efc5f3d036c85e2495f
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:22:34Z

    msvc.profile access - fixed

commit 788118364ae60781f56779f89b33e90aee116750
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:27:45Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

----


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r175284886
  
    --- Diff: compiler/cpp/src/thrift/generate/t_generator.h ---
    @@ -268,6 +271,30 @@ class t_generator {
         return out.str();
       }
     
    +  const std::string emit_double_as_string(const double value) {
    +      std::stringstream double_output_stream;
    +      // sets the maximum precision: http://en.cppreference.com/w/cpp/io/manip/setprecision
    +      // sets the output format to fixed: http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
    +      double_output_stream << std::setprecision(std::numeric_limits<double>::digits10 + 1);
    +
    +      #ifdef _MSC_VER
    +          // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
    +          // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
    +          //               c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
    +          //               and
    +          //               http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
    +          #if _MSC_VER >= MSC_2015_VER
    +              double_output_stream << std::fixed;
    +          #endif
    +      #else
    +          double_output_stream << std::fixed;
    --- End diff --
    
    No, in both cases it should use `std::fixed`, that outputs the fixed floating point representation of the number. But if the MSVC compiler is older than 2015, that function is broken, and should not be called. In short, we want the compiler to generate the doubles in the fixed floating point representation if possible. On the Erlang side, if the compiler is older than 2015, we want the compiler to use `std::scientific` since there can be cases where the mantissa is being output as integer, which is illegal. `std::scientific` always outputs the floating points numbers with a floating point mantissa. But if it is 2015 or later, `std::fixed` works correctly and can be used to get rid of the exponent representation. 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Btw, what's the full name of the test which you want to succeed? I squashed to another commit and will take a look if this test has passed or not (I hope there is not something wrong related to my change set :))


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    You may be missing my point here.  Regardless of the compiler used to build the thrift compiler, whether it is gcc-4.6, clang-6.0, MSVC 2010, MSVC 2013, MSVC 2017, the floating constants emitted from the thrift compiler should be the same.  If they are not, I recommend fixing the thrift compiler and NOT disabling the new tests which caught an important issue.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174686538
  
    --- Diff: lib/java/gradle/unitTests.gradle ---
    @@ -68,6 +68,20 @@ test {
         include '**/Test*.class'
         exclude '**/Test*\$*.class'
     
    +    println 'Check if msvc.profile exists'
    --- End diff --
    
    I did not want to disable the entire `JavaTest` just for a single test suite.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
GitHub user ozymaxx reopened a pull request:

    https://github.com/apache/thrift/pull/1496

    THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python)

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++
    
    see the related issue: https://issues.apache.org/jira/browse/THRIFT-4476

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ozymaxx/thrift THRIFT-4476

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1496.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 #1496
    
----
commit f5a6337d4448a30c284a3033307865dfc992ac7d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-19T13:55:09Z

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++

commit 6dbc35f74fe53a9bacf06867036c21f2761c823e
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-22T09:47:27Z

    - a comment added (about setprecision)
    - BOOST_TEST -> BOOST_CHECK

commit 55e3371ed9ea7eadc5826a7f17d3e432e9b99858
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:10:15Z

    double rendering in erlang, fixed (some additional tests added)

commit 36cc6f9dfe5b508a531ad70958ca3209c3b65e84
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:44:31Z

    less than operator - correction in erlang test

commit a22d72f024bbf463895022792a31d04054a79445
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:17:16Z

    style changes, some fixes in erlang test

commit a615812729096a7fa106f8259eb2f41d61ac99ce
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:51:24Z

    some fixes in erlang test

commit 50b9a3fc69b82a9af2e78602a2e60b9a649874ee
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T07:22:41Z

    some fixes in erlang test - no need for including lists library

commit b4e9ce56aecf75beffbf77864f02891808072204
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T09:31:46Z

    more style changes - something overlooked

commit 39bb546d0ccbec7177866a1a78d9b5f4de0d9ec1
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T10:24:45Z

    expected 2 lines before main - fixed (not seen locally)

commit 98db81883af06d10e9277fb32988edfd3f044d1d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T11:24:05Z

    noticed that test cases should be added manually in Python

commit 64aae330efac4ee23ed01694bc7dceb5a7a9164b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-27T06:48:48Z

    DoubleConstantTest thrift compilation directive is now given in generate.cmake

commit decb436f40990e0a04be9730accd3becab951c39
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-28T06:13:34Z

    render_double_to_string -> emit_double_as_string

commit e9b2e389b1184efa4d8ad1d6424ec52a108b80c9
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-02T12:45:54Z

    added assertion error messages to see the problems inside Java tests in more detail

commit 248e8a4c208e518da30f761a2d836ff790cd9b0d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T06:59:59Z

    RDP access to AppVeyor build workers

commit dd9b65113f143f13e6524917add6c216b16f86e5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:01:07Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

commit 65a6588d049dcbbef7d3df5d7c3ba9a7b141917a
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:03:20Z

    duplicate key problem in appveyor settings

commit 6305384d9203bd532983d18a992515db802b29a5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T09:31:22Z

    ignore the test cases with (-+)1.7e+308 in Java tests

commit 74667acf55c435c661c5fc19de729eb5f82758dc
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T10:00:31Z

    ignore the test cases with (-+)1.7e+308 in Java tests - 2

commit 85b872bac39078446aa27336ca57001022524fc0
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T12:18:14Z

    pyflakes8 style changes

commit af11ef8c7475c5f57f2d6619bedea23856a0fd17
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:21:51Z

    disabling the test named 'TestRenderedDoubleConstants' on machines with the MSVC2010 configuration

commit 8ed1997141dce6c4b4505b09d2d6843344dfa04b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:58:25Z

    msvc.profile parameter existence check

commit beb12744f990ed0890648b07d81f19c121303c59
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:05:04Z

    passing "msvc profile" on build stage

commit 1a0756aa8d48b693ae39dd860bdbeb7de5252cae
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:50:10Z

    "MSVC_PROFILE" as a cache variable

commit b40fc4e06083f2787db3f805756abe7e6c2c704d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:01:48Z

    wrong usage of cache variables

commit ef43030fd69f1dee060c3cf447acfcceefb4be71
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:19:42Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit ec32218d28d6054f6fa5148c0d5d24236660afcf
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:23:39Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit bd1207f29b0662022e46371d1b1cc7be37013151
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:03:27Z

    let's see whether TestRenderedDoubleConstants is excluded or not

commit 6c3e243c2afc09d726113ae578b5236e673640ad
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:49:41Z

    msvc.profile access - fixed

commit 201fcb541ae3a5774b509efc5f3d036c85e2495f
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:22:34Z

    msvc.profile access - fixed

commit 788118364ae60781f56779f89b33e90aee116750
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:27:45Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

----


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174687128
  
    --- Diff: test/py/RunClientServer.py ---
    @@ -298,7 +300,11 @@ def main():
         print('----------------')
         for genpydir in generated_dirs:
             for script in SCRIPTS:
    -            runScriptTest(options.libdir, options.gen_base, genpydir, script)
    +            # The MSVC 2013 C++ compiler stores the double literals differently from
    +            # the MSVC 2015 and G++ compilers, so the TestRenderedDoubleConstants test
    +            # fails on the VMs with the MSVC2013 configuration
    +            if not (script == 'TestRenderedDoubleConstants.py' and options.msvc_profile == 'MSVC2013'):
    +                runScriptTest(options.libdir, options.gen_base, genpydir, script)
    --- End diff --
    
    and here @jeking3 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Okay, I will add a condition on the tests. The values to do the assertions with will depend on the compiler. 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    I put the MSVC2013 job back in to make sure we had some backwards compatibility... glad I did.   I wonder if we have 32-bit issues with double in general in thrift?


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    @jeking3 , I will now re-enable the tests. But please note that the problem is at the C++ compiler level. The pre-2015 versions of the Visual Studio C++ compilers uses a different parsing and formatting algorithms on floating point numbers. Check this link out: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/ (go to the title "Floating Point Formatting and Parsing Correctness")
    
    Another article describing the issue: http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
    The comment made on the article, by a Visual Studio dev. team member: http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/#comment-9403
    
    In the light of these, the best solution seems checking the compiler version, and doing the tests accordingly. 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    If there's a way to fix it in the compiler so that it works properly, that's much better.  Having compiler-version-specific branches in the thrift compiler to handle floating point oddities like this is fine.  Carrying the C++ compiler identifier down through the build system and pass into the tests as an argument/option/variable should really be avoided.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    The AppVeyor builder with the `MSVC2013` configuration generates large double constants differently as the old `MSVC2010` configuration used to do. So I will add a condition to not do the `TestRenderedDoubleConstants` test on that builder. I also suspect that this problem might be due to the architecture (x86). The result of the `CYGWIN x86` builder will give us some clue on this.  


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    I have just disabled my new tests on `MSVC2013` builders and kicked a build. Let's see what happens.
    If the tests also fail on the `CYGWIN` machine, I will also write out new tests for the x86 machines. However, the issue is not about the architecture, only some modifiers seem to be overlooked in the generators. 


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Okay, I will open it back once I am done.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Yes, however, it needs to be squashed to a single commit.  I would prefer if you did that, but I can do it if you really need me to.  It's just a lot more work for the committers to do that, and we have a lot of work we're doing already.  Looks like a good change though - thanks for that!  Clearly you care about your doubles a lot.
    
    https://softwareengineering.stackexchange.com/questions/263164/why-squash-git-commits-for-pull-requests


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    I merged the changes in the master branch into mine, and ran the builds. There are still some errors: 
    
    On AppVeyor: 
    ```
    3125"C:\projects\build\MSVC2017\x64\INSTALL.vcxproj" (default target) (1) ->
    3126"C:\projects\build\MSVC2017\x64\ALL_BUILD.vcxproj" (default target) (3) ->
    3127"C:\projects\build\MSVC2017\x64\test\cpp\TestServer.vcxproj" (default target) (32) ->
    3128(ClCompile target) -> 
    3129  C:\projects\thrift\test\cpp\src\TestServer.cpp(799): error C3861: 'sleep': identifier not found [C:\projects\build\MSVC2017\x64\test\cpp\TestServer.vcxproj]
    3130
    3131    24 Warning(s)
    3132    1 Error(s)
    ```
    
    On Travis: 
    ```
    depbase=`echo gen-c_glib/t_test_second_service.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
    /bin/bash ../../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I../.. -I../../lib/cpp/src/thrift -I../../lib/c_glib/src/thrift  -I../../lib/c_glib/src -Igen-c_glib  -g -Wall -Wextra -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -O2 -MT gen-c_glib/t_test_second_service.lo -MD -MP -MF $depbase.Tpo -c -o gen-c_glib/t_test_second_service.lo gen-c_glib/t_test_second_service.c &&\
    mv -f $depbase.Tpo $depbase.Plo
    libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I../.. -I../../lib/cpp/src/thrift -I../../lib/c_glib/src/thrift -I../../lib/c_glib/src -Igen-c_glib -g -Wall -Wextra -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -O2 -MT gen-c_glib/t_test_second_service.lo -MD -MP -MF gen-c_glib/.deps/t_test_second_service.Tpo -c gen-c_glib/t_test_second_service.c  -fPIC -DPIC -o gen-c_glib/.libs/t_test_second_service.o
    gen-c_glib/t_test_second_service.c: In function 't_test_second_service_processor_process_secondtest_string':
    gen-c_glib/t_test_second_service.c:397:3: error: unknown type name 'TTestSecondServiceSecondtestStringArgs'; did you mean 'TTestSecondServiceProcessorClass'?
       TTestSecondServiceSecondtestStringArgs * args =
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       TTestSecondServiceProcessorClass
    gen-c_glib/t_test_second_service.c:398:19: error: 'T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_ARGS' undeclared (first use in this function); did you mean 'T_TEST_TYPE_SECOND_SERVICE_PROCESSOR'?
         g_object_new (T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_ARGS, NULL);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                       T_TEST_TYPE_SECOND_SERVICE_PROCESSOR
    gen-c_glib/t_test_second_service.c:398:19: note: each undeclared identifier is reported only once for each function it appears in
    gen-c_glib/t_test_second_service.c:408:5: error: unknown type name 'TTestSecondServiceSecondtestStringResult'; did you mean 'TTestSecondServiceProcessorClass'?
         TTestSecondServiceSecondtestStringResult * result_struct;
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         TTestSecondServiceProcessorClass
    gen-c_glib/t_test_second_service.c:417:35: error: 'T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_RESULT' undeclared (first use in this function); did you mean 'T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_ARGS'?
         result_struct = g_object_new (T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_RESULT, NULL);
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                       T_TEST_TYPE_SECOND_SERVICE_SECONDTEST_STRING_ARGS
    depbase=`echo gen-c_glib/t_test_thrift_test.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
    /bin/bash ../../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I../.. -I../../lib/cpp/src/thrift -I../../lib/c_glib/src/thrift  -I../../lib/c_glib/src -Igen-c_glib  -g -Wall -Wextra -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -O2 -MT gen-c_glib/t_test_thrift_test.lo -MD -MP -MF $depbase.Tpo -c -o gen-c_glib/t_test_thrift_test.lo gen-c_glib/t_test_thrift_test.c &&\
    mv -f $depbase.Tpo $depbase.Plo
    depbase=`echo gen-c_glib/t_test_thrift_test_types.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
    /bin/bash ../../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I../.. -I../../lib/cpp/src/thrift -I../../lib/c_glib/src/thrift  -I../../lib/c_glib/src -Igen-c_glib  -g -Wall -Wextra -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -O2 -MT gen-c_glib/t_test_thrift_test_types.lo -MD -MP -MF $depbase.Tpo -c -o gen-c_glib/t_test_thrift_test_types.lo gen-c_glib/t_test_thrift_test_types.c &&\
    mv -f $depbase.Tpo $depbase.Plo
    Makefile:654: recipe for target 'gen-c_glib/t_test_second_service.lo' failed
    ```
    
    ```
    npm ERR! missing: hawk@3.1.3, required by node-pre-gyp@0.6.39
    npm ERR! missing: mkdirp@0.5.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: rimraf@2.6.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: tar@2.2.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: boom@2.10.1, required by hawk@3.1.3
    npm ERR! missing: cryptiles@2.0.5, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by hawk@3.1.3
    npm ERR! missing: sntp@1.0.9, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by boom@2.10.1
    npm ERR! missing: boom@2.10.1, required by cryptiles@2.0.5
    npm ERR! missing: hoek@2.16.3, required by sntp@1.0.9
    npm ERR! missing: minimist@0.0.8, required by mkdirp@0.5.1
    npm ERR! missing: console-control-strings@1.1.0, required by npmlog@4.1.0
    npm ERR! missing: readable-stream@2.2.9, required by are-we-there-yet@1.1.4
    npm ERR! missing: console-control-strings@1.1.0, required by gauge@2.7.4
    npm ERR! missing: string-width@1.0.2, required by gauge@2.7.4
    npm ERR! missing: strip-ansi@3.0.1, required by gauge@2.7.4
    npm ERR! missing: code-point-at@1.1.0, required by string-width@1.0.2
    npm ERR! missing: is-fullwidth-code-point@1.0.0, required by string-width@1.0.2
    npm ERR! missing: strip-ansi@3.0.1, required by string-width@1.0.2
    npm ERR! missing: number-is-nan@1.0.1, required by is-fullwidth-code-point@1.0.0
    npm ERR! missing: ansi-regex@2.1.1, required by strip-ansi@3.0.1
    npm ERR! missing: string-width@1.0.2, required by wide-align@1.1.2
    npm ERR! missing: combined-stream@1.0.5, required by request@2.81.0
    npm ERR! missing: hawk@3.1.3, required by request@2.81.0
    npm ERR! missing: mime-types@2.1.15, required by request@2.81.0
    npm ERR! missing: safe-buffer@5.0.1, required by request@2.81.0
    npm ERR! missing: delayed-stream@1.0.0, required by combined-stream@1.0.5
    npm ERR! missing: combined-stream@1.0.5, required by form-data@2.1.4
    npm ERR! missing: mime-types@2.1.15, required by form-data@2.1.4
    npm ERR! missing: extsprintf@1.0.2, required by jsprim@1.4.0
    npm ERR! missing: extsprintf@1.0.2, required by verror@1.3.6
    npm ERR! missing: mime-db@1.27.0, required by mime-types@2.1.15
    npm ERR! missing: safe-buffer@5.0.1, required by tunnel-agent@0.6.0
    npm ERR! missing: glob@7.1.2, required by rimraf@2.6.1
    npm ERR! missing: fs.realpath@1.0.0, required by glob@7.1.2
    npm ERR! missing: inflight@1.0.6, required by glob@7.1.2
    npm ERR! missing: inherits@2.0.3, required by glob@7.1.2
    npm ERR! missing: minimatch@3.0.4, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by glob@7.1.2
    npm ERR! missing: path-is-absolute@1.0.1, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by inflight@1.0.6
    npm ERR! missing: wrappy@1.0.2, required by inflight@1.0.6
    npm ERR! missing: brace-expansion@1.1.7, required by minimatch@3.0.4
    npm ERR! missing: balanced-match@0.4.2, required by brace-expansion@1.1.7
    npm ERR! missing: concat-map@0.0.1, required by brace-expansion@1.1.7
    npm ERR! missing: block-stream@0.0.9, required by tar@2.2.1
    npm ERR! missing: fstream@1.0.11, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by block-stream@0.0.9
    npm ERR! missing: graceful-fs@4.1.11, required by fstream@1.0.11
    npm ERR! missing: inherits@2.0.3, required by fstream@1.0.11
    npm ERR! missing: mkdirp@0.5.1, required by fstream@1.0.11
    npm ERR! missing: rimraf@2.6.1, required by fstream@1.0.11
    npm ERR! missing: fstream@1.0.11, required by tar-pack@3.4.0
    npm ERR! missing: once@1.4.0, required by tar-pack@3.4.0
    npm ERR! missing: readable-stream@2.2.9, required by tar-pack@3.4.0
    npm ERR! missing: rimraf@2.6.1, required by tar-pack@3.4.0
    npm ERR! missing: tar@2.2.1, required by tar-pack@3.4.0
    npm ERR! missing: fstream@1.0.11, required by fstream-ignore@1.0.5
    npm ERR! missing: inherits@2.0.3, required by fstream-ignore@1.0.5
    npm ERR! missing: minimatch@3.0.4, required by fstream-ignore@1.0.5
    npm ERR! missing: wrappy@1.0.2, required by once@1.4.0
    npm ERR! missing: buffer-shims@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: core-util-is@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: inherits@2.0.3, required by readable-stream@2.2.9
    npm ERR! missing: isarray@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: process-nextick-args@1.0.7, required by readable-stream@2.2.9
    npm ERR! missing: string_decoder@1.0.1, required by readable-stream@2.2.9
    npm ERR! missing: util-deprecate@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: safe-buffer@5.0.1, required by string_decoder@1.0.1
    Makefile:811: recipe for target 'check-local' failed
    make[4]: *** [check-local] Error 1
    make[4]: Leaving directory '/thrift/src/lib/js'
    Makefile:686: recipe for target 'check-am' failed
    make[3]: *** [check-am] Error 2
    make[3]: Leaving directory '/thrift/src/lib/js'
    Makefile:536: recipe for target 'check-recursive' failed
    make[2]: *** [check-recursive] Error 1
    make[2]: Leaving directory '/thrift/src/lib/js'
    Makefile:580: recipe for target 'check-recursive' failed
    make[1]: *** [check-recursive] Error 1
    make[1]: Leaving directory '/thrift/src/lib'
    Makefile:662: recipe for target 'check-recursive' failed
    make: *** [check-recursive] Error 1
    ```
    
    Am I missing something? @jeking3 


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174687088
  
    --- Diff: test/py/CMakeLists.txt ---
    @@ -27,7 +27,7 @@ add_test(NAME python_test_generate
     )
     
     add_test(NAME python_test
    -    COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/RunClientServer.py --gen-base=${CMAKE_CURRENT_BINARY_DIR}
    +    COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/RunClientServer.py --gen-base=${CMAKE_CURRENT_BINARY_DIR} --msvc-profile=${MSVC_PROFILE}
    --- End diff --
    
    @jeking3 By the way, I also disabled my Python tests. Here are the places I modified to disable a test suite in Python.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Sourceforge was down for a while.  I am working on getting the builds back to green (passing).  I recommend that you keep your pull request squashed to one commit.  In fact since you are doing iterative development you are clogging up the pull request build pipeline.  I recommend you close this pull request and work on your own fork until you are satisfied you have resolved the issues.  I am hoping to have the builds back to "green" by the end of today.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Need to squash to one commit, ideally, to make a merge easier, and fix the issue causing older MSVC compilers (which people still use...) to generate different floating constants than other compilers.  There could be a compiler flag, or a way to fix it in the compiler.  It's great that we added tests to verify the output of the compiler; we shouldn't disable the new tests just because the C++ compiler is different.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174494320
  
    --- Diff: lib/java/gradle/unitTests.gradle ---
    @@ -68,6 +68,20 @@ test {
         include '**/Test*.class'
         exclude '**/Test*\$*.class'
     
    +    println 'Check if msvc.profile exists'
    --- End diff --
    
    It seems to me this fix is in the wrong place.  I don't like having the C++ compiler type passed down into the java build so it can disable a test.  The compiler code be modified somehow.



---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r175276410
  
    --- Diff: compiler/cpp/src/thrift/generate/t_generator.h ---
    @@ -268,6 +271,30 @@ class t_generator {
         return out.str();
       }
     
    +  const std::string emit_double_as_string(const double value) {
    +      std::stringstream double_output_stream;
    +      // sets the maximum precision: http://en.cppreference.com/w/cpp/io/manip/setprecision
    +      // sets the output format to fixed: http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
    +      double_output_stream << std::setprecision(std::numeric_limits<double>::digits10 + 1);
    +
    +      #ifdef _MSC_VER
    +          // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
    +          // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
    +          //               c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
    +          //               and
    +          //               http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
    +          #if _MSC_VER >= MSC_2015_VER
    +              double_output_stream << std::fixed;
    +          #endif
    +      #else
    +          double_output_stream << std::fixed;
    --- End diff --
    
    Did you mean both cases to be the same?  Both use std::fixed; should one be using std::scientific?


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    I have reverted the changes on passing the compiler flag to the testers, and modified the double emitter. 


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx closed the pull request at:

    https://github.com/apache/thrift/pull/1496


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Thanks! I have just caught the meaning of squash since it's been the first time i try to contribute an open source project. I will squash all into a single one and let you know from here.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx closed the pull request at:

    https://github.com/apache/thrift/pull/1496


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    I finalized the changes, however there are a couple of errors in my builds which I think are irrelevant to my changes.
    
    Here are the links to my build results (please let me know if you cannot access them):
    Travis-CI: https://travis-ci.org/ozymaxx/thrift
    AppVeyor: https://ci.appveyor.com/project/ozymaxx/thrift/build/1.0.0-dev.42
    
    Here are the error descriptions:
    On Travis CI `#19.7`:
    ```
      +-- js-reporters@1.2.1
      +-- resolve@1.5.0
      | `-- path-parse@1.0.5
      +-- shelljs@0.2.6
      `-- walk-sync@0.3.2
        +-- ensure-posix-path@1.0.2
        `-- matcher-collection@1.0.5
          `-- minimatch@3.0.4
            `-- brace-expansion@1.1.11 deduped
    npm ERR! missing: hawk@3.1.3, required by node-pre-gyp@0.6.39
    npm ERR! missing: mkdirp@0.5.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: rimraf@2.6.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: tar@2.2.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: boom@2.10.1, required by hawk@3.1.3
    npm ERR! missing: cryptiles@2.0.5, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by hawk@3.1.3
    npm ERR! missing: sntp@1.0.9, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by boom@2.10.1
    npm ERR! missing: boom@2.10.1, required by cryptiles@2.0.5
    npm ERR! missing: hoek@2.16.3, required by sntp@1.0.9
    npm ERR! missing: minimist@0.0.8, required by mkdirp@0.5.1
    npm ERR! missing: console-control-strings@1.1.0, required by npmlog@4.1.0
    npm ERR! missing: readable-stream@2.2.9, required by are-we-there-yet@1.1.4
    npm ERR! missing: console-control-strings@1.1.0, required by gauge@2.7.4
    npm ERR! missing: string-width@1.0.2, required by gauge@2.7.4
    npm ERR! missing: strip-ansi@3.0.1, required by gauge@2.7.4
    npm ERR! missing: code-point-at@1.1.0, required by string-width@1.0.2
    npm ERR! missing: is-fullwidth-code-point@1.0.0, required by string-width@1.0.2
    npm ERR! missing: strip-ansi@3.0.1, required by string-width@1.0.2
    npm ERR! missing: number-is-nan@1.0.1, required by is-fullwidth-code-point@1.0.0
    npm ERR! missing: ansi-regex@2.1.1, required by strip-ansi@3.0.1
    npm ERR! missing: string-width@1.0.2, required by wide-align@1.1.2
    npm ERR! missing: combined-stream@1.0.5, required by request@2.81.0
    npm ERR! missing: hawk@3.1.3, required by request@2.81.0
    npm ERR! missing: mime-types@2.1.15, required by request@2.81.0
    npm ERR! missing: safe-buffer@5.0.1, required by request@2.81.0
    npm ERR! missing: delayed-stream@1.0.0, required by combined-stream@1.0.5
    npm ERR! missing: combined-stream@1.0.5, required by form-data@2.1.4
    npm ERR! missing: mime-types@2.1.15, required by form-data@2.1.4
    npm ERR! missing: extsprintf@1.0.2, required by jsprim@1.4.0
    npm ERR! missing: extsprintf@1.0.2, required by verror@1.3.6
    npm ERR! missing: mime-db@1.27.0, required by mime-types@2.1.15
    npm ERR! missing: safe-buffer@5.0.1, required by tunnel-agent@0.6.0
    npm ERR! missing: glob@7.1.2, required by rimraf@2.6.1
    npm ERR! missing: fs.realpath@1.0.0, required by glob@7.1.2
    npm ERR! missing: inflight@1.0.6, required by glob@7.1.2
    npm ERR! missing: inherits@2.0.3, required by glob@7.1.2
    npm ERR! missing: minimatch@3.0.4, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by glob@7.1.2
    npm ERR! missing: path-is-absolute@1.0.1, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by inflight@1.0.6
    npm ERR! missing: wrappy@1.0.2, required by inflight@1.0.6
    npm ERR! missing: brace-expansion@1.1.7, required by minimatch@3.0.4
    npm ERR! missing: balanced-match@0.4.2, required by brace-expansion@1.1.7
    npm ERR! missing: concat-map@0.0.1, required by brace-expansion@1.1.7
    npm ERR! missing: block-stream@0.0.9, required by tar@2.2.1
    npm ERR! missing: fstream@1.0.11, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by block-stream@0.0.9
    npm ERR! missing: graceful-fs@4.1.11, required by fstream@1.0.11
    npm ERR! missing: inherits@2.0.3, required by fstream@1.0.11
    npm ERR! missing: mkdirp@0.5.1, required by fstream@1.0.11
    npm ERR! missing: rimraf@2.6.1, required by fstream@1.0.11
    npm ERR! missing: fstream@1.0.11, required by tar-pack@3.4.0
    npm ERR! missing: once@1.4.0, required by tar-pack@3.4.0
    npm ERR! missing: readable-stream@2.2.9, required by tar-pack@3.4.0
    npm ERR! missing: rimraf@2.6.1, required by tar-pack@3.4.0
    npm ERR! missing: tar@2.2.1, required by tar-pack@3.4.0
    npm ERR! missing: fstream@1.0.11, required by fstream-ignore@1.0.5
    npm ERR! missing: inherits@2.0.3, required by fstream-ignore@1.0.5
    npm ERR! missing: minimatch@3.0.4, required by fstream-ignore@1.0.5
    npm ERR! missing: wrappy@1.0.2, required by once@1.4.0
    npm ERR! missing: buffer-shims@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: core-util-is@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: inherits@2.0.3, required by readable-stream@2.2.9
    npm ERR! missing: isarray@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: process-nextick-args@1.0.7, required by readable-stream@2.2.9
    npm ERR! missing: string_decoder@1.0.1, required by readable-stream@2.2.9
    npm ERR! missing: util-deprecate@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: safe-buffer@5.0.1, required by string_decoder@1.0.1
    Makefile:811: recipe for target 'check-local' failed
    make[4]: *** [check-local] Error 1
    make[4]: Leaving directory '/thrift/src/lib/js'
    Makefile:686: recipe for target 'check-am' failed
    make[3]: *** [check-am] Error 2
    make[3]: Leaving directory '/thrift/src/lib/js'
    Makefile:536: recipe for target 'check-recursive' failed
    make[2]: *** [check-recursive] Error 1
    make[2]: Leaving directory '/thrift/src/lib/js'
    Makefile:580: recipe for target 'check-recursive' failed
    make[1]: *** [check-recursive] Error 1
    make[1]: Leaving directory '/thrift/src/lib'
    Makefile:662: recipe for target 'check-recursive' failed
    make: *** [check-recursive] Error 1
    ```
    
    On Travis CI `19.11`:
    ```
    
    npm ERR! missing: hawk@3.1.3, required by node-pre-gyp@0.6.39
    npm ERR! missing: mkdirp@0.5.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: rimraf@2.6.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: tar@2.2.1, required by node-pre-gyp@0.6.39
    npm ERR! missing: boom@2.10.1, required by hawk@3.1.3
    npm ERR! missing: cryptiles@2.0.5, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by hawk@3.1.3
    npm ERR! missing: sntp@1.0.9, required by hawk@3.1.3
    npm ERR! missing: hoek@2.16.3, required by boom@2.10.1
    npm ERR! missing: boom@2.10.1, required by cryptiles@2.0.5
    npm ERR! missing: hoek@2.16.3, required by sntp@1.0.9
    npm ERR! missing: minimist@0.0.8, required by mkdirp@0.5.1
    npm ERR! missing: console-control-strings@1.1.0, required by npmlog@4.1.0
    npm ERR! missing: readable-stream@2.2.9, required by are-we-there-yet@1.1.4
    npm ERR! missing: console-control-strings@1.1.0, required by gauge@2.7.4
    npm ERR! missing: string-width@1.0.2, required by gauge@2.7.4
    npm ERR! missing: strip-ansi@3.0.1, required by gauge@2.7.4
    npm ERR! missing: code-point-at@1.1.0, required by string-width@1.0.2
    npm ERR! missing: is-fullwidth-code-point@1.0.0, required by string-width@1.0.2
    npm ERR! missing: strip-ansi@3.0.1, required by string-width@1.0.2
    npm ERR! missing: number-is-nan@1.0.1, required by is-fullwidth-code-point@1.0.0
    npm ERR! missing: ansi-regex@2.1.1, required by strip-ansi@3.0.1
    npm ERR! missing: string-width@1.0.2, required by wide-align@1.1.2
    npm ERR! missing: combined-stream@1.0.5, required by request@2.81.0
    npm ERR! missing: hawk@3.1.3, required by request@2.81.0
    npm ERR! missing: mime-types@2.1.15, required by request@2.81.0
    npm ERR! missing: safe-buffer@5.0.1, required by request@2.81.0
    npm ERR! missing: delayed-stream@1.0.0, required by combined-stream@1.0.5
    npm ERR! missing: combined-stream@1.0.5, required by form-data@2.1.4
    npm ERR! missing: mime-types@2.1.15, required by form-data@2.1.4
    npm ERR! missing: extsprintf@1.0.2, required by jsprim@1.4.0
    npm ERR! missing: extsprintf@1.0.2, required by verror@1.3.6
    npm ERR! missing: mime-db@1.27.0, required by mime-types@2.1.15
    npm ERR! missing: safe-buffer@5.0.1, required by tunnel-agent@0.6.0
    npm ERR! missing: glob@7.1.2, required by rimraf@2.6.1
    npm ERR! missing: fs.realpath@1.0.0, required by glob@7.1.2
    npm ERR! missing: inflight@1.0.6, required by glob@7.1.2
    npm ERR! missing: inherits@2.0.3, required by glob@7.1.2
    npm ERR! missing: minimatch@3.0.4, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by glob@7.1.2
    npm ERR! missing: path-is-absolute@1.0.1, required by glob@7.1.2
    npm ERR! missing: once@1.4.0, required by inflight@1.0.6
    npm ERR! missing: wrappy@1.0.2, required by inflight@1.0.6
    npm ERR! missing: brace-expansion@1.1.7, required by minimatch@3.0.4
    npm ERR! missing: balanced-match@0.4.2, required by brace-expansion@1.1.7
    npm ERR! missing: concat-map@0.0.1, required by brace-expansion@1.1.7
    npm ERR! missing: block-stream@0.0.9, required by tar@2.2.1
    npm ERR! missing: fstream@1.0.11, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by tar@2.2.1
    npm ERR! missing: inherits@2.0.3, required by block-stream@0.0.9
    npm ERR! missing: graceful-fs@4.1.11, required by fstream@1.0.11
    npm ERR! missing: inherits@2.0.3, required by fstream@1.0.11
    npm ERR! missing: mkdirp@0.5.1, required by fstream@1.0.11
    npm ERR! missing: rimraf@2.6.1, required by fstream@1.0.11
    npm ERR! missing: fstream@1.0.11, required by tar-pack@3.4.0
    npm ERR! missing: once@1.4.0, required by tar-pack@3.4.0
    npm ERR! missing: readable-stream@2.2.9, required by tar-pack@3.4.0
    npm ERR! missing: rimraf@2.6.1, required by tar-pack@3.4.0
    npm ERR! missing: tar@2.2.1, required by tar-pack@3.4.0
    npm ERR! missing: fstream@1.0.11, required by fstream-ignore@1.0.5
    npm ERR! missing: inherits@2.0.3, required by fstream-ignore@1.0.5
    npm ERR! missing: minimatch@3.0.4, required by fstream-ignore@1.0.5
    npm ERR! missing: wrappy@1.0.2, required by once@1.4.0
    npm ERR! missing: buffer-shims@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: core-util-is@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: inherits@2.0.3, required by readable-stream@2.2.9
    npm ERR! missing: isarray@1.0.0, required by readable-stream@2.2.9
    npm ERR! missing: process-nextick-args@1.0.7, required by readable-stream@2.2.9
    npm ERR! missing: string_decoder@1.0.1, required by readable-stream@2.2.9
    npm ERR! missing: util-deprecate@1.0.2, required by readable-stream@2.2.9
    npm ERR! missing: safe-buffer@5.0.1, required by string_decoder@1.0.1
    Makefile:811: recipe for target 'check-local' failed
    make[4]: *** [check-local] Error 1
    make[4]: Leaving directory '/thrift/src/lib/js'
    Makefile:686: recipe for target 'check-am' failed
    make[3]: *** [check-am] Error 2
    make[3]: Leaving directory '/thrift/src/lib/js'
    Makefile:536: recipe for target 'check-recursive' failed
    make[2]: *** [check-recursive] Error 1
    make[2]: Leaving directory '/thrift/src/lib/js'
    Makefile:580: recipe for target 'check-recursive' failed
    make[1]: *** [check-recursive] Error 1
    make[1]: Leaving directory '/thrift/src/lib'
    Makefile:662: recipe for target 'check-recursive' failed
    make: *** [check-recursive] Error 1
    ```
    
    On AppVeyor `Environment: PROFILE=CYGWIN, PLATFORM=x64, CONFIGURATION=RelWithDebInfo, DISABLED_TESTS=(ZlibTest|OpenSSLManualInitTest|TNonblockingServerTest|StressTestNonBlocking)`:
    ```
    2314Scanning dependencies of target thrift_static
    2315      0 [main] make 3676 fork: child -1 - forked process 3656 died unexpectedly, retry 0, exit code 0xC0000005, errno 11
    2316make[2]: fork: Resource temporarily unavailable
    2317make[1]: *** [CMakeFiles/Makefile2:1171: lib/cpp/CMakeFiles/thrift_static.dir/all] Error 2
    2318make: *** [Makefile:161: all] Error 2
    2319
    2320C:\projects\thrift>set lastexitcode=2 
    2321
    2322C:\projects\thrift>set  1>C:\Users\appveyor\AppData\Local\Temp\tmp86A3.tmp 
    2323
    2324C:\projects\thrift>echo C:\projects\thrift  1>C:\Users\appveyor\AppData\Local\Temp\tmp86A4.tmp 
    2325
    2326C:\projects\thrift>exit /b 2 
    2327
    ```


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Thanks for dealing with the relatively unstable CI.  Our project has support for 25 languages, so needless to say without a fixed build environment (which is impossible, since packages change over time), the build becomes unstable pretty quickly without constant TLC.
    
    Please squash this to a single commit to prepare it for merge.
    
    In addition, I will need to make sure this test passes:
    ```
    ===============================================================================
    *** Following 1 failures were unexpected ***:
    If it is introduced by you, please fix it before submitting the code.
    ===============================================================================
    server-client:          protocol:         transport:               result:
    hs-csharp               json              framed-ip                failure(1)
    ===============================================================================
    Unexpected failures are logged to test/log/unexpected_failures.log
    You can browse results at:
    	file:///thrift/src/test/index.html
    # If you use Chrome, run:
    # 	cd /thrift/src
    #	python -m http.server 8001
    # then browse:
    # 	http://localhost:8001/test/
    Full log for each test is here:
    	test/log/server_client_protocol_transport_client.log
    	test/log/server_client_protocol_transport_server.log
    1 failed of 3948 tests in total.
    Test execution took 1966.1 seconds.
    Wed Mar 14 13:26:25 2018
    ```


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    All green! :)


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    It appears that there is another compilation problem in Erlang. In the file `test/DoubleConstantsTest.thrift`, the double literal `1000000.1` is compiled to `1e+06`. However, Erlang does not accept exponent literals where the number before the exponent does not have floating point. This causes an error in some tests.


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    @jeking3 or another alternative path to go: emitting double can depend on the msvc compiler version. If the compiler version is 2013 or less, we can ignore using fixed. 


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx closed the pull request at:

    https://github.com/apache/thrift/pull/1496


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
GitHub user ozymaxx reopened a pull request:

    https://github.com/apache/thrift/pull/1496

    THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python)

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++
    
    see the related issue: https://issues.apache.org/jira/browse/THRIFT-4476

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ozymaxx/thrift THRIFT-4476

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1496.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 #1496
    
----
commit f5a6337d4448a30c284a3033307865dfc992ac7d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-19T13:55:09Z

    - high precision double rendering in JS, Java, Python and C++
    - testing double rendering in JS, Java, Python and C++

commit 6dbc35f74fe53a9bacf06867036c21f2761c823e
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-22T09:47:27Z

    - a comment added (about setprecision)
    - BOOST_TEST -> BOOST_CHECK

commit 55e3371ed9ea7eadc5826a7f17d3e432e9b99858
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:10:15Z

    double rendering in erlang, fixed (some additional tests added)

commit 36cc6f9dfe5b508a531ad70958ca3209c3b65e84
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-23T14:44:31Z

    less than operator - correction in erlang test

commit a22d72f024bbf463895022792a31d04054a79445
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:17:16Z

    style changes, some fixes in erlang test

commit a615812729096a7fa106f8259eb2f41d61ac99ce
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T06:51:24Z

    some fixes in erlang test

commit 50b9a3fc69b82a9af2e78602a2e60b9a649874ee
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T07:22:41Z

    some fixes in erlang test - no need for including lists library

commit b4e9ce56aecf75beffbf77864f02891808072204
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T09:31:46Z

    more style changes - something overlooked

commit 39bb546d0ccbec7177866a1a78d9b5f4de0d9ec1
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T10:24:45Z

    expected 2 lines before main - fixed (not seen locally)

commit 98db81883af06d10e9277fb32988edfd3f044d1d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-26T11:24:05Z

    noticed that test cases should be added manually in Python

commit 64aae330efac4ee23ed01694bc7dceb5a7a9164b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-27T06:48:48Z

    DoubleConstantTest thrift compilation directive is now given in generate.cmake

commit decb436f40990e0a04be9730accd3becab951c39
Author: Ozan Can Altiok <oz...@...>
Date:   2018-02-28T06:13:34Z

    render_double_to_string -> emit_double_as_string

commit e9b2e389b1184efa4d8ad1d6424ec52a108b80c9
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-02T12:45:54Z

    added assertion error messages to see the problems inside Java tests in more detail

commit 248e8a4c208e518da30f761a2d836ff790cd9b0d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T06:59:59Z

    RDP access to AppVeyor build workers

commit dd9b65113f143f13e6524917add6c216b16f86e5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:01:07Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

commit 65a6588d049dcbbef7d3df5d7c3ba9a7b141917a
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T07:03:20Z

    duplicate key problem in appveyor settings

commit 6305384d9203bd532983d18a992515db802b29a5
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T09:31:22Z

    ignore the test cases with (-+)1.7e+308 in Java tests

commit 74667acf55c435c661c5fc19de729eb5f82758dc
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T10:00:31Z

    ignore the test cases with (-+)1.7e+308 in Java tests - 2

commit 85b872bac39078446aa27336ca57001022524fc0
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-05T12:18:14Z

    pyflakes8 style changes

commit af11ef8c7475c5f57f2d6619bedea23856a0fd17
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:21:51Z

    disabling the test named 'TestRenderedDoubleConstants' on machines with the MSVC2010 configuration

commit 8ed1997141dce6c4b4505b09d2d6843344dfa04b
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T08:58:25Z

    msvc.profile parameter existence check

commit beb12744f990ed0890648b07d81f19c121303c59
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:05:04Z

    passing "msvc profile" on build stage

commit 1a0756aa8d48b693ae39dd860bdbeb7de5252cae
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T12:50:10Z

    "MSVC_PROFILE" as a cache variable

commit b40fc4e06083f2787db3f805756abe7e6c2c704d
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:01:48Z

    wrong usage of cache variables

commit ef43030fd69f1dee060c3cf447acfcceefb4be71
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:19:42Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit ec32218d28d6054f6fa5148c0d5d24236660afcf
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T13:23:39Z

    let's see whether the MSVC_PROFILE parameter is passed correctly

commit bd1207f29b0662022e46371d1b1cc7be37013151
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:03:27Z

    let's see whether TestRenderedDoubleConstants is excluded or not

commit 6c3e243c2afc09d726113ae578b5236e673640ad
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-06T14:49:41Z

    msvc.profile access - fixed

commit 201fcb541ae3a5774b509efc5f3d036c85e2495f
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:22:34Z

    msvc.profile access - fixed

commit 788118364ae60781f56779f89b33e90aee116750
Author: Ozan Can Altiok <oz...@...>
Date:   2018-03-07T06:27:45Z

    Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476

----


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    @jeking3 Forget about what I said on Travis. There is only 1 failure in Apache arsenal of Travis (at `cross-test`) as follows:
    ```
    ===============================================================================
    Using JSON protocol
    Using framed transport
    testVoid() = void
    testString("Test") = "Test"
    testBool(true) = True
    testBool(false) = False
    testByte(1) = 1
    testI32(-1) = -1
    testI64(-34359738368) = -34359738368
    testDouble(5.325098235) = 5.325098235
    testDouble(-0.000341012439638598279) = -0.000341012439638598
    testBinary(000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7C8C9CACBCCCDCECFD0D1D2D3D4D5D6D7D8D9DADBDCDDDEDFE0E1E2E3E4E5E6E7E8E9EAEBECEDEEEFF0F1F2F3F4F5F6F7F8F9FAFBFCFDFEFF) = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7C8C9CACBCCCDCECFD0D1D2D3D4D5D6D7D8D9DADBDCDDDEDFE0E1E2E3E4E5E6E7E8E
 9EAEBECEDEEEFF0F1F2F3F4F5F6F7F8F9FAFBFCFDFEFF
    testBinary(000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7C8C9CACBCCCDCECFD0D1D2D3D4D5D6D7D8D9DADBDCDDDEDFE0E1E2E3E4E5E6E7E8E9EAEBECEDEEEFF0F1F2F3F
    .
    .
    .
    *** FAILED ***
    *** FAILED ***
    *** FAILED ***
    *** FAILED ***
    *** FAILED ***
    Test CrazyNesting
    testStruct({"Zero", 1, -3, -5}) = {"Zero", 1, -3, -5}
    testNest({1, {"Zero", 1, -3, -5}, 5}) = {1, {"Zero", 1, -3, -5}, 5}
    testMap({0 => -10, 1 => -9, 2 => -8, 3 => -7, 4 => -6}) = {0 => -10, 1 => -9, 2 => -8, 3 => -7, 4 => -6}
    testList({-2, -1, 0, 1, 2}) = {-2, -1, 0, 1, 2}
    testSet({-2, -1, 0, 1, 2}) = {0, 1, 2, -2, -1}
    testEnum(ONE) = ONE
    testEnum(TWO) = TWO
    testEnum(THREE) = THREE
    testEnum(FIVE) = FIVE
    testEnum(EIGHT) = EIGHT
    testTypedef(309858235082523) = 309858235082523
    testMapMap(1) = {4 => {1 => 1, 2 => 2, 3 => 3, 4 => 4, }, -4 => {-4 => -4, -3 => -3, -2 => -2, -1 => -1, }, }
    testInsanity() = {1 => {TWO => {{FIVE => 5000, }, {{"Truck", 8, 8, 8}, }}, THREE => {{FIVE => 5000, }, {{"Truck", 8, 8, 8}, }}, }, 2 => {SIX => {{}, {}}, }, }
    Test Multi(1,2,9223372036854775807,System.Collections.Generic.Dictionary`2[System.Int16,System.String],FIVE,5000000) = Xtruct(byte_thing:1,String_thing:Hello2,i32_thing:2,i64_thing:9223372036854775807)
    testException("Xception")
    testException("TException")
    testException("ok")
    testMultiException("Xception", ...)
    testMultiException("Xception2", ...)
    testMultiException("success", "OK")
    Test Oneway(1)
    Test Calltime() = 59 ms a testVoid() call
    Total time: 00:00:04.3274560
    
    ===============================================================================
    Return code: 1
    Test execution took 4.5 seconds.
    Wed Mar 14 13:10:58 2018
    ```
    
    But the error is the same in the Windows builders (although it seems green here). 


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174686397
  
    --- Diff: test/DoubleConstantsTest.thrift ---
    @@ -0,0 +1,17 @@
    +namespace java thrift.test
    +namespace cpp thrift.test
    +
    +// more tests on double constants (precision and type checks)
    --- End diff --
    
    If I'm not mistaken, only Java and Python tests are running on Windows builders. One of the C++ compilers on these builders compiles the Thrift compiler source code such that the produced relatively large double constants does not match the ones given in the test. That's why I have disabled `TestRenderedDoubleConstants` only for Java and Python.


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r175284941
  
    --- Diff: build/appveyor/MSVC-appveyor-build.bat ---
    @@ -39,7 +39,7 @@ CD "%BUILDDIR%"                             || EXIT /B
         -DWITH_PYTHON=%WITH_PYTHON% ^
         -DWITH_%THREADMODEL%THREADS=ON ^
         -DWITH_SHARED_LIB=OFF ^
    -    -DWITH_STATIC_LIB=ON                    || EXIT /B
    +    -DWITH_STATIC_LIB=ON ^                    || EXIT /B
    --- End diff --
    
    Yes, you're right. I've removed it now. 


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r174771623
  
    --- Diff: test/DoubleConstantsTest.thrift ---
    @@ -0,0 +1,17 @@
    +namespace java thrift.test
    +namespace cpp thrift.test
    +
    +// more tests on double constants (precision and type checks)
    --- End diff --
    
    There's probably another way to resolve this, such as fixing a compiler flag being passed to older MSVC2013 compilers.  I'll swing back around to this one when I can.  I don't like the notion that we say, "because the compiler was built with MSVC2013, let's disable a test".  Shouldn't any thrift compiler, regardless of the C++ compiler used to create it, produce working thrift generated code that has the same values as any other thrift compiler?


---

[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1496#discussion_r175276394
  
    --- Diff: build/appveyor/MSVC-appveyor-build.bat ---
    @@ -39,7 +39,7 @@ CD "%BUILDDIR%"                             || EXIT /B
         -DWITH_PYTHON=%WITH_PYTHON% ^
         -DWITH_%THREADMODEL%THREADS=ON ^
         -DWITH_SHARED_LIB=OFF ^
    -    -DWITH_STATIC_LIB=ON                    || EXIT /B
    +    -DWITH_STATIC_LIB=ON ^                    || EXIT /B
    --- End diff --
    
    The carat addition looks wrong to me; it belongs on the end of a string, so in this case I think it does nothing at all?


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    Appveyor: It looks like I broke the build yesterday with a check-in that I did not run through CI.  I am fixing it.
    
    Travis: c_glib, that's new; npm; that might be an intermittent failure
    
    Could you squash your PR to one commit to prepare it for merge?


---

[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...

Posted by ozymaxx <gi...@git.apache.org>.
Github user ozymaxx commented on the issue:

    https://github.com/apache/thrift/pull/1496
  
    @jeking3 the build results have come out. 


---