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

Applying to multi module project #4

Closed
hvisser opened this issue Aug 3, 2022 · 24 comments
Closed

Applying to multi module project #4

hvisser opened this issue Aug 3, 2022 · 24 comments

Comments

@hvisser
Copy link

hvisser commented Aug 3, 2022

I'm trying to use this plugin for https://github.com/littlerobots/version-catalog-update-plugin. That project has a plugin module and a separate module containing some logic around the version catalogs.

Following the README I've setup the plugin like this:

def shadeConfiguration = configurations.create("shade")

gr8 {
    def shadowedJar = create("gr8") {
        proguardFile("rules.pro")
        configuration("shade")
    }
    // Replace the regular jar with the shadowed one in the publication
    replaceOutgoingJar(shadowedJar)

    // Make the shadowed dependencies available during compilation/tests
    configurations.named("compileOnly").configure {
        extendsFrom(shadeConfiguration)
    }
    configurations.named("testImplementation").configure {
        extendsFrom(shadeConfiguration)
    }
}

repositories {
    gradlePluginPortal()
}

dependencies {
    shade project(":catalog")
    compileOnly("dev.gradleplugins:gradle-api:7.3.2")
    shade "org.jetbrains.kotlin:kotlin-stdlib:1.4"
    shade "com.fasterxml.jackson.dataformat:jackson-dataformat-toml:2.13.0"
    shade "com.squareup.moshi:moshi:1.12.0"
    shade "com.squareup.moshi:moshi-kotlin:1.12.0"
    testImplementation 'junit:junit:4.13.2'
    testImplementation 'com.github.ben-manes:gradle-versions-plugin:0.40.0'
    testImplementation(gradleTestKit())
    compileOnly 'com.github.ben-manes:gradle-versions-plugin:0.40.0'
}

It seems that I have to add the catalog module to the shade configuration too, otherwise none of the classes from that module appear to be added to the plugin jar. But this introduces an issue where I can no longer run my tests; classes from that project are not found in testKit tests, even when testImplementation is extending the shade config.

I'm also not sure if I really need to redeclare the dependencies that that module uses here in the shade config. It also seems that not all of the kotlin classes are relocated in the gr8 jar so that also seems to point to more configuration / user errors 😄

Can you point me in the right direction?

@martinbonnin
Copy link
Member

classes from that project are not found in testKit tests, even when testImplementation is extending the shade config

testKit is wildly untested. More specifically withPluginClasspath is wildly untested. I'm pretty sure that wouldn't work. Instead. I'd recommend publishing your plugin to a local maven repository and using that in integration tests to ensure the same environment as your users.

I'm also not sure if I really need to redeclare the dependencies that that module uses here in the shade config.

I think this isn't required. The configuration should resolve the transitive dependencies.

It also seems that not all of the kotlin classes are relocated in the gr8 jar so that also seems to point to more configuration / user errors

I can take a look at this. Since everything is open source, the easiest way is certainly that you push your current state to a branch and we can dig into this.

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

Thanks for the offer :) I've also extended the implementation config from the shade config now, which fixes the tests, however at runtime I'm getting errors...I think that's due to some stuff in META-INF/services getting relocated and renamed too, but I'm not 100% sure yet.

If you want to take a look, the branch is here https://github.com/littlerobots/version-catalog-update-plugin/tree/feature/shade-dependencies

Edit: It's definitely renaming the service locator files, which causes issues at run time. Adding various versions of adaptresourcefilenames and adaptresourcefilecontents to the rules file doesn't seem to work unfortunately.

@martinbonnin
Copy link
Member

Do you know what's using a ServiceLocator? You might have to keep the implementation class in rules.pro

As for the resource files, it might be an issue in gr8 itslef, not r8. We "merge" all jars together to create an embedded jar before feeding it to r8: https://github.com/gradleup/gr8/blob/5b6d501292ad8e7282891f9872161311e09b63eb/plugin-common/src/main/kotlin/com/gradleup/gr8/EmbeddedJarTask.kt#L54

If two resources have the same name, I'm not sure what's happening...

@martinbonnin
Copy link
Member

Tried the repo, I get an error on SerializationConfig that's crashing because there's an enum whose values() method is renamed somewhere around here.

Something like this seems to get me a bit further:

-keepclassmembers enum ** {
    public static **[] values();
}

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

Do you know what's using a ServiceLocator? You might have to keep the implementation class in rules.pro

Yeah I know what libs are using the service locator, just not how to relocate + retain the correct META-INF/services file. If I keep the implementation then it potentially still conflicts at runtime with a different version of the same library. If I don't then I don't think it loads the correct implementation?

