[jira] [Commented] (THRIFT-4513) thrift generated java code is not stable for constants
[
https://issues.apache.org/jira/browse/THRIFT-4513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393828#comment-16393828
]
ASF GitHub Bot commented on THRIFT-4513:
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
@jeking3 Yes, thanks, PR had some assumption on Map keys being only
primitive for constant maps, which is not true in general. Latest revision
should no longer rely on it.
> thrift generated java code is not stable for constants
> --
>
> Key: THRIFT-4513
> URL: https://issues.apache.org/jira/browse/THRIFT-4513
> Project: Thrift
> Issue Type: Improvement
> Components: Java - Compiler
>Reporter: Roman Soroka
>Priority: Major
>
> In certain cases, output generated with java compiler is unstable, this
> prevents correct incremental generation, artifact caching, etc.
> This is especially crucible if the goal is reproducible builds: same results
> for the same source inputs.
> Here's example:
> Let's say we have thrift definition:
> const map CONSTANT_MAP =
> { 'key1' : \\{"enum1" : Enum1.ENUM1_VALUE1, "enum2" : Enum2.ENUM2_VALUE}
> ,
> 'key2' : \{"enum1" : Enum1.ENUM1_VALUE2, "enum2" : Enum2.ENUM2_VALUE},
> 'key3' : \{"enum1" : Enum1.ENUM1_VALUE3, "enum2" : Enum2.ENUM2_VALUE},
>
> }
> Compiler generates code like:
> public static final java.util.Map CONSTANT_MAP =
> new java.util.HashMap();
> static {
> Enum1Enum2 tmp199 = new Enum1Enum2();
> tmp199.setEnum1(com.java.package.Enum1.ENUM1_VALUE2);
> tmp199.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key2", tmp199);
> Enum1Enum2 tmp200 = new Enum1Enum2();
> tmp200.setEnum1(com.java.package.Enum1.ENUM1_VALUE1);
> tmp200.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key1", tmp200);
> Enum1Enum2 tmp201 = new Enum1Enum2();
> tmp201.setEnum1(com.java.package.Enum1.ENUM1_VALUE3);
> tmp201.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key3", tmp201);
> Possible reorderings:
> 1. Values are added in different order from run to run.
> 2. Set methods in value construction may be called in different order.
> Note: this results in equivalent java maps, so there is no runtime issues.
> Proposed remedy:
> When building construction of Constants from JSON, java code generator
> should use by-key ordering.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Github user romanoid commented on the issue: https://github.com/apache/thrift/pull/1505 @jeking3 Yes, thanks, PR had some assumption on Map keys being only primitive for constant maps, which is not true in general. Latest revision should no longer rely on it. ---
[GitHub] thrift issue #1474: Recognize fbthrift TApplicationException codes
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1474 @jfarrell can you please give a final say on this? ---
[GitHub] thrift issue #1497: Do not call workSocket() in TNonblockigServer without en...
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1497 Rebase on master and let's see what happens, thanks. ---
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393803#comment-16393803
]
ASF GitHub Bot commented on THRIFT-4476:
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.
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewhere and the items
> become integers in the generated Java code. Thus, when it comes to populating
> the array, Java cannot succeed. {{Double}} s cannot be unboxed to integer and
> Java thinks {{int}} and {{Double}} are not related.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...
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. ---
[jira] [Commented] (THRIFT-4474) PHP generator use PSR-4 default
[
https://issues.apache.org/jira/browse/THRIFT-4474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393801#comment-16393801
]
ASF GitHub Bot commented on THRIFT-4474:
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1479
Document all breaking changes in the language README.
> PHP generator use PSR-4 default
> ---
>
> Key: THRIFT-4474
> URL: https://issues.apache.org/jira/browse/THRIFT-4474
> Project: Thrift
> Issue Type: Improvement
> Components: PHP - Compiler
>Reporter: Robert Lu
>Assignee: Robert Lu
>Priority: Major
>
> Before, PHP generator generate php files like {{Types.php}}, {{Service.php}}.
> Those can only be load via
> [{{classmap}}|https://getcomposer.org/doc/04-schema.md#classmap] method. The
> latest PSR about autoload is [PSR-4|http://www.php-fig.org/psr/psr-4/].
> thrift compiler can generate PSR-4 code default, if want old-style code(which
> can only load via classmap), add {{classmap}} option.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift issue #1479: THRIFT-4474: generate PHP code use PSR-4 style default
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1479 Document all breaking changes in the language README. ---
[GitHub] thrift issue #1480: [nodejs] Fix issue with connection failures silently fai...
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1480 This needs to be retargeted at the master branch and it needs an Apache Jira thrift ticket. See https://thrift.apache.org/docs/HowToContribute ---
[jira] [Commented] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields
[
https://issues.apache.org/jira/browse/THRIFT-4495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393799#comment-16393799
]
ASF GitHub Bot commented on THRIFT-4495:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1494
> Erlang records should allow 'undefined' for non-required fields
> ---
>
> Key: THRIFT-4495
> URL: https://issues.apache.org/jira/browse/THRIFT-4495
> Project: Thrift
> Issue Type: Improvement
> Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> The Erlang records created by the Erlang code generator allow only the type
> declared by the Thrift definition file. If a field is not required, however,
> the Erlang record should also allow the value {{undefined}} (this is similar
> to a null value in other languages).
> Erlang includes a tool, dialyzer, that does type analysis of Erlang code.
> Until Erlang 19, dialyzer implicitly added `undefined` as an allowed type for
> all record fields, but as of Erlang 19 it no longer does. This means that
> dialyzer now emits error messages whenever a record is constructed and
> initial values are not specified for all of its fields.
> So, for example, the following thrift definition
> {noformat}
> struct Test {
> 1: required i32 a
> 2: i32 b
> 3: optional i32 c
> }{noformat}
> currently produced the following Erlang record:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer(),
> 'c' :: integer()}).{noformat}
> However it should produce the following:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer() | undefined,
> 'c' :: integer() | undefined}).{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4497) Erlang records should use map() for map type
[
https://issues.apache.org/jira/browse/THRIFT-4497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393798#comment-16393798
]
ASF GitHub Bot commented on THRIFT-4497:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1495
> Erlang records should use map() for map type
>
>
> Key: THRIFT-4497
> URL: https://issues.apache.org/jira/browse/THRIFT-4497
> Project: Thrift
> Issue Type: Bug
> Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> When the Thrift Erlang code generator is given the "maps" option, it
> generates records with #{} as the field type. However #{} is the type for an
> empty map. The Erlang [Types and Function
> Specifications|http://erlang.org/doc/reference_manual/typespec.html#id79546]
> says:
> {quote}
> Notice that the syntactic representation of {{map()}} is {{#\{any() =>
> any()\}}} (or {{#\{_ => _\}}}), not #\{ \}. The notation #\{ \} specifies the
> singleton type for the empty map.
> {quote}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift pull request #1495: THRIFT-4497: Use map() field type for Erlang type...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1495 ---
[GitHub] thrift pull request #1494: THRIFT-4495: Allow `undefined` for non-required E...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1494 ---
[jira] [Commented] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields
[
https://issues.apache.org/jira/browse/THRIFT-4495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393734#comment-16393734
]
ASF GitHub Bot commented on THRIFT-4495:
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1494
Ugh, build failure is dlang related, not to this.
> Erlang records should allow 'undefined' for non-required fields
> ---
>
> Key: THRIFT-4495
> URL: https://issues.apache.org/jira/browse/THRIFT-4495
> Project: Thrift
> Issue Type: Improvement
> Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> The Erlang records created by the Erlang code generator allow only the type
> declared by the Thrift definition file. If a field is not required, however,
> the Erlang record should also allow the value {{undefined}} (this is similar
> to a null value in other languages).
> Erlang includes a tool, dialyzer, that does type analysis of Erlang code.
> Until Erlang 19, dialyzer implicitly added `undefined` as an allowed type for
> all record fields, but as of Erlang 19 it no longer does. This means that
> dialyzer now emits error messages whenever a record is constructed and
> initial values are not specified for all of its fields.
> So, for example, the following thrift definition
> {noformat}
> struct Test {
> 1: required i32 a
> 2: i32 b
> 3: optional i32 c
> }{noformat}
> currently produced the following Erlang record:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer(),
> 'c' :: integer()}).{noformat}
> However it should produce the following:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer() | undefined,
> 'c' :: integer() | undefined}).{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift issue #1494: THRIFT-4495: Allow `undefined` for non-required Erlang r...
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1494 Ugh, build failure is dlang related, not to this. ---
[GitHub] thrift issue #1494: THRIFT-4495: Allow `undefined` for non-required Erlang r...
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1494 I'm running this and 4497 through local tests since it wasn't based on good CI builds. ---
[jira] [Commented] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields
[
https://issues.apache.org/jira/browse/THRIFT-4495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393733#comment-16393733
]
ASF GitHub Bot commented on THRIFT-4495:
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1494
I'm running this and 4497 through local tests since it wasn't based on good
CI builds.
> Erlang records should allow 'undefined' for non-required fields
> ---
>
> Key: THRIFT-4495
> URL: https://issues.apache.org/jira/browse/THRIFT-4495
> Project: Thrift
> Issue Type: Improvement
> Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> The Erlang records created by the Erlang code generator allow only the type
> declared by the Thrift definition file. If a field is not required, however,
> the Erlang record should also allow the value {{undefined}} (this is similar
> to a null value in other languages).
> Erlang includes a tool, dialyzer, that does type analysis of Erlang code.
> Until Erlang 19, dialyzer implicitly added `undefined` as an allowed type for
> all record fields, but as of Erlang 19 it no longer does. This means that
> dialyzer now emits error messages whenever a record is constructed and
> initial values are not specified for all of its fields.
> So, for example, the following thrift definition
> {noformat}
> struct Test {
> 1: required i32 a
> 2: i32 b
> 3: optional i32 c
> }{noformat}
> currently produced the following Erlang record:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer(),
> 'c' :: integer()}).{noformat}
> However it should produce the following:
> {noformat}
> -record('Test', {'a' :: integer(),
> 'b' :: integer() | undefined,
> 'c' :: integer() | undefined}).{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4024) Skip() should throw on unknown data types
[
https://issues.apache.org/jira/browse/THRIFT-4024?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393726#comment-16393726
]
ASF GitHub Bot commented on THRIFT-4024:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1503
> Skip() should throw on unknown data types
> -
>
> Key: THRIFT-4024
> URL: https://issues.apache.org/jira/browse/THRIFT-4024
> Project: Thrift
> Issue Type: Bug
> Components: .NETCore - Library, C# - Library, Delphi - Library, Go -
> Library, Haxe - Library
>Affects Versions: 0.10.0
>Reporter: Michael Antipin
>Assignee: Jens Geyer
>Priority: Major
> Fix For: 0.11.0
>
>
> I'm using TBinaryProtocol and a simple transport that reads from a given byte
> array.
> C# library contains the following code in TProtocolUtil.Skip(TProtocol prot,
> TType type):
> {code}
> case TType.List:
> TList list = prot.ReadListBegin();
> for (int i = 0; i < list.Count; i++) {
> Skip(prot, list.ElementType);
> }
> prot.ReadListEnd();
> break;
> {code}
> The type of elements is detected in ReadListBegin(), and, as Skip() does
> nothing for unknown types, the position in the binary remains the same until
> the for loop completes.
> So, when you try to deserialize invalid data, and a field type happens to be
> detected as TType.List, you may end up waiting for a random period of time
> until deserialization is completed (734707176 iterations of skipping in my
> case).
> I suggest throwing an exception immediately when list elements type is
> unknown. May be, it would be good to have a setting like *FailOnUnknownType*,
> so that Skip() will throw instead of ignoring.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift pull request #1503: THRIFT-4024: Skip() throws TProtocolException.INV...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1503 ---
[GitHub] thrift issue #1505: Fixing java thrift compiler to generate constants in sta...
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1505 Please add the issue number to your commit message and PR title. See [here](https://thrift.apache.org/docs/HowToContribute) (point 7 under Github). ---
[jira] [Commented] (THRIFT-4513) thrift generated java code is not stable for constants
[
https://issues.apache.org/jira/browse/THRIFT-4513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16393330#comment-16393330
]
ASF GitHub Bot commented on THRIFT-4513:
GitHub user romanoid opened a pull request:
https://github.com/apache/thrift/pull/1505
Fixing java thrift compiler to generate constants in stable order.
For description of the issue fixed, see:
https://issues.apache.org/jira/browse/THRIFT-4513
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/romanoid/thrift fix-constant-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1505.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 #1505
commit 6a280b458f95a24d20bbc7798e145fe703ed23a0
Author: Roman Soroka
Date: 2018-03-08T23:45:22Z
Fixing java thrift compiler to generate constants in stable order.
See https://issues.apache.org/jira/browse/THRIFT-4513
> thrift generated java code is not stable for constants
> --
>
> Key: THRIFT-4513
> URL: https://issues.apache.org/jira/browse/THRIFT-4513
> Project: Thrift
> Issue Type: Improvement
> Components: Java - Compiler
>Reporter: Roman Soroka
>Priority: Major
>
> In certain cases, output generated with java compiler is unstable, this
> prevents correct incremental generation, artifact caching, etc.
> This is especially crucible if the goal is reproducible builds: same results
> for the same source inputs.
> Here's example:
> Let's say we have thrift definition:
> const map CONSTANT_MAP =
> { 'key1' : \\{"enum1" : Enum1.ENUM1_VALUE1, "enum2" : Enum2.ENUM2_VALUE}
> ,
> 'key2' : \{"enum1" : Enum1.ENUM1_VALUE2, "enum2" : Enum2.ENUM2_VALUE},
> 'key3' : \{"enum1" : Enum1.ENUM1_VALUE3, "enum2" : Enum2.ENUM2_VALUE},
>
> }
> Compiler generates code like:
> public static final java.util.Map CONSTANT_MAP =
> new java.util.HashMap();
> static {
> Enum1Enum2 tmp199 = new Enum1Enum2();
> tmp199.setEnum1(com.java.package.Enum1.ENUM1_VALUE2);
> tmp199.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key2", tmp199);
> Enum1Enum2 tmp200 = new Enum1Enum2();
> tmp200.setEnum1(com.java.package.Enum1.ENUM1_VALUE1);
> tmp200.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key1", tmp200);
> Enum1Enum2 tmp201 = new Enum1Enum2();
> tmp201.setEnum1(com.java.package.Enum1.ENUM1_VALUE3);
> tmp201.setEnum2(com.java.package.Enum2.ENUM2_VALUE);
> CONSTANT_MAP.put("key3", tmp201);
> Possible reorderings:
> 1. Values are added in different order from run to run.
> 2. Set methods in value construction may be called in different order.
> Note: this results in equivalent java maps, so there is no runtime issues.
> Proposed remedy:
> When building construction of Constants from JSON, java code generator
> should use by-key ordering.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift pull request #1505: Fixing java thrift compiler to generate constants...
GitHub user romanoid opened a pull request: https://github.com/apache/thrift/pull/1505 Fixing java thrift compiler to generate constants in stable order. For description of the issue fixed, see: https://issues.apache.org/jira/browse/THRIFT-4513 You can merge this pull request into a Git repository by running: $ git pull https://github.com/romanoid/thrift fix-constant-order Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1505.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 #1505 commit 6a280b458f95a24d20bbc7798e145fe703ed23a0 Author: Roman Soroka Date: 2018-03-08T23:45:22Z Fixing java thrift compiler to generate constants in stable order. See https://issues.apache.org/jira/browse/THRIFT-4513 ---
