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

add javac plugin #22

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

add javac plugin #22

wants to merge 6 commits into from

Conversation

rhysdh540
Copy link

builds using java 8 since that's when javac plugins were introduced

I bundle it into the -all jar for easy depending (annotationProcessor("xyz.wagyourtail...:all"))

and all the test has to do is compile and run on java 8

@CLAassistant
Copy link

CLAassistant commented Nov 22, 2024

CLA assistant check
All committers have signed the CLA.

Comment on lines 62 to 97
for(String arg : args) {
if(arg.contains("=")) {
String[] split = arg.split("=", 2);
switch(split[0]) {
case "api":
if(flags.api == null) {
flags.api = new ArrayList<>();
}
for(String s : split[1].split(p)) {
flags.api.add(new File(s));
}
break;
case "logLevel":
case "log":
flags.logLevel = Logger.Level.valueOf(split[1].toUpperCase(Locale.ROOT));
break;
case "skipStubs":
for(String s : split[1].split(",")) {
flags.debugSkipStubs.add(Integer.parseInt(s));
}
break;
case "target":
flags.classVersion = Utils.majorVersionToClassVersion(Integer.parseInt(split[1]));
break;
case "classpath":
case "cp":
for(String s : split[1].split(p)) {
try {
classpathURLs.add(new File(s).toURI().toURL());
} catch (Throwable t1) {
Utils.sneakyThrow(t1);
}
}
break;
}
}
Copy link
Contributor

@wagyourtail wagyourtail Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have to have it's own seperate args. also the args should at least match the names of the ones in https://github.com/unimined/JvmDowngrader/blob/main/src/main/java/xyz/wagyourtail/jvmdg/cli/Main.java and maybe share some of the logic.

also, does it really make sense for the classpath for jvmdg to be different than the classpath provided to javac?

also, also, you can pass VM properties into javac somehow, which can also be used to set the jvmdg flags, so is this nececairy at all? https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#option-J

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could make it delegate to Main, that would just require a little moving of logic in Main around so I can get the parsed Flags. I'm not a particularly big fan of using -J though, I think having the arguments as the arguments to the plugin would work better.

@@ -17,7 +17,8 @@ dependencies {
}

tasks.compileTestJava {
classpath += rootProject.tasks["shadowJar"].outputs.files
classpath += project(":java-api").tasks.getByName("testJar").outputs.files +
files(rootProject.sourceSets.map { it.compileClasspath }.flatten())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second part could be rootProject.tasks["jar"]outputs.files I think

Copy link
Contributor

@wagyourtail wagyourtail Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I would've passed the api jar as a flag, instead of directly putting it on the classpath. like I do in the other testing subproject

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the second one is a good idea

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

Successfully merging this pull request may close these issues.

3 participants