Tried the repo, I get an error on SerializationConfig that's crashing because there's an enum whose values() method is renamed somewhere around here.

Ah yes, that's where I started but I somehow got distracted with those service locator files :) Will add that and dig further...

@martinbonnin
Copy link
Member

If I keep the implementation then it potentially still conflicts at runtime with a different version of the same library. If I don't then I don't think it loads the correct implementation?

Right. They'll have to be relocated... TBH, at this point I'd be looking for alternatives not using ServiceLocator. If it's for toml parsing, mayb https://github.com/akuleshov7/ktoml?

Will add that and dig further...

👍

To debug/investigate, it's sometimes useful to remap the symbols. The process to do so isn't super straightforward but it's doable if you have a local r8:

git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git
export PATH=/path/to/depot_tools:$PATH
git clone https://r8.googlesource.com/r8
cd r8 
./tools/gradle.py d8 r8
java -cp build/libs/r8_with_deps.jar com.android.tools.r8.retrace.Retrace path/to/your/repobuild/gr8/gr8/mapping
[copy paste your stacktrace and press Crtl-D to launch the retracing]

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

For now I ended up using the shadow plugin, which seems to work. I'm not relocating Kotlin with it so it would be nice to eventually switch to this plugin, but the service locator files and content are rewritten correctly with the shadow plugin, which seems to be the biggest hurdle.

One thing that is also needed with both this and the shadow plugin is that my additional module needs to stay out of the produced maven pom so that might something to keep in mind when others are trying a similar setup. Otherwise you end up with relocated things in your plugin artifact + a dependency on the non-relocated module + its dependencies.

I might give it another go in the coming days but for now I think this issue can be closed.

@hvisser hvisser closed this as completed Aug 3, 2022
@martinbonnin
Copy link
Member

martinbonnin commented Aug 3, 2022

Thanks for the update! Do you mind if I keep this issue open? I'd like to make it easier to consume the plugin and investigating real life use cases is a good exercise. I might give it a shot later this week.

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

I don't mind at all :)

@hvisser hvisser reopened this Aug 3, 2022
@martinbonnin
Copy link
Member

Quick question: how do you test? I'm running ./gradlew build --rerun-tasks --no-build-cache from the feature/shade-dependencies branch and it's all passing ✅

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

I'm testing this on a different project using the version catalog update plugin, publishing to mavenLocal() and then ./gradlew versionCatalogUpdate.

I made a few more changes on that branch to get it closer to the shadow plugin version, but it keeps failing on the service locator stuff so it seems. Due to the obfuscation it's a bit tricky to track down, I'm not sure if R8 is replacing the service locator string correctly. Feels like I'm close, chasing R8 issues like it's the year 2000 again 😛

@martinbonnin
Copy link
Member

Gotcha 👍 . Thanks 🙏

You don't have that other repo on some GitHub url by any chance? That'd save the trouble of writing a bunch of Gradle files. Not that I don't like it but... 😅

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

I don't have that specific one published, but https://github.com/code-with-the-italians/bundel is also using the plugin so you should be able to use that project, just replacing the version of the plugin. You may want to hardcode it in the plugins block otherwise the plugin will update itself. Though that would mean everything works so eh well 😅

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

Looks like I found it 🎉 Or at least, it's working now by adding -dontshrink to the rules.
Should have thought of that sooner I suppose...

Looking at the mapping file it seems like the kotlin std lib has also be relocated correctly, but it's still a bit scary 😬

@martinbonnin
Copy link
Member

Nice find! 🎉

Yea I feel you, it's definitely a bit scary 😅 but the only way to workaround the Gradle fixed runtime limitations. FWIW, we've been running for ~6 months now in the Apollo plugin and it's working well so far 🤞 .

Thanks for all the feedback in all cases 🙏

@hvisser
Copy link
Author

hvisser commented Aug 3, 2022

One question though, I can still not seem to bump to the latest Kotlin version. As soon as you apply the java-gradle-plugin it pulls in Gradle + some older version of Kotlin. Not sure what the difference is vs the Apollo plugin?

@martinbonnin
Copy link
Member

As soon as you apply the java-gradle-plugin it pulls in Gradle + some older version of Kotlin

There's a removeGradleApiFromApi() method in gr8. It's used there in Apollo to override the java-gradle-plugin defaults

@hvisser
Copy link
Author

hvisser commented Aug 4, 2022

hmm I actually have that so I'm not sure why that's not working for me.

@martinbonnin
Copy link
Member

Updating to Kotlin 1.7.10 seems to work for me: martinbonnin/version-catalog-update-plugin@d05321f. I get a module file like this:

plugin.module
{
  "formatVersion": "1.1",
  "component": {
    "group": "nl.littlerobots.vcu",
    "module": "plugin",
    "version": "0.5.4-SNAPSHOT",
    "attributes": {
      "org.gradle.status": "integration"
    }
  },
  "createdBy": {
    "gradle": {
      "version": "7.4-rc-1"
    }
  },
  "variants": [
    {
      "name": "apiElements",
      "attributes": {
        "org.gradle.category": "library",
        "org.gradle.dependency.bundling": "external",
        "org.gradle.jvm.environment": "standard-jvm",
        "org.gradle.jvm.version": 8,
        "org.gradle.libraryelements": "jar",
        "org.gradle.usage": "java-api",
        "org.jetbrains.kotlin.platform.type": "jvm"
      },
      "files": [
        {
          "name": "plugin-0.5.4-SNAPSHOT.jar",
          "url": "plugin-0.5.4-SNAPSHOT.jar",
          "size": 80162,
          "sha512": "58e82638ce9238cc702a7b325d2f3d5d82785a733ed84c555aa5006058d0b20ab6b8523f7d3bf9ba28a4aceb1871788cdb696aa5ce113861bb580e859a666f68",
          "sha256": "1d502706135b7887db0484571c74bc1484756ba0c777ff3e63710dcc6a3b85b2",
          "sha1": "adabfe4cdd30ba9c59367c8bff80bcb7681c25ff",
          "md5": "de61175e18bb0bfb68cfc60f04a0cf64"
        }
      ]
    },
    {
      "name": "gr8",
      "attributes": {
        "org.gradle.category": "library",
        "org.gradle.dependency.bundling": "shadowed",
        "org.gradle.libraryelements": "jar",
        "org.gradle.usage": "java-runtime"
      },
      "files": [
        {
          "name": "plugin-0.5.4-SNAPSHOT-shadowed.jar",
          "url": "plugin-0.5.4-SNAPSHOT-shadowed.jar",
          "size": 3750806,
          "sha512": "c46e304d8c57775fc2c1745af035cb5db11355725e4e06909ee9a837f5bc756a4d7906c574e323a28019c688c02ffa96d7bcde82972436fa24534d275d7cb45a",
          "sha256": "3878417019fa9aeb36b73cb006a9754f7762b67220bb2c4a2a0714c23e8ad712",
          "sha1": "3a9719b8dabb3306af4ace8996f4a95d9b60582c",
          "md5": "4a423a0eb60de768349b4e433c8cf8d8"
        }
      ]
    }
  ]
}

A bit weird that both "plugin-0.5.4-SNAPSHOT-shadowed.jar" and "plugin-0.5.4-SNAPSHOT.jar" but Kotlin isn't exposed as a transitive dependency

@hvisser
Copy link
Author

hvisser commented Aug 4, 2022

My main issue is that within the IDE everything is still locked to the older version, so I still can't really use newer Kotlin features (also with I check out your branch). Autocompleting the beloved uppercase() shows a crossed out toUppper() but no completion for uppercase() and also doesn't compile 😢

re: the module, I guess the file naming is off, but in terms of api/implementation it does seem to be correct right?

@martinbonnin
Copy link
Member

martinbonnin commented Aug 4, 2022

Ah yes, we bumped into a similar issue with @handstandsam. He wrote a nice blog post about it: https://handstandsam.com/2022/04/13/using-the-kotlin-dsl-gradle-plugin-forces-kotlin-1-4-compatibility/

The article was about the kotlin-dsl plugin but looks like the java-gradle-plugin is doing the same. You should be able to workaround with something like this:

afterEvaluate {
    tasks.withType(KotlinCompile).configureEach {
        kotlinOptions {
            apiVersion = "1.7"
            languageVersion = "1.7"
        }
    }
}

PS: this is actually super weird that java-gradle-plugin would know anything about KotlinCompile so I guess it's something else 🤔

PS2: it was actually there

@hvisser
Copy link
Author

hvisser commented Aug 4, 2022

🤦 yeah I just noticed when getting your reply in my email, thanks again! Going to look into that module.json thing (since I should probably remove the catalog and its modules in a better way) but looking good so far.

@martinbonnin
Copy link
Member

Hey @hvisser I'm bumping into this issue again and forgot most of the context 😓. Is it ok to close or is there something that still needs to be investigated there?

@hvisser
Copy link
Author

hvisser commented Apr 4, 2023

I'm fine closing this, I also forgot the context and in the end used the shadow plugin 😅

@hvisser hvisser closed this as completed Apr 4, 2023
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

2 participants