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

Antlr plugin adds compile dependency on complete binary JAR #820

Open
eriwen opened this issue Nov 7, 2016 · 22 comments
Open

Antlr plugin adds compile dependency on complete binary JAR #820

eriwen opened this issue Nov 7, 2016 · 22 comments

Comments

@eriwen
Copy link
Contributor

eriwen commented Nov 7, 2016

ANTLR has two dependency binaries, one is a smaller runtime dependency and another a larger compile only dependency.

The Gradle antlr plugin documentation says to add the compile only dependency in antlr configuration, which is then added to compile config by the plugin. This results in the compile only dependency being listed as a dependency for any project that uses gradle antlr plugin.

Expected Behavior

Project JARs built using the Gradle antlr plugin should only contain antlr-runtime classes (160 kB)

Current Behavior

Projects built using the Gradle antlr plugin include an additional 1.4MB of antlr classes that cause other problems explained below.

Context

Using antlr as a compile dependency is a problem for anyone trying to slim down shipped binaries. It is a slightly bigger problem for android projects that use antlr plugin, because android build will complain about the duplicated class files between the two dependency variants.

Use Cases to Consider

We need to support ANTLR2, which doesn't have a separation between runtime and tool. We can't break working builds except when we have made really egregious mistakes.

  • The build is using ANTLR2 with no explicit version.
  • The build is using ANTLR2 with an explicit version defined.
  • The build is using ANTLR3 with an explicit version defined and the build expects the "full" dependency.
  • The build is using ANTLR3 with an explicit version defined and the build only needs the runtime dependency.
  • The build is using ANTLR4 with an explicit version defined and the build expects the "full" dependency.
  • The build is using ANTLR4 with an explicit version defined and the build only needs the runtime dependency.

I think the current ANTLR support has evolved to where it is now, so it's not surprising that things are a bit muddled. I think it makes a lot of sense to treat the ANTLR tool classpath completely separate from the compile classpath.

Possible Fix Options

  1. Keep antlr configuration as-is, warts and all. Deprecate it.
  2. Introduce antlrTool that is completely separate from compile. antlrTool extends from antlr
  3. Update documentation for the ANTLR plugin to recommend that people use antlrTool to select the version of ANTLR to use and compile to select the version of the ANTLR runtime to use. For ANTLR2, the same dependency would be used for both.
  4. Bonus points for making parts of 3 automatic somehow.
@bysse
Copy link

bysse commented Mar 9, 2017

So you're saying that because of an old version of antlr, that became obsolete ten years ago, we should ship build tools to our customers / live environments? :)

@Bytekeeper
Copy link

A small workaround is to just removing the extendsFrom antlr and manually adding the runtime:

configurations {
    compile {
        extendsFrom = extendsFrom.findAll { it != configurations.antlr }
    }
}

dependencies {
    antlr  group: "org.antlr", name: "antlr4", version: "4.6"
    compile group: "org.antlr", name: "antlr4-runtime", version: "4.6"
}

@KaRkY
Copy link

KaRkY commented Jul 5, 2017

When will this issue be fixed it is at least 2 years old?

Supporting ANTLR 2.2 is in my opinion not needed since it is deprecated and at least 10 years old.

@oehme oehme removed the help-wanted label Sep 20, 2017
@gosp
Copy link

gosp commented Mar 16, 2018

Why #763 is reverted?
I use gradle 4.6, antlr jars still are configured as "compile" instead of "compileOnly".

@Vampire
Copy link
Contributor

Vampire commented Jun 7, 2019

Regarding point 3., compile is wrong anyway, it is deprecated since years.
I'd say the runtime should be added to implementation and if someone creates a parser to be used by consumer projects instead of internally, he should add the runtime dependency to api manually. (Or there could be some extension setting to configure whether it should be api or implementation with the latter being the default.

Another option to consider is doing it like for the codequality plugins.
What I mean is being able to just configure antlr { toolVersion = "4.7.2" } and then the plugin does the right configuration changes without the need (but the option) to add the dependency manually.

@gosp because it breaks existing builds

jarohen added a commit to bridje/bridje that referenced this issue Jan 23, 2020
derkoe added a commit to derkoe/tapestry-5 that referenced this issue Feb 10, 2020
 - remove antlr from dependencies, see gradle/gradle#820
 - remove duplicate Jetty dependency
@ice1000
Copy link

ice1000 commented Apr 22, 2020

Another workaround: let buildSrc use antlr("...") and generate the parser, so it's a runtime dependency of buildSrc instead of the project itself.

Example: https://github.com/JetBrains/Arend

@netomi
Copy link

netomi commented Aug 23, 2020

The workaround in #820 (comment) works for me, it is beyond my imagination why this problem is not fixed in a clean way on plugin level. Adding the compileOnly dependency needed to generate the parser / lexer classes as runtime dependency is just wrong.

