Skip to content

Adds the gradle wrapper#13744

Closed
adrianbk wants to merge 1 commit intoelastic:burn_maven_with_firefrom
adrianbk:gradle-wrapper
Closed

Adds the gradle wrapper#13744
adrianbk wants to merge 1 commit intoelastic:burn_maven_with_firefrom
adrianbk:gradle-wrapper

Conversation

@adrianbk
Copy link
Copy Markdown

It's generally a good practice to use the gradle wrapper so everyone is building with the same version of gradle. Instead of gradle someTask commands become ./gradlew someTask (assuming a nix environment)

@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 23, 2015

@adrianbk Thanks for the PR. I have been apprehensive about using the gradle wrapper for a number of reasons. Maybe you can assuage them.

  1. This means I have to run all gradle tasks from the root project? Or at least always refer back to it like ../../gradlew
  2. Autocomplete does not work because the gradle wrapper lib goes into gradle/. So typing ./g<tab> autocompletes to the dir (just requires typing an extra w, but still, a pain and seems to be counter to the point of the wrapper, ie easy running)
  3. (most important) It means adding binary files (a jar) to git history. This means every version we generate the wrapper for will be permanently in the history (pruning aside). In Lucene, all dependencies used to be checked into source control, and this has proved to be a huge pain when converting the repo to git (makes cloning take an enormous amount of time). Obviously this is a small jar, but it is still binary data.

What I would rather do is enforce the version of gradle within the build script itself. For example, the current build is written against the 2.6 api, but 2.6 or 2.7 should work fine. I haven't figured out an easy way to do this though (it does not appear, at least to me, that the gradle version is exposed programmatically).

FWIW, I would be completely fine with having this and the desired version check above, so that casual users trying to work on PRs would have less burden to have gradle set up. But I would probably not do it as long as there is a jar needed in source control. I have no idea what the technical limitations might be, but perhaps the wrapper could be bootstrapped my having a .java file instead of jar, and use javac first? It looks like the wrapper works with a JRE, but this seems silly since you can't run a build with a jre?

@big-guy
Copy link
Copy Markdown

big-guy commented Oct 1, 2015

I tried to reply to your concerns below. I think the wrapper is a good idea for a few reasons:

  1. Low barrier to building the project for first time contributors
  2. Switching branches/projects with the wrapper means you don't have to worry about $PATH pointing to the correct Gradle version
  3. When an upgrade occurs, you don't have to worry about some contributors lagging behind
  4. Testing a new Gradle version just means pointing the wrapper to a new URL
  5. If it becomes convenient to create a custom "Gradle with ES plugins" distribution, making sure everyone is using it is just an update to the URL

This means I have to run all gradle tasks from the root project? Or at least always refer back to it like ../../gradlew

Yes and no. Most of the time, I work from root, but I usually have a flatter project structure. To build from a subproject, you'd have to use relative paths to gradlew or use a script like gdub (https://github.com/dougborg/gdub).

You can also look at something like sdkman (http://sdkman.io/) if you're used to tools like rvm, but that raises the bar for contributors.

Autocomplete does not work because the gradle wrapper lib goes into gradle/. So typing ./g autocompletes to the dir (just requires typing an extra w, but still, a pain and seems to be counter to the point of the wrapper, ie easy running)

The point of the wrapper is to ensure everyone is building with the right version of Gradle. That could be a custom version of Gradle (bundled with plugins) or a particular release from gradle.org. In hindsight, it probably would have been nicer to call the script 'gw' or something unambiguous.

(most important) It means adding binary files (a jar) to git history. This means every version we generate the wrapper for will be permanently in the history (pruning aside). In Lucene, all dependencies used to be checked into source control, and this has proved to be a huge pain when converting the repo to git (makes cloning take an enormous amount of time). Obviously this is a small jar, but it is still binary data.

I looked at the history of the Gradle repo where we've updated the wrapper ~100 times. This makes up less than 5MB in the repo (assuming no compression), so I don't think this is a big issue. The wrapper can be used across many versions, so unless there are new wrapper features, you would never have to update it again.

What I would rather do is enforce the version of gradle within the build script itself. For example, the current build is written against the 2.6 api, but 2.6 or 2.7 should work fine. I haven't figured out an easy way to do this though (it does not appear, at least to me, that the gradle version is exposed programmatically).

The Gradle version is exposed as a property on the Gradle object: https://docs.gradle.org/current/dsl/org.gradle.api.invocation.Gradle.html#org.gradle.api.invocation.Gradle:gradleVersion

Enforcing (or complaining loudly about) the version of Gradle used within your build seems fine to me (to prevent the accidental use of another version), I think the wrapper is a nicer way to do that because you don't have to worry about telling someone how to go get Gradle.

If you start to publish your own Gradle plugins for consumption outside your build, the other thing to keep in mind is that you'll probably want to use the Gradle TestKit and plan for testing against multiple versions of Gradle (or just declare that version X is the supported one). At most, I'd have the plugins complain if they're used with an untested version of Gradle (I wouldn't fail the build).

