commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [VOTE] Release Apache Commons Lang 3.6 based on RC3
Date Fri, 09 Jun 2017 09:25:50 GMT
If the intention of the change was to ensure that the values were
correct, then there are two alternative option:

1) add a test to check the values
2) add assertions in the class to check the values



On 9 June 2017 at 08:07, Benedikt Ritter <britter@apache.org> wrote:
>
>> Am 09.06.2017 um 08:44 schrieb Duncan Jones <duncan@wortharead.com>:
>>
>> On Fri, 9 Jun 2017 at 02:35, Gary Gregory <garydgregory@gmail.com> wrote:
>>
>>> On Thu, Jun 8, 2017 at 6:29 PM, Simon Spero <sesuncedu@gmail.com> wrote:
>>>
>>>> There is a one other compatibility issue, which can be seen in the
>>> attached
>>>> code:
>>>>
>>>> import java.nio.charset.StandardCharsets;
>>>>
>>>> public class Weasel {
>>>>
>>>>    private static final String US_ASCII = "US-ASCII";
>>>>    private static final String UTF_8 = "UTF-8";
>>>>    private static final String STANDARD_US_ASCII =
>>>> StandardCharsets.US_ASCII.name();
>>>>    private static final String STANDARD_UTF_8 =
>>>> StandardCharsets.UTF_8.name
>>>> ();
>>>>
>>>>    public static void main(String[] args) {
>>>>
>>>>        switch (args[0]) {
>>>>            case US_ASCII:
>>>>                System.out.println("USA! USA!");
>>>>                break;
>>>>            case UTF_8:
>>>>                System.out.println("Emoji Lovin' Hippies!");
>>>>                break;
>>>>            default:
>>>>                System.out.println("Weirdo.");
>>>>        }
>>>>    }
>>>> }
>>>>
>>>>
>>>> This code compiles.
>>>> However, if the case labels are changed to STANDARD_US_ASCII and
>>>> STANDARD_UTF_8, the compilation will fail, because the  case labels are
>>> no
>>>> longer constant expressions.
>>>> In the actual code, this means that code compiled against an older
>>> version
>>>> of the library would work against the new code (because the old strings
>>> had
>>>> already been inlined), but could not be re-compiled.
>>>>
>>>
>>> Thank you for doing this research and POC :-)
>>>
>>> But Ugh! :-( We shot ourselves in the foot here.
>>
>>
>> D'oh. Sorry about that guys :-(
>
> No problem!
>
>>
>>
>>>
>>> Benedikt, how do you feel about a fix and a new RC?
>>>
>>> Gary
>>>
>>>
>>>>
>>>> I don't know why anyone would be doing this...
>>>>
>>>> (CLIRR pre-dates string switches)
>>>>
>>>> Simon
>>>>
>>>> On Thu, Jun 8, 2017 at 5:10 PM, sebb <sebbaz@gmail.com> wrote:
>>>>
>>>>> On 8 June 2017 at 18:09, Gary Gregory <garydgregory@gmail.com>
wrote:
>>>>>> On Thu, Jun 8, 2017 at 9:57 AM, sebb <sebbaz@gmail.com> wrote:
>>>>>>
>>>>>>> On 8 June 2017 at 17:19, Gary Gregory <garydgregory@gmail.com>
>>> wrote:
>>>>>>>> On Thu, Jun 8, 2017 at 5:38 AM, Simon Spero <sesuncedu@gmail.com>
>>>>> wrote:
>>>>>>>>
>>>>>>>>> [A Note, not a vote :) ]
>>>>>>>>>
>>>>>>>>> 1. Clirr is generally considered obsolete, as it hadn't
been
>>> worked
>>>>> on
>>>>>>> for
>>>>>>>>> about ten years.   japicmp is a good replacement, especially
for
>>>>> report
>>>>>>>>> generation, and is used in other commons projects.
>>>>>>>>>
>>>>>>>>
>>>>>>>> IIRC, we've started using japicm here and there. Each component
>>> can
>>>>>>> decide.
>>>>>>>> But yes, Clirr looks pretty dead.
>>>>>>>>
>>>>>>>>
>>>>>>>>> 2. Are the "changes" to the values in CharEncoding really
>>>>> necessary[1]
>>>>>>> (The
>>>>>>>>> deprecation surely is). Technically this is a potentially
>>> breaking
>>>>>>> binary
>>>>>>>>> incompatible change, as constant strings and primitives
are
>>> inlined
>>>>> at
>>>>>>>>> compile time. [2]
>>>>>>>>> In this particular case, the results of load-time evaluation
of
>>> the
>>>>> new
>>>>>>>>> initialization expressions  are identical to the old
constants,
>>> so
>>>>> it's
>>>>>>>>> behaviourally compatible; however, since this is the
case, and
>>>> since
>>>>>>> it's
>>>>>>>>> all deprecated anyway, why not leave the old values in-place?
>>>>>>>>>
>>>>>>>>
>>>>>>>> The changes are not "necessary" that for sure and we do get
Clirr
>>>>>>> warnings:
>>>>>>>>
>>>>>>>> Value of field ISO_8859_1 is no longer a compile-time constant
>>>>>>>> Value of field US_ASCII is no longer a compile-time constant
>>>>>>>> Value of field UTF_16 is no longer a compile-time constant
>>>>>>>> Value of field UTF_16BE is no longer a compile-time constant
>>>>>>>> Value of field UTF_16LE is no longer a compile-time constant
>>>>>>>> Value of field UTF_8 is no longer a compile-time constant
>>>>>>>>
>>>>>>>> It's source compatible. What is the issue at runtime that
could
>>> hurt
>>>>>>> users?
>>>>>>>
>>>>>>> As the OP wrote, constants are inlined by the compiler.
>>>>>>> So unless all code is recompiled, if it referenced the constant
it
>>> may
>>>>>>> have a stale value.
>>>>>>> That is not binary compatible.
>>>>>>>
>>>>>>
>>>>>> But in this case the actual values are the same are they not? So
what
>>>> is
>>>>>> the difference? Would this only be a problem if we changed the string
>>>>>> values?
>>>>>
>>>>> AFAIK the compiler only inlines compile-time constants.
>>>>> So going forward, the values won't be inlined.
>>>>> I assume the behaviour will be unaffected since the runtime value
>>>>> should be the same as the constant.
>>>>>
>>>>> The release notes really ought to explain why the Clirr report items
>>>>> aren't a problem.
>>>>>
>>>>>> Which we can't since these are defined in the JRE. And the JRE is
>>>>>> unlikely to change those.
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> 3. JDK9 adds some extra parameters to the Deprecated
annotation
>>>> (most
>>>>>>>>> notably  forRemoval=true, which is used to indicate that
the
>>>>> annotated
>>>>>>> item
>>>>>>>>> is really really deprecated.)   It's not needed in this
case, but
>>>> is
>>>>>>> worth
>>>>>>>>> thinking about  when jdk9 is eventually released (latest
schedule
>>>>>>> change :
>>>>>>>>> from 7/27/2017 to 9/21/2017).
>>>>>>>>>
>>>>>>>>
>>>>>>>> I do not think we plan on making Java 9 a requirement for
any
>>>> current
>>>>>>>> project.
>>>>>>>>
>>>>>>>> Gary
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Simon
>>>>>>>>>
>>>>>>>>> [1]  https://github.com/apache/commons-lang/commit/7c19a1ff4c217
>>>>>>>>> f03c0be62baf1169d689f566825#diff-820a48456e11e85bf6cf5356c1aed4
>>>> baR48
>>>>>>>>>
>>>>>>>>> [2] https://docs.oracle.com/javase/specs/jls/se8/html/jls-
>>>>>>>>> 13.html#jls-13.4.9
>>>>>>>>>
>>>>>>>>> On Jun 8, 2017 4:48 AM, "Benedikt Ritter" <britter@apache.org>
>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> we have fixed quite a few bugs and added some nice
new features
>>>>> since
>>>>>>>>>> Commons Lang 3.5 was released, so I would like to
release
>>> Commons
>>>>> Lang
>>>>>>>>> 3.6
>>>>>>>>>> based on RC3.
>>>>>>>>>> The following issues have been fixed since RC2:
>>>>>>>>>>
>>>>>>>>>> - Site build now works from source distribution
>>>>>>>>>> - IBM JDK test failures have been fixed
>>>>>>>>>> - Automatic-Module-Name MANIFEST entry has been added
for Java
>>> 9
>>>>>>>>>> compatibility
>>>>>>>>>>
>>>>>>>>>> Commons Lang 3.6 R3 is available for review here:
>>>>>>>>>> https://dist.apache.org/repos/dist/dev/commons/lang
(svn
>>>> revision
>>>>>>>>> 19928)
>>>>>>>>>>
>>>>>>>>>> The tag is here:
>>>>>>>>>> https://git-wip-us.apache.org/repos/asf?p=commons-lang.git;
>>>>> a=tag;h=
>>>>>>>>>> e454e79463ffbbd9114db43019dd211debbcc105
>>>>>>>>>>
>>>>>>>>>> Commit ID the tag points at:
>>>>>>>>>> 360198dfb6a2d68f1702f616dfacee34ae0541bb
>>>>>>>>>>
>>>>>>>>>> Maven Artifacts:
>>>>>>>>>> https://repository.apache.org/content/repositories/
>>>>>>>>> orgapachecommons-1250
>>>>>>>>>>
>>>>>>>>>> These are the Maven artifacts and their hashes:
>>>>>>>>>>
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>> javadoc.jar
>>>>>>>>>> (SHA1: c8adadb6c0b56c73f2cc2b4c77a09bfe34ec3a66)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>>>> sources.jar.asc
>>>>>>>>>> (SHA1: 46347c179ca9246d146d653bdc7363bda6f17d44)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6.pom.asc
>>>>>>>>>> (SHA1: 1309d4f3dd41a01ff9dd1f3c1a6eee10dad1ef77)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6.pom
>>>>>>>>>> (SHA1: 0fb4499188c94c63b3cba44f12481e193708c4a8)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6.jar.asc
>>>>>>>>>> (SHA1: e67e7d44751f1e346c2fda496193cbe251cfe098)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>>>> javadoc.jar.asc
>>>>>>>>>> (SHA1: 6b19fa12d319ced69c0f8a27001569514711f9dc)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>> sources.jar
>>>>>>>>>> (SHA1: f89c1df082d6f67cb7c956715c56d7e7a0508d0a)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6.jar
>>>>>>>>>> (SHA1: e58ba08b01d37a023746f0987dcd910036a63021)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>> tests.jar.asc
>>>>>>>>>> (SHA1: af050e8c29a801a5d6783268c56230b814f56240)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>>>>>>> test-sources.jar.asc
>>>>>>>>>> (SHA1: 71e4c11efb9e3b2eff402ba4648d21822fb8da4a)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>>>>> test-sources.jar
>>>>>>>>>> (SHA1: 04a0fc8293d4ed64a54dcc9ba5f996776a4657ea)
>>>>>>>>>> /org/apache/commons/commons-lang3/3.6/commons-lang3-3.6-
>>>> tests.jar
>>>>>>>>>> (SHA1: 87993a16c14a251062e3fe860fa53b5ef5304a0f)
>>>>>>>>>>
>>>>>>>>>> I have tested this with JDK 7, JDK 8 and JDK 9 EA
b172 using
>>>> Maven
>>>>>>> 3.5.0.
>>>>>>>>>>
>>>>>>>>>> Details of changes since 3.5 are in the release notes:
>>>>>>>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/lang/RELEASE-
>>>>>>> NOTES.txt
>>>>>>>>>>   http://home.apache.org/~britter/commons/lang/LANG_3_6_
>>>>>>>>>> RC3/changes-report.html
>>>>>>>>>>
>>>>>>>>>> Site:
>>>>>>>>>>     http://home.apache.org/~britter/commons/lang/LANG_3_6_RC3
>>>>>>>>>> (note some *relative* links are broken and the 3.6
directories
>>>> are
>>>>>>>>>> not yet created - these will be OK once the site
is deployed)
>>>>>>>>>>
>>>>>>>>>> Clirr Report (compared to 3.5):
>>>>>>>>>>   http://home.apache.org/~britter/commons/lang/LANG_3_6_
>>>>>>>>>> RC3/clirr-report.html
>>>>>>>>>>
>>>>>>>>>> RAT Report:
>>>>>>>>>>       http://home.apache.org/~britter/commons/lang/LANG_3_6_
>>>>>>>>>> RC3/rat-report.html
>>>>>>>>>>
>>>>>>>>>> KEYS:
>>>>>>>>>> https://www.apache.org/dist/commons/KEYS
>>>>>>>>>>
>>>>>>>>>> Please review the release candidate and vote.
>>>>>>>>>> This vote will close no sooner that 72 hours from
now,
>>>>>>>>>> i.e. sometime after 11:00 CEST (UTC+2) 11-June 2017
>>>>>>>>>>
>>>>>>>>>> [ ] +1 Release these artifacts
>>>>>>>>>> [ ] +0 OK, but...
>>>>>>>>>> [ ] -0 OK, but really should fix...
>>>>>>>>>> [ ] -1 I oppose this release because...
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> Benedikt
>>>>>>>>>> ------------------------------------------------------------
>>>>> ---------
>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>>>
>>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>>
>>>>
>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message