@bysse
Copy link

bysse commented Aug 23, 2020

With all the breaking changes in Gradle I don't think "because it's breaking builds" can be used as an argument any longer.

@lorenzleutgeb
Copy link

lorenzleutgeb commented Oct 29, 2020

I also think that the ANTLR plugin should be fixed. However, I am stuck with the workaround #820 (comment). How do I do that with Kotlin Script? I tried

configurations {
  compile {
    setExtendsFrom(configurations.matching { it != configurations.antlr })
  }
}

but got

Cyclic extendsFrom from configuration ':compile' and configuration ':apiElements' is not allowed. See existing hierarchy: [configuration ':apiElements', configuration ':runtime', configuration ':compile', configuration ':annotationProcessor', configuration ':antlr']

Edit: Fix in #820 (comment)

So I found yet another workaround:

configurations {
    runtimeClasspath {
        exclude(group = "org.antlr", module = "antlr4")
    }
}

dependencies {
    antlr("org.antlr:antlr4:4.8-1")
    runtimeOnly("org.antlr:antlr4-runtime:4.8-1")
}

@Vampire
Copy link
Contributor

Vampire commented Oct 29, 2020

In your first try you translated the workaround wrongly. You extend from all configurations except antlr, so you for example also extend compile from compile. The workaround filters the ones currently extended from. The correct Kotlin version is

configurations {
    compile {
        setExtendsFrom(extendsFrom.filterNot { it == antlr.get() })
    }
}

@netomi
Copy link

netomi commented Oct 29, 2020

There are other cases where this works as intended, e.g. the annotationProcessor configuration. It would also be totally insane that all these dependencies would be included at runtime. Why does this not work the same way for the antlr configuration out of the box? Does not make any sense imho.

@pangiole-tibco
Copy link

pangiole-tibco commented Dec 20, 2020

Thank you @Bytekeeper! You gave this community a pretty good workaround 👍

I preferred to replace the deprecated compile configuration with proper api and implementation configurations (more recently introduced for newer Gradle versions).

Following is my Groovy snippet:

configurations {
    compile {
        // Undo extendsFrom relationship between the 'antlr' configuration and the 'api' configuration
        // See https://github.com/gradle/gradle/blob/master/subprojects/antlr/src/main/java/org/gradle/api/plugins/antlr/AntlrPlugin.java#L60
        extendsFrom = extendsFrom.findAll { it != configurations.antlr }
    }
}

def antlrVersion = '4.9'
dependencies {
    antlr  group: 'org.antlr', name: 'antlr4', version: antlrVersion
    implementation group: 'org.antlr', name: 'antlr4-runtime', version: antlrVersion
}

I hope I could find some spare time to push for a proper fix in the Groovy source code of the ANTLR plugin instead of having to apply the above workaround in my build.gradle script

@martinbonnin
Copy link
Contributor

2021 build.gradle.kts version:

dependencies {
    // ...
    antlr("org.antlr:antlr4:4.9")
    implementation("org.antlr:antlr4-runtime:4.9")
}

configurations[JavaPlugin.API_CONFIGURATION_NAME].let { apiConfiguration ->
  apiConfiguration.setExtendsFrom(apiConfiguration.extendsFrom.filter { it.name != "antlr" })
}

@shoaniki
Copy link

It is now 15 years since antlr2 was last updated, and the Gradle antlr plugin is still adding build tools not just to runtime configurations, but to the 'api' configuration – when even the actual runtime dependency surely belongs in 'implementation' by default.

I realise that this may not be high enough priority to change the code, given the risk that people's builds are depending on this behavior, but please at least consider explaining it in the plugin documentation and making the above workaround more discoverable.

@AlexLandau
Copy link
Contributor

AlexLandau commented Jun 27, 2022

The fix that was tried in #763 is significantly different from the four-part fix in "Possible Fix Options" in the ticket description. Am I right in thinking that that is because the behavior of defaultDependencies makes the antlrTool fix hard to implement correctly?

Maybe the path forward here (without causing any unwarned breaks) is to allow the use of default dependencies to be deprecated for a particular configuration -- i.e., if someone is applying the antlr plugin in 7.x without specifying an antlr dependency, give them a deprecation warning with the dependency to add explicitly and "this behavior will change in 8.0". Then the default antlr 2.x dependency can be removed in 8.0, and the antlrTool fix can be offered then.

@rex-structorum
Copy link

The ANTLR plugin is modifying the api configuration to extend from the antlr configuration. That is not a very nice thing to do. This is the offending line:

apiConfiguration.extendsFrom(antlrConfiguration);

I think the plugin would still work if that line is removed.


Attempting to "un-extend" the api configuration fails with UnsupportedOperationException:

