Skip to content
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

False alarm: METHOD_NO_LONGER_VARARGS #393

Closed
He-Pin opened this issue Apr 21, 2024 · 5 comments
Closed

False alarm: METHOD_NO_LONGER_VARARGS #393

He-Pin opened this issue Apr 21, 2024 · 5 comments

Comments

@He-Pin
Copy link

He-Pin commented Apr 21, 2024

version:
0.20.0

The method is there, so this is a false alarm.

Reproduce progress:

  1. check out netty/netty 4.1 branch
  2. change the japicmp version to 0.20.0
  3. run japicmp check

Thanks for this great plugin, I find this when preparing a pr in: netty/netty#13989

@He-Pin He-Pin changed the title METHOD_NO_LONGER_VARARGS False alarm: METHOD_NO_LONGER_VARARGS Apr 21, 2024
@siom79
Copy link
Owner

siom79 commented Apr 21, 2024

Thanks for reporting.

The issues looks a bit weird. When debugging, it shows that in this line in the method extractVarargsModifier() the javassist library returns for behavior.getModifiers() the value 9 for the old version and 137 for the new version. And 137 & 128 means, that VARARGS is set in the new version but not in the old one. So from the point of japicmp it seems to be correct.

But if you compare both class files Unpooled.class, they are identical on each bit. So why does javassist read the value 9 in the old version and 137 in the new version?

@He-Pin
Copy link
Author

He-Pin commented Apr 21, 2024

Thanks for reporting.

The issues looks a bit weird. When debugging, it shows that in this line in the method extractVarargsModifier() the javassist library returns for behavior.getModifiers() the value 9 for the old version and 137 for the new version. And 137 & 128 means, that VARARGS is set in the new version but not in the old one. So from the point of japicmp it seems to be correct.

But if you compare both class files Unpooled.class, they are identical on each bit. So why does javassist read the value 9 in the old version and 137 in the new version?

Thanks for quick validation this, this maybe bump javasist version in later release ?I'm not an expert on this. But I checked 0.15.7, it works.

siom79 added a commit that referenced this issue Apr 21, 2024
siom79 added a commit that referenced this issue Apr 21, 2024
…..) parameter, methods are matched correctly
@siom79
Copy link
Owner

siom79 commented Apr 21, 2024

1,5 hours later: There was an issue with matching the corresponding methods in case one method had a byte array as parameter (byte[]) and the other one an array of byte arrays with varargs (byte[]...). The algorithm did falsely match the wrong arguments (treating byte[]... the same as byte[]). Therefore, the code comparing the varargs bit in the modifiers did compare two different methods. :-)

I have fixed this with the last commit.

@siom79 siom79 closed this as completed Apr 21, 2024
siom79 added a commit that referenced this issue Apr 21, 2024
@He-Pin
Copy link
Author

He-Pin commented Apr 22, 2024

Thanks, do you plan a newer release soon, I would like to submit a pr in Netty for this.

@siom79
Copy link
Owner

siom79 commented Apr 22, 2024

Released with 0.21.0.

normanmaurer added a commit to netty/netty that referenced this issue Apr 23, 2024
Motivation:

Bump japicmp to 0.21.0

Modification:

There was an issue in the newer version, but after reported now it's
fixed.
siom79/japicmp#393
siom79/japicmp#394

Result:

Bump japicmp to 0.21.0

---------

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
normanmaurer added a commit to netty/netty that referenced this issue Apr 23, 2024
Motivation:

Bump japicmp to 0.21.0

Modification:

There was an issue in the newer version, but after reported now it's
fixed.
siom79/japicmp#393
siom79/japicmp#394

Result:

Bump japicmp to 0.21.0

---------

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
He-Pin added a commit to He-Pin/netty that referenced this issue Apr 23, 2024
Motivation:

Bump japicmp to 0.21.0

Modification:

There was an issue in the newer version, but after reported now it's
fixed.
siom79/japicmp#393
siom79/japicmp#394

Result:

Bump japicmp to 0.21.0

---------

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
normanmaurer added a commit to netty/netty that referenced this issue Apr 23, 2024
Motivation:

Bump japicmp to 0.21.0

Modification:

There was an issue in the newer version, but after reported now it's
fixed.
siom79/japicmp#393
siom79/japicmp#394

Result:

Bump japicmp to 0.21.0


Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants