-
Notifications
You must be signed in to change notification settings - Fork 855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snapshot of APIs as of NetBeans 22 #7419
Conversation
Changes look sane at a glance. However, the reason why the paperwork test is complaining needs checking. Something to do with generic signatures in commons-io at a guess, but not sure why it's complaining about that now.
This needs to be generated in draft around rc1 to be reviewed. It's not possible to handle inadvertent API changes once the vote has completed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ebarboni what JDK was the PR generated with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit history of this file: https://github.com/apache/netbeans/commits/master/platform/o.apache.commons.commons_io/nbproject/org-apache-commons-commons_io.sig -> means the last sig was generated in the last API snapshot PR #6960, hard to say what JDK was in use, it could still be caused by the 11->17 jump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked and this was generated with 17. It doesn't even work with 11 for some reason. (edit: JDK 21 produces the same sig file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we wrap version 2.16.0 right now:
27875A7935F1DDCC13267EB6FAE1F719E0409572 commons-io:commons-io:2.16.0 |
lets look at the error:
Class org.apache.commons.io.build.AbstractOriginSupplier
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setWriter(java.io.Writer)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setURI(java.net.URI)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setReader(java.io.Reader)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setPath(java.lang.String)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setOutputStream(java.io.OutputStream)
"E3.1 - Changing method signature and/or return type" : method protected {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setOrigin(org.apache.commons.io.build.AbstractOrigin)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setInputStream(java.io.InputStream)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setFile(java.io.File)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setCharSequence(java.lang.CharSequence)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractOriginSupplier.setByteArray(byte[])
Class org.apache.commons.io.build.AbstractStreamBuilder
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setOpenOptions(java.nio.file.OpenOption[])
"E3.1 - Changing method signature and/or return type" : method protected {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setCharsetDefault(java.nio.charset.Charset)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setCharset(java.lang.String)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setBufferSizeMax(int)
"E3.1 - Changing method signature and/or return type" : method protected {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setBufferSizeDefault(int)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setBufferSizeChecker(java.util.function.IntUnaryOperator)
"E3.1 - Changing method signature and/or return type" : method public {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractStreamBuilder.setBufferSize(int)
Class org.apache.commons.io.build.AbstractSupplier
"E3.1 - Changing method signature and/or return type" : method protected {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractSupplier.asThis()
in sig file:
CLSS public abstract org.apache.commons.io.build.AbstractOriginSupplier<%0 extends java.lang.Object, %1 extends org.apache.commons.io.build.AbstractOriginSupplier<{org.apache.commons.io.build.AbstractOriginSupplier%0},{org.apache.commons.io.build.AbstractOriginSupplier%1}>>
cons public init()
meth protected boolean hasOrigin()
meth protected org.apache.commons.io.build.AbstractOrigin<?,?> checkOrigin()
meth protected org.apache.commons.io.build.AbstractOrigin<?,?> getOrigin()
meth protected static org.apache.commons.io.build.AbstractOrigin$ByteArrayOrigin newByteArrayOrigin(byte[])
meth protected static org.apache.commons.io.build.AbstractOrigin$CharSequenceOrigin newCharSequenceOrigin(java.lang.CharSequence)
meth protected static org.apache.commons.io.build.AbstractOrigin$FileOrigin newFileOrigin(java.io.File)
meth protected static org.apache.commons.io.build.AbstractOrigin$FileOrigin newFileOrigin(java.lang.String)
meth protected static org.apache.commons.io.build.AbstractOrigin$InputStreamOrigin newInputStreamOrigin(java.io.InputStream)
meth protected static org.apache.commons.io.build.AbstractOrigin$OutputStreamOrigin newOutputStreamOrigin(java.io.OutputStream)
meth protected static org.apache.commons.io.build.AbstractOrigin$PathOrigin newPathOrigin(java.lang.String)
meth protected static org.apache.commons.io.build.AbstractOrigin$PathOrigin newPathOrigin(java.nio.file.Path)
meth protected static org.apache.commons.io.build.AbstractOrigin$ReaderOrigin newReaderOrigin(java.io.Reader)
meth protected static org.apache.commons.io.build.AbstractOrigin$URIOrigin newURIOrigin(java.net.URI)
meth protected static org.apache.commons.io.build.AbstractOrigin$WriterOrigin newWriterOrigin(java.io.Writer)
meth protected {org.apache.commons.io.build.AbstractOriginSupplier%1} setOrigin(org.apache.commons.io.build.AbstractOrigin<?,?>)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setByteArray(byte[])
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setCharSequence(java.lang.CharSequence)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setFile(java.io.File)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setFile(java.lang.String)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setInputStream(java.io.InputStream)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setOutputStream(java.io.OutputStream)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setPath(java.lang.String)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setPath(java.nio.file.Path)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setReader(java.io.Reader)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setURI(java.net.URI)
meth public {org.apache.commons.io.build.AbstractOriginSupplier%1} setWriter(java.io.Writer)
supr org.apache.commons.io.build.AbstractSupplier<{org.apache.commons.io.build.AbstractOriginSupplier%0},{org.apache.commons.io.build.AbstractOriginSupplier%1}>
hfds origin
CLSS public abstract org.apache.commons.io.build.AbstractStreamBuilder<%0 extends java.lang.Object, %1 extends org.apache.commons.io.build.AbstractStreamBuilder<{org.apache.commons.io.build.AbstractStreamBuilder%0},{org.apache.commons.io.build.AbstractStreamBuilder%1}>>
cons public init()
meth protected int getBufferSize()
meth protected int getBufferSizeDefault()
meth protected java.io.InputStream getInputStream() throws java.io.IOException
meth protected java.io.OutputStream getOutputStream() throws java.io.IOException
meth protected java.io.Reader getReader() throws java.io.IOException
meth protected java.io.Writer getWriter() throws java.io.IOException
meth protected java.lang.CharSequence getCharSequence() throws java.io.IOException
meth protected java.nio.charset.Charset getCharsetDefault()
meth protected java.nio.file.OpenOption[] getOpenOptions()
meth protected java.nio.file.Path getPath()
meth protected {org.apache.commons.io.build.AbstractStreamBuilder%1} setBufferSizeDefault(int)
meth protected {org.apache.commons.io.build.AbstractStreamBuilder%1} setCharsetDefault(java.nio.charset.Charset)
meth public !varargs {org.apache.commons.io.build.AbstractStreamBuilder%1} setOpenOptions(java.nio.file.OpenOption[])
meth public java.nio.charset.Charset getCharset()
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setBufferSize(int)
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setBufferSize(java.lang.Integer)
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setBufferSizeChecker(java.util.function.IntUnaryOperator)
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setBufferSizeMax(int)
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setCharset(java.lang.String)
meth public {org.apache.commons.io.build.AbstractStreamBuilder%1} setCharset(java.nio.charset.Charset)
supr org.apache.commons.io.build.AbstractOriginSupplier<{org.apache.commons.io.build.AbstractStreamBuilder%0},{org.apache.commons.io.build.AbstractStreamBuilder%1}>
hfds DEFAULT_MAX_VALUE,DEFAULT_OPEN_OPTIONS,bufferSize,bufferSizeChecker,bufferSizeDefault,bufferSizeMax,charset,charsetDefault,defaultSizeChecker,openOptions
CLSS public abstract org.apache.commons.io.build.AbstractSupplier<%0 extends java.lang.Object, %1 extends org.apache.commons.io.build.AbstractSupplier<{org.apache.commons.io.build.AbstractSupplier%0},{org.apache.commons.io.build.AbstractSupplier%1}>>
cons public init()
intf org.apache.commons.io.function.IOSupplier<{org.apache.commons.io.build.AbstractSupplier%0}>
meth protected {org.apache.commons.io.build.AbstractSupplier%1} asThis()
supr java.lang.Object
looks like the generics are indeed the issue:
- the sig file looks correct to me since it matches the code
- the error looks wrong?
Class org.apache.commons.io.build.AbstractSupplier
"E3.1 - Changing method signature and/or return type" : method protected {org.apache.commons.io.build.AbstractStreamBuilder%1} org.apache.commons.io.build.AbstractSupplier.asThis()
it should be {org.apache.commons.io.build.AbstractSupplier%1}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generated with jdk 17 as ant build fail on jdk 11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me this also looks like an issue in sigtest. I ran commons-io 2.15.1 vs. 2.16.0 through japi-compliance-checker
and while there are source incompatible changes reported for commons-io
, binary compatibility is ok.
If I remember correctly we have seen other instances where sigtest reported problems and had to be deactivated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the last thing I wanted to check but ran out of time was the jar itself, and yes it matches both the sig file and the source of 2.16.0. So the change in this PR looks correct to me.
#7117 which updated to jakarta sigtest 2.2 did already disable sigtest on some modules, we probably can do this here too. Running sigtests over third party libs has probably only informational value anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw sigtest-maven-plugin had a release in the meantime (2.3). I can't find an issue connected to generics but it might be worth checking if it gives better results.
edit: checking this right now
lib upgrade PR: #7426
CI run with this PR + lib upgrade: https://github.com/mbien/netbeans/actions/runs/9331767904
edit2: this won't solve this issue: eclipse-ee4j/jakartaee-tck-tools@sigtest-2.2...sigtest-2.3
The PR is updated thanks to @mbien . Removing the sig for common io removing the generation/ rebased on master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine i think.
There would be a few signatures which could be left out, the new -IgnoreJDKClass
might help there in some cases since it filters out java.*
and javax.*
.
this should also make the following step from the script no longer necessary (assuming it works as intended):
# remove getPeer() calls
find . -name "*.sig" -exec sed -i '/java.awt.peer.ComponentPeer/{N;d;}' {} \;
(and it would remove all the @Deprecated
, Set.*
, Map.*
sigs too)
_serializedATN
should be also filtered out in my opinion since it dumps a binary blob (from generated antlr code?) into the sig file. Binary blobs should not be in repos since they are not reviewable (see xz).
The question is should we do it now or the next time?
maybe next time ? |
sure! |
merging sorry for the delay |
Snapshot of APIs as of NetBeans 22. For review purposes only at this stage. Not to be merged as / until release vote completes and updated if necessary.
Built in the usual way ...
Filtered diff created using -
filtered-apis-nb220.txt