configurations.api {
    extendsFrom.remove(configurations.antlr.get())
}

It appears that Gradle doesn't support "un-inheritance".

Here is my current workaround:

dependencies {
    antlr(libs.antlr4)
    api(libs.antlr4.runtime)
}

val antlrDependencies = HashSet<Dependency>()

configurations.antlr {
    allDependencies.forEach {
        antlrDependencies.add(it)
    }
}

configurations {
    api {
        antlrDependencies.forEach {
            exclude(it.group, it.name)
        }
    }
}

@rex-structorum
Copy link

Looking deeper, I think my previous "workaround" was insufficient. It also looks possible to "un-extend" configurations with setExtendsFrom, but it looks a bit tricky and possibly sensitive to dependency resolution and when you attempt it. This is really a mess. Probably easier to write my own task to run the ANTLR compiler.

@Vampire
Copy link
Contributor

Vampire commented Jun 10, 2023

@rex-structorum well, that is the work-around since years and working fine here since years.
It is also documented above in the comments already.
This is the current work-around (Kotlin DSL version):

configurations {
    api {
        setExtendsFrom(extendsFrom.filterNot { it == antlr.get() })
    }
}

@rex-structorum
Copy link

In your original post, you excluded it from the compile config. Now you are saying to exclude it from the api config.

Can you run a dependencies report to see if these are still showing up in other configs? And/or confirm that these JARs are not showing up as transitives in a dependent project?

In my recollection, through debugging I inferred that other configs were extending api before my build "un-extended" api from antlr. In my case, removal from api did not cleanup the dependencies in compile and elsewhere. Is this dependent on when dependency resolution occurs? In that case, I think I was triggering it early by callling the get() method on a configuration or trying to iterate through them.

@rex-structorum
Copy link

I got frustrated enough to drop the plugin and use the ANTLR tool cli in a custom gradle convention. That works for me because I'm collecting all the grammars I need into a single module-per-grammar project and publishing a bom for antlr-runtime version compat.

@Vampire
Copy link
Contributor

Vampire commented Jun 11, 2023

In your original post, you excluded it from the compile config. Now you are saying to exclude it from the api config.

Because back then it was added to the compile config and nowadays it is added to the api config.
You have to adapt the work-around to the Gradle version you are using.
compile was deprecated many years already and was finally removed in Gradle 7.
Also in Gradle 7 it was changed in the antlr plugin that not compile, but api extends from the antlr configuration: 2cb45cd#diff-665b2c549e6ddaa4ffba3a29504348df7aa5de7068bf66e935f73b4a6f9f2514L66-L69

Can you run a dependencies report to see if these are still showing up in other configs? And/or confirm that these JARs are not showing up as transitives in a dependent project?

They are not, why should they?

other configs were extending api before my build "un-extended" api from antlr.

Of course, many configurations extend api directly or indirectly just as it was with compile.
But this only becomes a problem if you prematurely resolve those configurations before you fix-up the configuration.
And even then it would not be a silent failure, as then you get an error thrown when you try to change the extendsFrom after api took part in the resolution process already.
So if you for resolve compileClasspath before doing the work-around, then when Gradle comes to the work-around you get

[...]
Caused by: org.gradle.api.InvalidUserDataException: Cannot change hierarchy of dependency configuration ':api' after it has been included in dependency resolution.
[...]

In my case, removal from api did not cleanup the dependencies in compile and elsewhere.

Of course not. If you still have compile, then you are on Gradle <7 where it is still compile that is made to extend antlr and needs to be manipulated.
As I said, you have to adapt the work-around to the Gradle version you are using.

Is this dependent on when dependency resolution occurs?

Yes and no.
As I said, if you resolved it before doing the fixup, you get an error on fixup.
If you do the fixup and do not get an error, then you didn't resolve too early, you just fixed the wrong configuration.

In that case, I think I was triggering it early by callling the get() method on a configuration

That does not yet resolve it, given you mean a Provider<Configuration>.

or trying to iterate through them.

Iterate through the resolution result would of course trigger resolution and should be done after the fixup.
Besides that optimally, it should not be done at configuration time at all.
There are even voices to forbid this by default: #2298

@rex-structorum
Copy link

Thanks for the detailed clarification. I'm contributing to some confusion by mixing up compile with compileOnly and compileClasspath. I'll try to reproduce my earlier results before going deeper because my memory is OK but not good enough for this issue from 2 months prior.

hhu-stups-mirror pushed a commit to hhu-stups/ltl-pattern-parser that referenced this issue Aug 21, 2023
hhu-stups-mirror pushed a commit to hhu-stups/antlr-parser that referenced this issue Sep 27, 2023
Workaround for gradle/gradle#820. This significantly reduces the size of
the shadowJar (14 MB -> 1 MB).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests