Building Magic from source: Question about the Errorprone IntellIJ Plugin.
Remodactyl opened this issue ยท 25 comments
I've seen a group of people attempting to build Magic from source using the project cloned from Github, and using the IntellIJ IDEA I see most of them running into the same problem.
This flag is part of the Errorprone plugin which presumably you use in developing Magic. And the definition thereof can be found in the Magic-parent Pom.xml file.
However, it seems the current version of this plugin, available on IntelIJ, does not resolve this 'Invalid Flag' issue. (even if the Java-compiler is, as per instruction, set to 'javac with error-prone').
My theory is currently that you use an Outdated version of this plugin in the pom.xml file, and that we can no longer find the correct version that recognizes these flags, but this is difficult to test as very little information seems to be openly available about the plugin.
My question is this:
If you update your version of the Errorprone plugin, can you still get it to work with your IDEA and Magic in IntellIJ? And if not, considering how many flags are used in the config to suppress warnings, is there still reason for keeping this plugin around?
This is a little weird since the pom file and Maven are supposed to determine what version of errorprone gets used, I am not sure how others are getting different versions. They are all still available since they come from Maven Central.
Anyway I updated errorprone to latest, removed that flag and fixed the errors that popped up. Hopefully that helps!
I see a commit has come in removing the top XepDisableAllChecks flag and updating the Errorprone version, now the warning simply happens for the next flag:
This likely means that for some reason on our own builds (mine and that of other developers who have tried building from source) the build is not properly running with errorprone, and the flags are not being defined by the plugin. For some reason, your setup seems to recognize the plugin, do you have any idea what changes you made back when you first started using it to allow for it to work?
I wasn't actually the one to add ErrorProne. As far as I know I didn't need to do anything to my build to make it work. The pom should be all that is needed to specify how the build should work.
Are you just doing like mvn clean package
to build and that's it?
Also worth noting that the build server has never had any issues and it's just using a stock JDK.
install should be fine, too- that's the same as package except it installs the results to your local maven repo.
What JDK are you using? I think I am using Oracle (21) in both places.
Nope, mvn clean package still gives the same flag errors, currently running JDK 21 (From Oracle)
Errorprone is super helpful. In fact that two things I had to fix when removing the "DisableAllChecks" flag were real issues that had gone unnoticed. So I'm not really in favor of removing it, I'd prefer to figure out what the actual problem is.
From how I understand it, Errorprone is essentially just a plugin that helps detect code smells, if you've never noticed this plugin affecting your work, it may be easier to remove it altogether?
I really don't know .... could be a platform difference I guess? I test on Ubuntu and MacOS ... maybe something Windows specific, assuming y'all are on Windows?
If you remove all of the -Xep compiler flags does it work?
I am also not opposed to figuring this out through a voicecall in the discord if that makes things easier, it's possible this is a difference in our IDEA configurations
I develop and build locally with intellij, but the build server is just straight JDK via Jenkins. I consider it a good secondary check to rule out anything IDE or Mac-specific.
I'm guessing this has something to do with the Windows JDK for whatever reason. I don't see anything via Google though.
I'd be ok with removing most of those flags, but definitely want to keep Xep:RemoveUnusedImports:ERROR
.. can you try with just that one? It makes sure the code stays clean, catches unused imports constantly.
It sounds somewhat like errorprone on Windows is just broken, though, and I'm not really sure what I can do about that.
Unfortunately it does not recognize the flag, looks like for the time being windows users will have to know to edit these out if they want to build source code, could be a good temporary solution to mention this in the magic documentation?
Beyond the flag we now just run into the expected:
'Caused by: org.eclipse.aether.resolution.DependencyResolutionException: The following artifacts could not be resolved: com.elmakers.mine.bukkit:EffectLib:jar:10.3 (absent): com.elmakers.mine.bukkit:EffectLib:jar:10.3 was not found in https://maven.elmakers.com/repository/ during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of elMakers has elapsed or updates are forced
at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies (DefaultRepositorySystem.java:365)'
Which I assume has to do with EffectLib being modified since this only started about a week ago.
Oops, yeah the EffectLib it just a new version I need to upload to Maven Central.
It should be there now.
As for errorprone, I don't know what else to look at there. @killme are you still around? (if so, hi, long time!) - do you still build Magic, and if so is it on Windows?
The OS is the only variable I can really think of at this point.
Interestingly, I've just tested errorprone on one of my own projects, I copied the maven configuration arguments that magic-parent uses and in my project errorprone DOES WORK!
So there must be some difference in the setup for the magic plugin.
I am going to keep looking!
Scratch that, I didn't use all the same flags, but it is catching code smells correctly so the plugin is at least partially functional, I'll be curious to see your friend attempt this on Windows in IntellIJ though!
Ok, the same flags are broken in that project for me too, false alarm. Could still be a Windows issue.
Well for now I have commented out the flags. I'd like to put them back eventually, but there are probably better ways (like the errorprone config) of enabling those checks. I have to admit I'm not super well versed in this plugin, but I do like the results.
Yea, it'd be a shame to throw it out, here's hoping your friend is more familiar with it, until then we'll have to watch our own unused import clauses xS
It could be the case that something funky is going on with the line
endings on windows
Oh that's a good point. One easy thing we could try then is putting the whole command line on one line in the pom.
@Remodactyl can you please edit the pom like this and try it?
<arg>-XDcompilePolicy=simple</arg>
<arg>
-Xplugin:ErrorProne -Xep:ProtectedMembersInFinalClass:ERROR -Xep:RemoveUnusedImports:ERROR -Xep:InlineMeSuggester:OFF -Xep:CatchAndPrintStackTrace:OFF
</arg>
Looks like that works! So the issue all this time was that the flags were not being seen as part of the same arg as Xplugin:Errorprone?
I had tried deleting the backslashes before but I hadn't moved the errorprone args to the same line yet, and this appears to have fixed it finally.
Looks like you have one more commit to make and people will have the errorprone plugin functional.
Thanks a ton!
This means you can probably put back the flag you deleted before when we were still testing it.
the '-XepDisableAllChecks', since that used to be there before.
After that I think this issue is resolved!
Thanks, I just pushed the change.
I was also thinking about putting that DisableAllChecks flag back in, but since the build is building ok with all checks enabled I think I'll just leave it.
It's not totally clear to me if the following flags are needed (maybe :ERROR is the default if you're not disabling checks up front?) but I think I am ok leaving this all as-is for now.