search

sergei-lapin / napt

Alternative to KAPT that skips Java stub generation and therefore is as efficient as Java APT but with Kotlin
MIT License
88 stars 6 forks source link

Compiler args are overridden #8

Closed ZacSweers closed 1 year ago

ZacSweers commented 1 year ago

this bit of code currently overwrites any existing arguments. would you be open to a PR to make it additive instead?

ZacSweers commented 1 year ago

errr sorry, I was looking at the wrong place. I think it's here, but I'm less sure now. The reason I filed this is because we see this issue now

Caused by: java.lang.IllegalAccessError: class com.google.googlejavaformat.java.JavaInput (in unnamed module @0x47a3f73a) cannot access class com.sun.tools.javac.parser.Tokens$TokenKind (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.parser to unnamed module @0x47a3f73a
        at com.google.googlejavaformat.java.JavaInput.buildToks(JavaInput.java:349)
        at com.google.googlejavaformat.java.JavaInput.buildToks(JavaInput.java:334)
        at com.google.googlejavaformat.java.JavaInput.<init>(JavaInput.java:276)
        at com.google.googlejavaformat.java.Formatter.getFormatReplacements(Formatter.java:260)
        at com.google.googlejavaformat.java.Formatter.formatSource(Formatter.java:247)
        at com.google.googlejavaformat.java.Formatter.formatSource(Formatter.java:213)
        at com.google.googlejavaformat.java.Formatter.formatSource(Formatter.java:200)
        at com.google.googlejavaformat.java.filer.FormattingJavaFileObject$1.close(FormattingJavaFileObject.java:72)
        at com.squareup.javapoet.JavaFile.$closeResource(JavaFile.java:144)
        at com.squareup.javapoet.JavaFile.writeTo(JavaFile.java:173)
        at dagger.android.processor.ContributesAndroidInjectorGenerator.generate(ContributesAndroidInjectorGenerator.java:124)
        at dagger.android.processor.ContributesAndroidInjectorGenerator.process(ContributesAndroidInjectorGenerator.java:84)
        at dagger.android.processor.ContributesAndroidInjectorGenerator.process(ContributesAndroidInjectorGenerator.java:53)
sergei-lapin commented 1 year ago

I'm open for a PR on this.🙂
I wonder how these packages end up opened when compiling without napt though 🤔

ZacSweers commented 1 year ago

I'm going to try to debug this today in our project and get to the bottom of it

ZacSweers commented 1 year ago

Ok I've figured it out. The issue is that when fork is called with new jvmArgs, they replace the existing ones entirely. As such, our existing args are overridden. I think the right solution would actually be to remove these entirely (or at least offer an opt-out) for consumers that implement their own and then just document which arguments must be included in order for this to work. Does that make sense?

ZacSweers commented 1 year ago

More specifically - we configure these arguments in the gradle daemon itself, but these fork options override them

ZacSweers commented 1 year ago

working like a charm now, thanks!