FWIW, I would be completely fine with having this and the desired version check above, so that casual users trying to work on PRs would have less burden to have gradle set up. But I would probably not do it as long as there is a jar needed in source control. I have no idea what the technical limitations might be, but perhaps the wrapper could be bootstrapped my having a .java file instead of jar, and use javac first? It looks like the wrapper works with a JRE, but this seems silly since you can't run a build with a jre?

The wrapper can start with the JRE and then find a JDK to run the build with (e.g., setting org.gradle.java.home to a JDK). When Groovy was moving over to Apache, I think there was a minor fracas about a binary artifact in the repository, so they considered some different things. I think they were eventually allowed to keep the wrapper, but I think one of the suggestions was to download the gradle-wrapper.jar from somewhere in the gradlew/gradlew.bat scripts.

HTH

@uschindler
Copy link
Copy Markdown
Contributor

Hi,
Ryan was talking with me about this as I am the "Apache Guy". I wrote the gradle plugin for forbiddenapis, released yesterday as version 2.0. I also investigated using the gradle wrapper to test the plugin in an gradle environment (the build of forbiddenapis is ANT and that is fine for multi-build-system plugin: we have Ant, Maven, Gradle, CLI in forbiddenapis and bugs like gradleApi() introducing tons of bullshit into your compile environment, including incomaptible ASM versions stopped me from migrating the whole build to gradle - so gradle not always have good sides). The most horrible thing is: You cannot get Gradle and all its internal dependencies on Maven Central! I have no idea why this is like that, but this makes developing plugins a pain, unless you use Gradle to build your plugin with gradleApi() f*cking up your classpath,

While reviewing Gradle wrapper to have an easy test environment for the plugin, I just noticed the following problems: In Lucene and Solr we are not allowed to put JAR files into source distributions or commit them to SVN. There are checks of the download archives regularily that enforce this. This was a big issue before Lucene's build moved to Ant/Ivy, because we also had compile time dependencies committed to SVN, and this was a blocker at some point! On top of that: Recently Groovy got into Apache Incubation as it wants to join as a full fledged ASF project. One of the first commits after importing the Git repo into ASF was - you might guess it - removing the wrapper jar and wrapper scripts (see https://mail-archives.apache.org/mod_mbox/incubator-groovy-notifications/201505.mbox/%3Cc28c218e8f694efbaa5fa774ceb26b72@git.apache.org%3E). In the meantine they seem to had some discussions (also on the legal forums) and I think it was added back as a "special case", but having binary files in a source distrbution is still a no-go.

Here was the discussion: http://mail-archives.apache.org/mod_mbox/groovy-dev/201504.mbox/%3C552F3B8F.6030207@gmx.org%3E

Other Apache projects use custom shell scripts to download the gradle wrapper. It would all be simple if the gradle wrapper would be deployed to Maven central, but it isn't. In Lucene we have similar code that downloads the IVY version to use with Ant using "ant ivy-bootstrap".

As Elasticsearch is no ASF project, the situation might be different here, but my personal opinion is also: Don't do this! The Gradle people want to push you to this, but this is not real open-source like (you know there is a company behind, looks like they want to push their binary into every source repo).

I don't see a problem requiring a Gradle installation to build Elasticsearch. Where is the difference to Maven or Ant required to build a project?

@melix
Copy link
Copy Markdown
Contributor

melix commented Oct 1, 2015

To be clear we never removed the wrapper. I think it's a problem that the ASF doesn't allow it (the Eclipse Foundation, which is even worse than ASF in terms of IP compliance, allows it). What we did is removing the wrapper from the source zip that we produce. With my Gradle hat on, I would never recommand not to use the wrapper. You have to use it.

As for gradleApi() polluting your classpath, I'm not sure what you mean but it might be worth pulling this on the forums. The thing is that gradleApi() is for a Gradle plugin, so it will bring everything on classpath that Gradle needs, and it is expected I think.

@uschindler
Copy link
Copy Markdown
Contributor

With my Gradle hat on, I would never recommand not to use the wrapper. You have to use it.

No, you don't have to use it. You can download the source distribution and install gradle in parallel (as I did) and run the build by entering "gradle task". Why do I need a wrapper for that? I don't need one for Maven or Ant, whats different here for Gradle?

As for gradleApi() polluting your classpath, I'm not sure what you mean but it might be worth pulling this on the forums. The thing is that gradleApi() is for a Gradle plugin, so it will bring everything on classpath that Gradle needs, and it is expected I think.

It is this issue: https://issues.gradle.org/browse/GRADLE-1715

I have no idea why there is no work on resolving this. This is a blocker for using Gradle to build stuff that relies on for example on ASM (and shaded later). There are also issues to build a plugin for 3 build systems: Gradle, Maven and Ant. The classpath is polluted also with Ant and Maven dependencies (of course in wrong versions). Elasticsearch also has a custom plugin (see buildSrc), so it affects uses here, too.

