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 <brit...@apache.org> wrote: > >> Am 09.06.2017 um 08:44 schrieb Duncan Jones <dun...@wortharead.com>: >> >> On Fri, 9 Jun 2017 at 02:35, Gary Gregory <garydgreg...@gmail.com> wrote: >> >>> On Thu, Jun 8, 2017 at 6:29 PM, Simon Spero <sesunc...@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 <seb...@gmail.com> wrote: >>>> >>>>> On 8 June 2017 at 18:09, Gary Gregory <garydgreg...@gmail.com> wrote: >>>>>> On Thu, Jun 8, 2017 at 9:57 AM, sebb <seb...@gmail.com> wrote: >>>>>> >>>>>>> On 8 June 2017 at 17:19, Gary Gregory <garydgreg...@gmail.com> >>> wrote: >>>>>>>> On Thu, Jun 8, 2017 at 5:38 AM, Simon Spero <sesunc...@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" <brit...@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-unsubscr...@commons.apache.org >>>>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> >>> --------------------------------------------------------------------- >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>> >>>>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>> >>>>> >>>> >>> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org