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

Make an equivalent to Ant's "run" target for Luke module [LUCENE-9448] #10488

Closed
asfimport opened this issue Aug 7, 2020 · 42 comments
Closed

Comments

@asfimport
Copy link

asfimport commented Aug 7, 2020

With Ant build, Luke Swing app can be launched by "ant run" after checking out the source code. "ant run" allows developers to immediately see the effects of UI changes without creating the whole zip/tgz package (originally, it was suggested when integrating Luke to Lucene).

In Gradle, :lucene:luke:run task would be easily implemented with JavaExec, I think.


Migrated from LUCENE-9448 by Tomoko Uchida (@mocobeta), resolved Aug 13 2020
Parent: #10119
Attachments: LUCENE-9448.patch (versions: 2)

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I forgot about this, @ErickErickson reminded me (thanks Erick).

It is not an ordinary build task but a helper for developers - @dweiss does it make sense we also have this in Gradle ?

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

Hmm... This would keep the gradle daemon running while the subprocess is launched.

My opinion is that it makes little sense, to be honest. Rather, the assembly should be straightened up so that build/luke contains the scripts and dependencies required to run Luke. Then you could do:

gradlew :lucene:luke:assemble

and it'd compile everything needed, then maybe print something like:

Luke is ready to launch, run it with:
java -jar lucene/luke/build/luke/luke....jar

