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

Setup app uses non standard dist task for lwjgl #7328

Open
3 of 6 tasks
lyze237 opened this issue Jan 28, 2024 · 14 comments
Open
3 of 6 tasks

Setup app uses non standard dist task for lwjgl #7328

lyze237 opened this issue Jan 28, 2024 · 14 comments

Comments

@lyze237
Copy link
Contributor

lyze237 commented Jan 28, 2024

Issue details

Hello!

The setup app uses a non-standard task to create a jar file (the dist one). It's of type jar but it's not the task named jar.

This means that gradle plugins can't reference the main class file.

An example gradle plugin could have code like this:

val jarTask = (tasks.findByName("shadowJar") ?: tasks.findByName("jar")) as Jar

if (!pluginExtension.mainClass.isPresent) {
   pluginExtension.mainClass.set(jarTask.manifest.attributes["Main-Class"] as String)
}

A suggestion would be to implement the shadow or application plugin.

  • The shadow plugin would take care of creating a "fat" jar.
  • The application plugin doesn't do that on its own, but liftoff has an example implementation.

To create a non-breaking change, you could create an alias task for the dist task. So, everyone can still run dist and it'd just call the jar task or whatever one.

tasks.register('dist') {
  dependsOn 'jar'
}

Version of libGDX and/or relevant dependencies

1.12.1

Stacktrace

N/A

Please select the affected platforms

  • Android
  • iOS
  • HTML/GWT
  • Windows
  • Linux
  • macOS
@Berstanio
Copy link
Contributor

I honestly prefer the "dist" task approach. It keeps the default "jar" task clean and doing it's job (just jaring the sources) and "dist" is more descriptive on what it does. But my opinion on that is not very strong, I would be fine with both.

This means that gradle plugins can't reference the main class file.

Maybe I'm wrong, but I think, a gradle plugin should not try to infer these things from the jar task. On the one side, there is no guarentee, that the jar task defines the "main-class" attribute. The default jar task just puts the classes/resources in a jar without any transitive dependencies added.
Also, it disables any way of post-processing (like obfuscation), if it just takes the jar result of the "jar" task directly.

@tommyettinger
Copy link
Member

If main-class isn't present in the manifest of a JAR, then that JAR isn't runnable and anything that expects main-class probably doesn't make sense to run on it.

The application plugin is actually official and meant for the task of making executable JARs, so I'm not sure what issues anyone has with it.

As for post-processing, with Lyze's last suggestion, there is still a dist task that could be easily be made to depend on proguard or some other obfuscation task instead of jar. ProGuard also doesn't, to my knowledge, obfuscate the name of the main class, since that would make the JAR un-runnable unless the manifest was also changed.

I think it's generally better to configure existing Gradle tasks (when possible) than to make new ones that aren't known to plugins.

For context, the reason this came up, I believe, was that gdx-setup isn't compatible with at least some version of the configuration for Construo, a newer and more-cross-architecture alternative to Packr. So, there is a practical reason backing this up. Any alternative solutions to getting the main class might also work, though I'm not certain of this.

@Berstanio
Copy link
Contributor

The application plugin is actually official and meant for the task of making executable JARs, so I'm not sure what issues anyone has with it.

But the application plugin also doesn't modify the jar task from what I can tell, so I'm not sure what this would it would fix in this case? Also, the application plugin just creates their own dist tasks.

As for post-processing, with Lyze's last suggestion, there is still a dist task that could be easily be made to depend on proguard or some other obfuscation task instead of jar.

My whole second paragraph was related to construo and the inference of jar/main-class from the jar task, which was the reason behind this issue.

I think it's generally better to configure existing Gradle tasks (when possible) than to make new ones that aren't known to plugins.

In my opinon plugins shouldn't depend on custom implementation of "known" gradle tasks. Even more, plugins might rather expect the known tasks to work in the default way (even though they of course shouldn't)

@tommyettinger
Copy link
Member

OK, now I'm really confused.

My whole second paragraph was related to construo and the inference of jar/main-class from the jar task, which was the reason behind this issue.

I'm pretty sure you're referring to this, your second paragraph, but it doesn't once mention construo, so I'm not sure how you expected me to guess that it was related to it:

Maybe I'm wrong, but I think, a gradle plugin should not try to infer these things from the jar task. On the one side, there is no guarentee, that the jar task defines the "main-class" attribute. The default jar task just puts the classes/resources in a jar without any transitive dependencies added.

Now I'm discussing your actual second paragraph. How do you think we should get the value of the main-class attribute?

You also appeared to be replying to this paragraph of mine, though I could be wrong:

As for post-processing, with Lyze's last suggestion, there is still a dist task that could be easily be made to depend on proguard or some other obfuscation task instead of jar.

So, I should explain what I meant more thoroughly. If the dist task by default just depends on the jar task, then configurations to jar will get passed onto the output of dist. If you customize dist to depend instead on, say, the proguard task, instead of jar, then it would be proguard that depends on jar, and then dist upon proguard. I believe the official ProGuard plugin's proguard task either depends upon jar by default or can be made to depend on it with one line. Obfuscation is still very much feasible.

In my opinon plugins shouldn't depend on custom implementation of "known" gradle tasks. Even more, plugins might rather expect the known tasks to work in the default way (even though they of course shouldn't)

Gradle allows configuring known Gradle tasks for a reason, and it seems widespread throughout the Gradle ecosystem. We aren't talking about writing a plugin here, this is the application-specific configuration generated by gdx-setup. There is a plugin for construo being skirted around, but unless we have a better alternative to where we can get the main-class information, configuring the jar task seems like the best solution so far.

As for the application plugin, it's just generally a good idea for applications; it provides a run task that mostly eliminates the need for custom run configurations.

@lyze237
Copy link
Contributor Author

lyze237 commented Jan 30, 2024

As for the application plugin, it's just generally a good idea for applications; it provides a run task that mostly eliminates the need for custom run configurations.

Or the shadow plugin linked as that takes care of everything including bundling dependencies into the jar itself.

Also, in my opinion the lwjgl3 jar file should always be ready to run as there's not much point in creating a lwjgl3 jar without any dependencies or manifest.

Additionally the lwjgl3 gradle file is also not configured well because of sourceSets.main.java.srcDirs = [ "src/" ] as that makes junit not work without modifications from what I've read on discord.

@Berstanio
Copy link
Contributor

but it doesn't once mention construo, so I'm not sure how you expected me to guess that it was related to it:

Yeah, sorry, I wasn't really clear on that, just woke up! I was talking about gradle plugins, that infer the "main-class" attribute. This means that gradle plugins can't reference the main class file.
Construo is a example of such gradle plugin, as lyze mentioned one line below: An example gradle plugin could have [code like this](https://github.com/fourlastor-alexandria/construo/blob/main/construo/src/main/java/io/github/fourlastor/construo/ConstruoPlugin.kt#L91):

How do you think we should get the value of the main-class attribute?

In my opinion, a gradle plugin should expose a configuration, where you can set the main-class. Furthermore, I think it should either build their own jar based on the runtime classpath or adding a configuration to supply a path to a input jar.

So, I should explain what I meant more thoroughly.

I got what you were saying and I agree that you are right. I was talking about "a gradle plugin, that uses the jar task to get it's input jar", like construo as an example. If a gradle plugin does that, obfuscation before that gradle plugin is not really possible.

we have a better alternative to where we can get the main-class information, configuring the jar task seems like the best solution so far.

I think just needing the user to set it, seems not like a bad way? Or am I missing any downsides?

As for the application plugin, it's just generally a good idea for applications; it provides a run task that mostly eliminates the need for custom run configurations.

I'm not against the application plugin, I just didn't saw how it fixes the issue of:

This means that gradle plugins can't reference the main class file.

@tommyettinger
Copy link
Member

First, to Lyze:

Additionally the lwjgl3 gradle file is also not configured well because of sourceSets.main.java.srcDirs = [ "src/" ] as that makes junit not work without modifications from what I've read on discord.

JUnit can be made to work, it's just a hassle that isn't necessary if you use the JVM project layout standardized by Maven many years ago, and also used by Gradle. (That is, using src/main/java for the main Java code and src/test/java for test Java code.) The issue there is in all projects produced by gdx-setup, including core and desktop, which are the most likely places you would have tests. I'm open to using the shadow plugin in Liftoff (assuming there's some advantage over what it already has), though that discussion should be in that project's issues.

Next, to Berstanio:

Having configuration for construo's plugin does seem like the best idea we have so far. It would be compatible with arbitrary runnable JARs made by whatever project generator, if I understand it right. It does involve repeating the configuration, once to make the runnable JAR, and once so construo knows what to launch. I feel like it should be possible to look inside any given JAR and see if it has a main-class attribute, since we probably only want to use construo to make applications out of runnable JARs. That would keep configuration in one place, since Gradle's jar or dist task would get the manifest all set up, and construo would just have to pull it from there -- I just don't know how that works.

@fourlastor
Copy link
Contributor

In my opinion, a gradle plugin should expose a configuration, where you can set the main-class. Furthermore, I think it should either build their own jar based on the runtime classpath or adding a configuration to supply a path to a input jar.

