SpigotMC / BungeeCord

BungeeCord, the 6th in a generation of server portal suites. Efficiently proxies and maintains connections and transport between multiple Minecraft servers.
https://www.spigotmc.org/go/bungeecord
Other
1.53k stars 1.09k forks source link

Bungeecode source not delomboked #3657

Closed lynxplay closed 2 months ago

lynxplay commented 2 months ago

Bungeecord version

N/A

Server version

N/A

Client version

N/A

Bungeecord plugins

N/A

The bug

BungeeCord deploys its api sources to the sonatype snapshot repository. This is great as it enables IDEs to skip the decomplication process when inspecting bungeecord-api types.

Unfortunately however, bungeecord makes use of lombok, meaning that the source jars provided do not match up with the actual generated bytecode found in the main artefact as the sources do not seem to be based on a delombok output. This can be validated by

1) Download latest bungeecord-api sources wget https://oss.sonatype.org/service/local/repositories/releases/content/net/md-5/bungeecord-api/1.20-R0.2/bungeecord-api-1.20-R0.2-sources.jar -O bungee-sources.jar 2) Inspect a file that makes use of lombok via unzip -c bungee-sources.jar net/md_5/bungee/api/event/PostLoginEvent.java 3) Observe the following output: https://pastebin.com/ySN59ZPV

Log output (links)

N/A

Checking

Janmm14 commented 2 months ago

I do think this is intended behaviour. The delomboked source would have different line numbers compared to the compiled debug information. Providing delomboked source would be worse here.

lynxplay commented 2 months ago

Have not looked into it enough, if that is the case the project should stop publishing the sources all together imo. The current state of sources is non functional too given that not only are line numbers off but also full methods are missing.

I would not call this "intented behaviour". Intended behaviour would be uploading delomboked sources if they are usuable (I have not used the maven delombok plugin), or to stop publishing sources all together if the delomboked sources are indeed not usuable as you suggest.

md-5 commented 2 months ago

Pretty sure, as per Jan's comment, Lombok doesn't alter line numbers — last I checked they matched up perfectly correctly. I think also the source is the source — it's what is used to compile. Seems to be a very strong opinion based on one IDE about what the "source" should be.

Have not looked into it enough, if that is the case the project should stop publishing the sources all together imo. The current state of sources is non functional too given that not only are line numbers off but also full methods are missing.

I would not call this "intented behaviour". Intended behaviour would be uploading delomboked sources if they are usuable (I have not used the maven delombok plugin), or to stop publishing sources all together if the delomboked sources are indeed not usuable as you suggest.

lynxplay commented 2 months ago

Given you were the one that told me to report it I'll close this then lmao.

lynxplay commented 2 months ago

As a follow up, this would be the difference on the intellij side of things when publishing delombok'ed sources.

Current

image

Delombok'ed

image

So yea, I guess this is IDE specific to IntelliJ (and obviously does not apply to the IDEs a significant portion of the developer base uses /s)

md-5 commented 2 months ago

Given you were the one that told me to report it I'll close this then lmao.

Without a ticket there would be no record of any discussion or the helpful input of Jan.

All I'm saying is that I don't think it's clearly wrong to be providing the source as-is.

lynxplay commented 2 months ago

Yea I guess in the ends it's a philosophical question on what you consider the source of your program when compiler hacks like Lombok are involved.

It's annoying for intellij users as pointed out above, but I digress. Not worth any more comments to the issues Lombok brings with it, especially when used in the environment of an API.

md-5 commented 2 months ago

Would you expect the source jar to contain the methods generated by 'record'? Arguably the same situation.

lynxplay commented 2 months ago

I am unsure if I'd compare a proper language feature that is understood by the java compiler to a third party compiler hack, but, again, I digress. This already took up more of my email inbox than I wanted it to.

At least it was brought up and, as you said, we now have it nicely documented here with your and Janm's input! Thank you for the quick responses.

Janmm14 commented 2 months ago

Here is a proposal to make the lombok-intellij-plugin shut up the warning: https://github.com/mplushnikov/lombok-intellij-plugin/issues/438 The plugin is an official ij plugin since some time, maybe the issue needs to be transferred to jetbrains issue tracker for extendend visibility.