A proper classpath manifest and main class would be also helpful (then you don't have to run any scripts, just the jar itself).

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

I still think that a "quick tester" not assembling everything into JAR files and just using the runtime classpath would be a good idea. That's my personal opinion. luke.jar is not a "fat jar", it still needs all dependencies on classpath.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

It doesn't need to be a fat jar if it had proper classpath attribute in the manifest (relative references to lib/*.jar). Then you can just launch it and classpath will be set automatically - no need for scripts to do that.

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

The problem is that the lib folder structure looks different in releases and in the local build system. I tried to set this up back at the time when Tomoko and I did the inital import.

But this is unrelated to just allowing to start the GUI from gradle's command line. That's a simple task which depends on compile (or similar). Makes life easier and faster for quick tests.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

The local assemble task should sync everything needed from the runtime configuration - then you'd be running the "actual" distribution package.

I don't mind having a 'run' task which wouldn't assemble anything, of course, but I don't think it makes practical sense as it'll hang the build in the IDE, for example. But that's up to you, really.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

While I try to write up my comment, the discussion has been proceeding... I will just throw in my understanding here, although there may be no new information.

Luke is not a self-contained jar and the relative location of the dependent jars are different in building and packaging time, we can't specify build time Class-Path in the JAR Manifest. (Am I correct?)

I'm not sticking to make a Gradle task at all, but I think an alternative which is run as easy as "ant run" would be needed. Let me keep open this until we find a good solution; anyway it is not urgent.

 

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

I don't mind having a 'run' task which wouldn't assemble anything, of course, but I don't think it makes practical sense as it'll hang the build in the IDE, for example. But that's up to you, really.

Why would that hang the IDE? If you dont execute the task and it's not part of the standard "build all", who cares? We have many "run-like" tasks which the IDE would never run, like "regenerate".

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

One example: I would also like to have the "ant smoke-test", "ant jenkins-nightly", which is triggered by jenkins for easier CI configuration. That's not different to the current. Some "helper tasks" not used by IDEs, just for special use cases are perfectly fine, IMHO.

You can argue to add scripts for this, but as we run builds on windows and linux, I tend to not use shell scripts for this.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

> Why would that hang the IDE?

I'm guessing IntelliJ has a single worker gradle daemon.... guessing though.

Luke is not a self-contained jar and the relative location of the dependent jars are different in building and packaging time, we can't specify build time Class-Path in the JAR Manifest. (Am I correct?)

They are different but they point at the same set of artifacts. It's really simple to sync libs and set proper relative manifest based on that. Look at this snippet, for example:

https://github.com/carrot2/carrot2/blob/master/dcs/distribution/build.gradle#L38-L54

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

@dweiss thanks for the pointer. I'll open another issue for the jar manifest.

 

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

You can do this here as well, Tomoko - these are related things anyway.

@asfimport
Copy link
Author

asfimport commented Aug 7, 2020

Erick Erickson (@ErickErickson) (migrated from JIRA)

I mentioned this to Tomoko when I got back to SOLR-13412, "Make the Lucene Luke module available from a Solr distribution" (really, just making it another command from bin/solr or similar). I eventually figured out that we don't actually package up the Luke jar or add it to the distro in the ant build and dropped it, and just this week got back to thinking about it.

It seemed like something that'd be useful to have accessible from Solr if it was easy. I wanted to mention this as an input into the discussion of how to build Luke in the Gradle world. That said, do whatever is easiest from your perspective. The motivation behind SOLR-13412 was just that if it was easy to access from a distro, then I can imagine some Solr users finding it useful. If it's not simple I'll just close the Solr JIRA. Solr users who need/want to access the indexes via Luke may be comfortable downloading Solr and running Luke from a gradle anyway.

@asfimport
Copy link
Author

asfimport commented Aug 9, 2020

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Hi @ErickErickson
as for SOLR-13412, I think there would be nothing special to ship Luke with Solr - it's just an ordinary JAR file (like other lucene modules) and its all dependent jars may be already included in Solr. Once correct class paths are set, Luke runs on everywhere else. (Please see this launch shell/bat: https://github.com/apache/lucene-solr/tree/master/lucene/luke/bin)

I'm not fully sure if it'd be somewhat helpful/useful for Solr users though...

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I attached a poc patch LUCENE-9448.patch.
Main class and classpaths are specified in the Manifest, so that Luke is launched by java command for testing.

$ ./gradlew lucene:luke:testAssemble
$ java -jar lucene/luke/build/libs/lucene-luke-9.0.0-SNAPSHOT.jar

There remains one TODO; how can we set correct Class-Path for distribution package, or maybe we should omit it for the distro for now? @dweiss what do you think?

 

We could emulate the directory structure of the final distribution package for all dependent jars when testing (so that the same Class-Path attribute can be used for UI testing and packaging), but it would mess up `luke/build/` ...

LUCENE-9448.patch
diff --git a/lucene/luke/build.gradle b/lucene/luke/build.gradle
index 6e32b1b35a5..3168ba817c4 100644
--- a/lucene/luke/build.gradle
+++ b/lucene/luke/build.gradle
@@ -19,6 +19,14 @@ apply plugin: 'java-library'
 
 description = 'Luke - Lucene Toolbox'
 
+ext {
+  standaloneDistDir = file("$buildDir/${archivesBaseName}-${project.version}")
+}
+
+configurations {
+  standalone
+}
+
 dependencies {
   api project(':lucene:core')
 
@@ -33,3 +41,88 @@ dependencies {
 
   testImplementation project(':lucene:test-framework')
 }
+
+// Configure main class name for all JARs.
+tasks.withType(Jar) {
+  manifest {
+    attributes("Main-Class": "org.apache.lucene.luke.app.desktop.LukeMain")
+  }
+}
+
+// Configure the default JAR without any class path information
+// (this may actually be wrong - perhaps we should add the
+// "distribution" paths here.
+jar {
+  manifest {
+  }
+}
+
+// Configure "stand-alone" JAR with proper dependency classpath links.
+task standaloneJar(type: Jar) {
+  dependsOn classes
+
+  archiveFileName = "${archivesBaseName}-${project.version}-standalone.jar"
+
+  from(sourceSets.main.output)
+
+  // manifest attributes are resolved eagerly and we can't access runtimeClasspath
+  // at configuration time so push it until execution.
+  doFirst {
+    manifest {
+      attributes("Class-Path": configurations.runtimeClasspath.collect {
+        "lib/${it.getName()}"
+      }.join(' '))
+    }
+  }
+}
+
+task standaloneAssemble(type: Sync) {
+  from standaloneJar
+  from(configurations.runtimeClasspath, {
+    into "lib"
+  })
+
+  into standaloneDistDir
+
+  doLast {
+    logger.lifecycle("Standalone Luke distribution assembled. You can run it with:\n"
+        + "java -jar " + file("${standaloneDistDir}/${standaloneJar.archiveFileName.get()}"))
+  }
+}
+
+// Create a standalone package bundle.
+task standalonePackage(type: Tar) {
+  from standaloneAssemble
+
+  into "${archivesBaseName}-${project.version}/"
+
+  compression = Compression.GZIP
+  archiveFileName = "${archivesBaseName}-${project.version}-standalone.tgz"
+}
+
+// Create a set of artifacts for standalone distribution in a specific
+// exported configuration so that it can be pulled in by other projects.
+artifacts {
+  standalone standaloneDistDir, {
+    builtBy standaloneAssemble
+  }
+}
+
+// Utility to launch Luke (and fork it from the build).
+task run() {
+  dependsOn standaloneAssemble
+  description "Launches (spawns) Luke directly from the build process."
+  group "Utility launchers"
+
+  doFirst {
+    logger.lifecycle("Launching Luke ${project.version} right now...")
+    ant.exec(
+        executable: 'java',
+        spawn: true,
+        vmlauncher: false
+    ) {
+      arg(value: '-jar')
+      arg(value: file("${standaloneDistDir}/${standaloneJar.archiveFileName.get()}").absolutePath)
+    }
+  }
+}
diff --git a/solr/packaging/build.gradle b/solr/packaging/build.gradle
index 55b78ca0182..3e222af7bfb 100644
--- a/solr/packaging/build.gradle
+++ b/solr/packaging/build.gradle
@@ -37,6 +37,7 @@ configurations {
   contrib
   example
   server
+  luke
 }
 
 dependencies {
@@ -64,6 +65,8 @@ dependencies {
 
   example project(path: ":solr:example", configuration: "packaging")
   server project(path: ":solr:server", configuration: "packaging")
+
+  luke project(path: ":lucene:luke", configuration: "standalone")
 }
 
 task toDir(type: Sync) {
@@ -76,6 +79,10 @@ task toDir(type: Sync) {
     include "README.md"
   })
 
+  from(configurations.luke, {
+    into "tools/luke"
+  })
+
   from(project(":lucene").projectDir, {
     include "CHANGES.txt"
     rename { file -> 'LUCENE_CHANGES.txt' }

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

Looks good to me, Tomoko. Regarding Erick's question - I always thought Luke is still a "stand-alone" tool so I suggested dependency assembly for a stand-alone tool (relative to its jar). If you want to avoid JAR duplication and put it in the distribution then things become a bit more convoluted.

Give me some time and I'll try to provide a patch for this, have to think about how to do it myself.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I always thought Luke is still a "stand-alone" tool so I suggested dependency assembly for a stand-alone tool

I'm not sure if it is related... when Luke was integrated into Lucene, my very first suggestion was creating a stand-alone Luke app (zip/tar) that is separately distributed from Lucene, just like Solr; it was rejected and I did not argue about it. I just remembered that.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I attached a patch that demonstrates a couple of things. It's not really something to commit in directly - it's bits and pieces to consider.

  • luke's build file declares an exportable configuration 'standalone' that includes a set of preassembled dependencies and classpath-enriched Luke JAR. This configuration is separate from the default project JAR. It can be assembled (standaloneAssemble), compressed into a tgz archive (standalonePackage) or exported and reused elsewhere (I added copying to tools/luke in Solr as a demonstration of how trivial it is).
  • I also added a "forkable" target that launches Luke ("run"). It's done via ant but allows the build to terminate gracefully, without waiting for the forked process to complete.

Setting proper "relative" classpaths in the Lucene distribution package (so that there is no JAR duplication) is a bit of a hassle because logically distribution packaging is a downstream task from Luke's assembly. I'm sure it can be done, but I'd wait until there is a packaging project in place.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Thank you @dweiss for your help. 

luke's build file declares an exportable configuration 'standalone' that includes a set of preassembled dependencies and classpath-enriched Luke JAR. This configuration is separate from the default project JAR. It can be assembled (standaloneAssemble), compressed into a tgz archive (standalonePackage) or exported and reused elsewhere

As for 'standalone' package, there are drop-in runtime only dependencies (many analysis modules) which are not required for development at all. If we make complete standalone distribution package by Gradle script, we need to collect all such jars or add all of them to compile time dependencies.
I'll try it a little later (have little time right now).

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I added a pull request this time - removed Solr distribution bit since we established it's not the right time, but added an optional "standalone distribution-only" JAR as an example (highlighter) and a readme file with substitutable parameters. Once you run:

gradlew -p lucene\luke assemble

you'll see the readme file will contain a java command to launch Luke. I don't think scripts are needed after this (it's a developer tool after all, so I assume minimal terminal capabilities).

I'm leaving the rest to you, Tomoko - continue as you please.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Seeing standalonePackage task, I started to reconsider about distribution - would it be natural and understandable that we distribute stand-alone Luke app (and exclude the luke artifact from Lucene package)? I don't think it should be related to 9.0.0 release, I'd like to revisit it in the future releases. Just an idea.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

It is absolutely possible and doable. I think Luke should be part of Lucene distribution artifacts - whether together or separately, that's an open question. I'm sure this question will pop up again once we get down to how to assemble distribution packages from gradle level.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Fix for this issue (merged): apache/lucene-solr#1742

I think this can be resolved?

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit bbd21aa in lucene-solr's branch refs/heads/master from Tomoko Uchida
https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=bbd21aa

LUCENE-9448: Move README.txt to README.md; We no lonnger have txt format README on the master.

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit 1610409 in lucene's branch refs/heads/main from Tomoko Uchida
https://gitbox.apache.org/repos/asf?p=lucene.git;h=1610409

LUCENE-9448: clean up unused start scripts for luke.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I cleaned up the obsoleted sh and bat scripts. Now luke is a standalone JAR - I am wondering if it makes sense that we provide luke as a FAT JAR; there is a gradle plugin to do so: https://imperceptiblethoughts.com/shadow/introduction/ (I have never used it, though.)

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I prefer if it's an explicit - JAR + proper manifest. This makes it essentially the same functionally (you can run it with java -jar luke.jar) and you can see which dependencies it needs/ uses. Fat jars are making things more obscure than they need to be.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Ok thank you @dweiss for your comment.

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

No fat jars, please! It is fine with shading sometimes to prevent incompatibilities, but not just to allow Java -jar.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Ok thank you @uschindler for your feedback, too.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

A question - how Luke will be distributed as of lucene 9.0.0.

Its MANIFEST says:

Main-Class: org.apache.lucene.luke.app.desktop.LukeMain
Class-Path: lucene-codecs-9.0.0-SNAPSHOT.jar lucene-backward-codecs-9.0.
 0-SNAPSHOT.jar lucene-analysis-icu-9.0.0-SNAPSHOT.jar lucene-analysis-k
 uromoji-9.0.0-SNAPSHOT.jar lucene-analysis-morfologik-9.0.0-SNAPSHOT.ja
 r lucene-analysis-nori-9.0.0-SNAPSHOT.jar lucene-analysis-opennlp-9.0.0
 -SNAPSHOT.jar lucene-analysis-phonetic-9.0.0-SNAPSHOT.jar lucene-analys
 is-smartcn-9.0.0-SNAPSHOT.jar lucene-analysis-stempel-9.0.0-SNAPSHOT.ja
 r lucene-suggest-9.0.0-SNAPSHOT.jar lucene-analysis-common-9.0.0-SNAPSH
 OT.jar lucene-queryparser-9.0.0-SNAPSHOT.jar lucene-highlighter-9.0.0-S
 NAPSHOT.jar lucene-sandbox-9.0.0-SNAPSHOT.jar lucene-queries-9.0.0-SNAP
 SHOT.jar lucene-misc-9.0.0-SNAPSHOT.jar lucene-memory-9.0.0-SNAPSHOT.ja
 r lucene-core-9.0.0-SNAPSHOT.jar log4j-core-2.13.2.jar icu4j-68.2.jar c
 ommons-codec-1.13.jar log4j-api-2.13.2.jar opennlp-tools-1.9.1.jar morf
 ologik-polish-2.1.5.jar morfologik-stemming-2.1.5.jar morfologik-fsa-2.
 1.5.jar morfologik-ukrainian-search-4.9.1.jar

In brief, this means we will distribute luke as a standalone app ("lucene/luke/build/distributions/lucene-luke-9.0.0-SNAPSHOT-standalone.tgz") that is separated from lucene binary package (Am I missing something)?

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I don't think there is any decision on this, Tomoko. What do you think would be more useful? I think a stand-alone package (download) with just Luke makes sense as it's a low-level tool. But I can live with it being part of the distribution too.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I would prefer a standalone Luke package to current Lucene bundled distribution, if "duplication of many jars" is acceptable. I think a  Swing app that contains minimum dependencies to run it makes sense for users (I mean, it is easy to understand to users who usually don't download lucene binary from the website but maven repositories by the build tools).

Meanwhile I'm not sure what changes are required on the release procedure and the download page... maybe an issue needs to be opened. ?

 

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit 2160d72 in lucene's branch refs/heads/main from Tomoko Uchida
https://gitbox.apache.org/repos/asf?p=lucene.git;h=2160d72

Revert "LUCENE-9448: clean up unused start scripts for luke."

This reverts commit 1610409.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I think so. This requires changes in how the distribution is assembled. I don't think there is much harm in having a separate artifact to download though.

@asfimport
Copy link
Author

asfimport commented May 27, 2021

Tomoko Uchida (@mocobeta) (migrated from JIRA)

I opened #11017 to explore the possibility, also reverted the premature previous commit.

@asfimport
Copy link
Author

asfimport commented Sep 16, 2021

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Just to note: I noticed "gradlew -p lucene/luke run" does not work on the latest main (w/ gradle 7.2) for me; I guess some follow-ups of #11029 are needed, I haven't examined the error yet.

> Task :lucene:luke:run FAILED
Launching Luke 9.0.0-SNAPSHOT right now...

FAILURE: Build failed with an exception.

* Where:
Build file '/mnt/hdd/repo/lucene/lucene/luke/build.gradle' line: 137

* What went wrong:
Execution failed for task ':lucene:luke:run'.
> Could not get unknown property 'JavaInstallationRegistry' for task ':lucene:luke:run' of type org.gradle.api.DefaultTask.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

I'll take a look, @mocobeta

@asfimport
Copy link
Author

asfimport commented Sep 16, 2021

ASF subversion and git services (migrated from JIRA)

Commit de45b68 in lucene's branch refs/heads/main from Dawid Weiss
https://gitbox.apache.org/repos/asf?p=lucene.git;h=de45b68

LUCENE-9448, #11029: fix Luke's launcher task.

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

Fixed, thank you for noticing this.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

@dweiss thank you, it works!

@asfimport
Copy link
Author

Adrien Grand (@jpountz) (migrated from JIRA)

Closing after the 9.0.0 release

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

1 participant