I would say, this is not related to this issue, I just brought it in here, because Gradle has problems as a build system like others have, too. I just wanted to bring in the whole story.

At the moment I would still argue against using Gradle to build Lucene. I am happy with Ant.

@melix
Copy link
Copy Markdown
Contributor

melix commented Oct 1, 2015

You can download the distribution, but you should not have to. Ideally your CI server or your users shouldn't have any prerequisite to build ElasticSearch (apart from a JDK). So using the wrapper, you're locking things down, including the version of the tool which has been validated to work. If tomorrow someone upgrades something in the build, and that this little something requires an upgrade of Gradle, without the wrapper, it can easily be unnoticed. Using the wrapper you don't have such a problem. If your build requires an upgrade of the toolchain (Gradle), the build knows. The CI configuration will not need to be updated, and you won't have to know, it's all transparent.

@uschindler
Copy link
Copy Markdown
Contributor

I would be fine if all this could be done with a simple shell script that downloads gradle-wrapper.jar (from Maven Central, please), but having a binary JAR in your source distro is a big no-go for me.

@melix
Copy link
Copy Markdown
Contributor

melix commented Oct 1, 2015

You can do exactly like what we did for Groovy: have it in Git, exclude it from the source zip. It's a pity to have to do this, but that's what the ASF requires, nothing more. Having it in Git is important because a shell script that downloads the wrapper is easy to write, but it has problems too:

  • dependency on curl or whatever tool you would use to download the jar. (wget, ...), so you add another pre-requisite
  • there's still no way to do such a thing under Windows, and Gradle has to support it.

@uschindler
Copy link
Copy Markdown
Contributor

As said before, I have no problem in documenting that you need a specific version of Gradle to build Elasticsearch and put a link in README. This is similar to Maven today (or Ant/Ivy in terms of Lucene). If you put something into the build.gradle that complains if the version is wrong (like <antversion later=.../> in Ant) I am happy. And for Jenkins it is perfectly fine to declare this as a "Gradle build for version XY", refer to build.gradle. Very easy for the CI manager; and similar to statement "dowload from Github URL xy before starting gradle build". It is just a simple GUI configuraion in your CI server.

@melix
Copy link
Copy Markdown
Contributor

melix commented Oct 1, 2015

You can do it, but doing this you're doing too much work. You are asking people to install a build tool, which they can avoid. You are asking them to check the README if something goes wrong because you happened to upgrade Gradle, which we can avoid. You are also putting more work on the CI configuration side, where again you could avoid it. And last but not least, if you have several branches and that they use different versions of Gradle, you have to configure it when again you could avoid it.

I don't say you must use the wrapper. I'm saying there is no good reason not to use it.

@adrianbk
Copy link
Copy Markdown
Author

adrianbk commented Oct 1, 2015

Gradle is far more than both Maven and Ant, you simply will not get the same capabilities and flexibility. Gradle is very much designed with automation in mind, the gradle Wrapper being a single example.

just a simple GUI configuraion in your CI server

is the antithesis of automation. Consider having hundreds or even thousands of CI servers.

@uschindler
Copy link
Copy Markdown
Contributor

Elasticsearch master already has minimum Java 8 as requirement. Lucene trunk is same and branch_5.x may migrate soon. Java 8 requires to have the "jjs" (the command line tool of nashorn) scripting engine in JDK available, so I would prefer to have a simple Javascript for beginners available. Gradle should also think about using this instead of binaries like gradle-wrapper.jar as "internet ähm build engine downloader".

@nik9000
Copy link
Copy Markdown
Member

nik9000 commented Oct 1, 2015

I'd be supper happy with a shell script and a bat script and sad about a jar file. For what that is worth.

@ywelsch
Copy link
Copy Markdown
Contributor

ywelsch commented Oct 1, 2015

The Gradle wrapper makes it much simpler to get started working on Elasticsearch without having to worry about a locally installed Gradle version. I think that ease of use should be a determining factor here. Having the Gradle wrapper still gives you the freedom of not using it and running the build from a custom Gradle installation.

About the wrapper jar specifically: I fail to see the relevance of ASF policies here. Pivotal, Netflix and many others are using the Gradle wrapper in their OSS. As for the wrapper jar itself, it is very small and can, if need be, directly be built from the Gradle sources.

@rjernst The directory of the wrapper jar does not need to be named gradle, see property jarFile of Wrapper task. As for why Gradle does not rely on the JDK but only JRE, note that Gradle is not only used to build Java projects (and other Java compilers like ecj do not need a JDK).

@clintongormley
Copy link
Copy Markdown
Contributor

Given the desire not to have a binary blob in our git repo, and the fact that we've settled into our gradle installation now, I'm going to close this PR.

thanks for submitting, and for the interest

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.

8 participants