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

Build overhaul #413

Merged
merged 15 commits into from
Aug 29, 2021
Merged

Build overhaul #413

merged 15 commits into from
Aug 29, 2021

Conversation

weisJ
Copy link
Contributor

@weisJ weisJ commented Aug 11, 2021

See #412

This time gradle is rewired to use the old source/resource locations.

The only thing which couldn't be avoided is to move the test resources into their own directory. This is due to the nature of the java-library package.

Github history is preserved this time: e.g. https://github.com/gurkenlabs/litiengine/commits/136a19900ca8d5a79b760d2c90ab6f9d0646be44/src/de/gurkenlabs/litiengine/Game.java

@weisJ
Copy link
Contributor Author

weisJ commented Aug 11, 2021

This PR doesn’t include the automatic formatting through spotless. I can add it back in or create a separate PR.

@nightm4re94
Copy link
Member

This PR doesn’t include the automatic formatting through spotless. I can add it back in or create a separate PR.

I think it's best to open another PR for that once everything else is back in order.
Thank you!

.gitattributes Outdated Show resolved Hide resolved
This avoids duplicating .gitignore files.
This ensures consistent line endings
The gradle.kts build files provide a more type safe dsl.
It also makes it easier to not accidentally cause tasks
instantiation when its not necessary.

* Extract common configuration into the root build script.
* Configure gradle to produce reproducible builds.
* Move litiengine specific config into the litiengine subproject.
  To avoid moving the source files we rewire the project
  layout expected by the `java-library` plugin.
* Extract dependency/plugin configuration into `settings.gradle.kts`.
  Using the dependency catalogs introduced in gradle 7 we can abstract
  away from specific implementations and ensure consistent version.
  All dependency and plugin versions are declared in `gradle.properties`
* Replace launch4J config with a jpackage based approach for utiliti.
  This also packages a stripped down version of the java runtime avoiding
  any version conflicts on the user side.
* Update to java 16
The previous approach of adding the class output
of the tests as an dependency was brittle and prevented
proper incremental builds when tests are compiled.
The old `javax.xml.bind` implementation doesn't support
newer java releases. The project has moved to `jakarta.xml.bind`.
The `java-library` plugin doesn't know about resources in the
source directory. We need to move the test resources into their
own directory to compensate for this.
All paths referenced in the tests need to be updated for this.
Resource get embedded into the jar at runtime, hence our tests
should also treat them as such.
The test relied on a file not being present when executed.
To ensure this we delete the file if it exists.
Ensure the image is present at the given location by pinning the commit hash.
To be more spec compliant jdk 16 sets the unknown OS flag (255) in the gzip header instead of just 0.
The expected result needed to be changed in the test case.
This change was introduced in the following commit:
openjdk/jdk@e5866aa
@nightm4re94
Copy link
Member

Thanks for addressing the open issues :) Is this PR now ready for review?

@weisJ
Copy link
Contributor Author

weisJ commented Aug 22, 2021

Yes it is :)

@nightm4re94 nightm4re94 merged commit 4c7b5ed into gurkenlabs:master Aug 29, 2021
@nightm4re94
Copy link
Member

This has been a huge improvement - we really appreciate the effort you've put into this PR! <3

@weisJ
Copy link
Contributor Author

weisJ commented Aug 30, 2021

I’d suggest providing a new version for utiliti with the jpackage build. It seems like there have been more and more issues with people not getting it to run with their Java version. The jpackage build bundles it’s own jre, so this shouldn’t be a problem anymore.

Edit: I have just seen in discord that this is indeed planned :)

@nightm4re94
Copy link
Member

Hi again @weisJ, we are currently trying to get a local clone of the engine imported as a Gradle dependency in a project.
Usually, we had used the following gradle configuration to make this work:

in build.gradle:

dependencies {
    implementation project(':litiengine')
}

in settings.gradle:

include ":litiengine"
project(":litiengine").projectDir = file("../litiengine")

As we have just found out, this setup is no longer working for our existing projects. What would the correct configuration be to use a local copy of the engine?

@weisJ
Copy link
Contributor Author

weisJ commented Sep 27, 2021

Assuming the following project structure:

project/settings.gradle
litiengine/setting.gradle.kts

Then in project/settings.gradle:

includeBuild "../litiengine"

You'll be able to declare litiengine as a dependency as usual (i.e. as if you were consuming it from maven). Gradle will automatically replace any dependency with the version provided by includeBuild if it finds a matching module.
At first this will fail with the error message that two modules satisfy the requested dependency. Simply add the following line to
litiengine/setting.gradle.kts:

rootProject.name = "litiengine-root"

It will disambiguate the module names. (Its safe to permanently commit this change as it only affects the name of the root project which isn't published anyway)

@nightm4re94
Copy link
Member

Thanks for clarifying!

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