On Tue, Nov 28, 2023 at 10:01 AM Eirik Bjørsnøs <eir...@gmail.com> wrote:
> Hi, > > JDK-4512189 [1] raised the issue that ZipFile, ZipEntry, ZipInputStream > and ZipOutputStream all implement the non-public ZipConstants interface. > While the interface itself is non-public, the constants defined > unfortunately leak into the public API as constants of the mentioned > classes. This means they show up in Javadoc, and maybe worse in code > completions. > > JDK-4512189 was closed in 2001 because of concerns of binary compatibility: > > Unfortunately, whether or not a class implements an interface is part of >> the classes public API and having those classes *not* implement >> ZipConstants will break binary compatibility. Therefore, unfortunately, >> this bug is being closed as will not fix. > > > However, will this really break binary compatibility? Any reference to > ZipFile.CENSIG or any other constant (which are all of type long and int) > will be compiled into byte code in the referencing class, with or without a > copy of the constant in the constant pool of the referencing class. Thus, > any compiled reference should continue to work. > > There is of course a source compatibility issue, but these constants > should generally not be of interest to anyone outside the implementation, > and if anyone is accidentally using them, it's most probably a bug. In any > case, they would get warned when trying to recompile their sources. The > easy solution then will be to define their own constants or fix the bug. > > In light of this, I would like to revisit this issue, 22 years later: > > - Is my assessment that this change is actually not binary incompatible > sound, or did I miss something? > - Would it in any case make sense to mark ZipConstants as @Deprecated, > maybe for removal to alert people we want to remove the constants? > - Could we aim to make the mentioned classes *not* implement ZipConstants, > following the regular deprecation process, CSR and release note etc? > I agree, I always thought that this was a bizarre thing to expose in a public API so it being accidental does explain things. But maybe the appropriate fix is even simpler now in these modern times: one could remove the `implements` and add a `static import` of `ZipConstants.*`, which at approximately one or two lines is a much more minimal (and conflict-resistant, for those who care about backporting subsequent fixes of this class) change IMO. -- - DML • he/him