Construo does that (there's a main class option, not a jar location option), but it's frankly the least enjoyable options for devs, which kept asking if I could automatically support a way to guess it (note, this also happens on the beryx's jpackage plugin, it can automatically guess your main class from the application plugin). I think this makes sense as from a developer point of view, you're being forced to copy-paste config that you could have otherwise specified once.

Construo could generate its own jar task for it, but that's extra mainteinance on something already done (likely better than I could), and I don't think it's going to make it into the plugin for the foreseeable future.

Gradle is designed with interop between plugins in mind, to the point there's a whole paragraph in the documentation about how to interact with other plugins.

While I can see your point of view, I don't think there's any value for the developer to have to define all custom stuff that replicates exactly what other, well known plugins already do, and that are constantly updated to fix bugs and function with newer gradle versions, unless that's required by some project constraint (not libgdx's case), I find that a suboptimal solution in terms of dev experience, and it generates a lot of messy and hard to understand gradle files, which given many devs aren't that familiar with gradle, makes it just harder for them to keep maintaining their project.

Proguard does not have a desktop-compatible plugin, but if it had, it would be trivial to support it by depending on the right task (the one producing the obfuscated jar), similarly to how construo reacts to either shadowJar or jar.

IMHO, this discussion should be around what's best for the devs, instead of focusing on what is perceived to be the "right" solution, and I think the "just works" approach is preferable and far more mainteinable.

@Berstanio
Copy link
Contributor

First of all, I'm now a bit confused. Is this about a convinence feature to more easily infer the main class, or is this about something that does not work. I assumed it was the latter? Because of that my point was, that a gradle plugin should not rely on special configuration of some tasks, to work. If this is only about convenience, I understand this request better.

In general, as stated earlier, I prefer the seperate "dist" task. However, because liftoff also uses the jar task already, I think there is a strong argument to make, to make it consistent.

Just a few points:

I don't think there's any value for the developer to have to define all custom stuff that replicates exactly what other, well known plugins already do,

I'm not sure, what you are refering to? There is no in build gradle way to build a fat jar I'm aware of, libGDX also just supplies it's own code to build a fat jar.

Proguard does not have a desktop-compatible plugin, but if it had, it would be trivial to support it by depending on the right task (the one producing the obfuscated jar), similarly to how construo reacts to either shadowJar or jar.

proguard has no gradle plugin, but it is still commonly used for obfuscation. What I meant was, whether a user can obfuscate their code before it gets processed by construo. But this is not really about libGDX.

@fourlastor
Copy link
Contributor

First of all, I'm now a bit confused. Is this about a convinence feature to more easily infer the main class, or is this about something that does not work. I assumed it was the latter? Because of that my point was, that a gradle plugin should not rely on special configuration of some tasks, to work. If this is only about convenience, I understand this request better.

Construo, right now, will use either the shadowJar or jar tasks to produce the jar, it won't be able to produce a jar from a custom defined task.

It can infer the main class if the application plugin is set up, or if the jar/shadowJar tasks have the correct manifest property set, however that can be worked around via the construo { mainClass.set("my.main.Class") } option.

I'm not sure, what you are refering to? There is no in build gradle way to build a fat jar I'm aware of, libGDX also just supplies it's own code to build a fat jar.

Shadow is the de-facto standard for this, it achieves a similar result as the current dist task, however it doesn't require configuration (and can infer configuration from the jar task and application plugin).

proguard has no gradle plugin, but it is still commonly used for obfuscation. What I meant was, whether a user can obfuscate their code before it gets processed by construo. But this is not really about libGDX.

I think it might (with some custom config), but I haven't tested it, and there might be some quirks with it not being managed by a plugin (and so missing a well known api to attach to).

@tommyettinger
Copy link
Member

Proguard does not have a desktop-compatible plugin, but if it had, it would be trivial to support it by depending on the right task (the one producing the obfuscated jar), similarly to how construo reacts to either shadowJar or jar.

This official Gradle plugin doesn't count?

@fourlastor
Copy link
Contributor

That is the proguard api, not a gradle plugin :) you can use it to create your own task but it doesn't provide a standardised way of doing so. You could query for ProguardTask to see if there's any registered

@tommyettinger
Copy link
Member

tommyettinger commented Feb 5, 2024

That is the proguard api, not a gradle plugin :) you can use it to create your own task but it doesn't provide a standardised way of doing so. You could query for ProguardTask to see if there's any registered

I feel like I'm taking crazy pills here! I use the GuardSquare ProGuard Gradle Plugin in a few of my apps! https://github.com/yellowstonegames/SquidLib-Demos/blob/master/SquidSquad/DawnSquad/build.gradle#L11 https://github.com/yellowstonegames/SquidLib-Demos/blob/master/SquidSquad/DawnSquad/lwjgl3/proguard-rules.pro https://github.com/yellowstonegames/SquidLib-Demos/blob/master/SquidSquad/DawnSquad/lwjgl3/build.gradle#L77-L93

I don't know how you can confidently say there is no Gradle plugin!

EDIT: Not to mention the folder in the ProGuard source literally called gradle-plugin...

@fourlastor
Copy link
Contributor

fourlastor commented Feb 5, 2024

I think we're saying the same thing in different ways. What you're doing in your example is importing the gradle task in the buildscript classpath, and then defining your custom task using that type (which, as I mentioned earlier, could be used to find the correct task in construo).

What I meant is that it doesn't have a plugin which gets applied to the project, and instead it only contains the api to define such tasks.

This is not a problem per se, and construo could even configure its own proguard task on demand instead (so to be able to integrate the proguard config in the construo block)

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

4 participants