You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/02/06 15:34:03 UTC

[GitHub] [lucene-solr] mocobeta opened a new pull request #1242: LUCENE-9201: Port documentation-lint task to Gradle build

mocobeta opened a new pull request #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242
 
 
   # Description
   
   This PR adds an equivalent to "documentation-lint" to Gradle build.
   
   # Solution
   
   The `gradle/validation/documentation-lint.gradle` includes 
   - `documentationLint` task that supposed to be called from `precommit` task,
   - a root project level sub-task `checkBrokenLinks`, 
   - sub-project level sub-tasks `ecjJavadocLint`, `checkMissingJavadocsClass`, and `checkMissingJavadocsMethod`.
   
   # Note
   
   For now, Python linters - `checkBrokenLinks`,  `checkMissingJavadocsClass` and `checkMissingJavadocsMethod` - will fail because the Gradle-generated Javadocs seems to be slightly different to Ant-generated ones.
   e.g.; 
   - Javadoc directory structure: "ant documentation" generates "analyzers-common" docs dir for "analysis/common" module, but "gradlew javadoc" generates "analysis/common" for the same module. I think we can adjust the structure, but where is the suitable place to do so? 
   - Package summary: "ant documentation" uses "package.html" as package summary description, but "gradlew javadoc" ignores "package.html" (so some packages lacks summary description in "package-summary.html" when building javadocs by Gradle). We might be able to make Gradle Javadoc task to properly handle "package.html" files with some options. Or, should we replace all "package.html" with "package-info.java" at this time? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584631168
 
 
   Hi @mocobeta, 
   what's the problem with the source folder? Here it is: https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSet.html#org.gradle.api.tasks.SourceSet:java
   
   The second problem is the "joining" classpath: This won't work on windows (":" is only valid on Linux). With sourceSets, use the following method: `getAsPath()` https://docs.gradle.org/current/javadoc/org/gradle/api/file/FileCollection.html#getAsPath-- (also the compileClassPath for the test sourceset really also contains the classes from the main sourceset). 
   
   The problem may only be incorrect dependencies. The bug is that you have to define a separate task per sourceset. So don't add a global lintJavadocs path and instead just register a new task for each project named `sourceLint` (remove Javadocs from it, the javadocs in the ECJ call is obsolete, its just from former times. We now primarily use it to find obsolete imports) that depends on the per-sourceset `sourceLintMain`(autogenerated, see forbiddenapis source code) and `sourceLintTest`. I think you can more or less copypaste the whole task installation from forbiddenapis (see previous comments).
   
   If you then execute sourceLint from top-level it will execute the task for every project separately. You should also be able to call it separately for a single unit. Ideally that task should then be depended on each project's "check".
   
   I'd rewrite the whole thing - should I work on it? (I don't have much time, but spending too much time here in explaining what to do costs more time). IMHO, the current setup is very gradle-unlike. You'd never do it like that, feels like Ant. :-)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-585101615
 
 
   @dweiss thank you for your comments and the patch in Jira. 
   
   Let me close this PR for now and open another ones (in a week or so), to narrow down the scopes. 
   - source code linting by ECJ (unused imports check)
   - documentation linting by the python checkers: this may be further split into 
     - "missing javadoc check" (defined at each sub-project) and 
     - "broken links check" (defined at the root project to check inter-project links)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dweiss commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584679461
 
 
   Don't close the PR, Tomoko. I agree with Uwe that it should be split but what you did is a good start. I'll try to write up the ecj lint to give you an example what it can look like.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-583751044
 
 
   The task should just be defined for each sourceSet. Then tests and compile works automatically. Grafles will automatically add 2 tasks (one for each sourceSet): ecjLintMain and ecjLintTest (if you call it ecjLint base name). To set this up ask Gradle for current sourceSets and generate a task with automatic name based on SourceSet name. Classpath is provided gratis.
   
   See e.g Gradle internal tasks or forbiddenapis source code how those tasks should be declared. This easy seen here is not in line with the model behind Gradle (you define tasks per sourceSet, so it's extensible).
   
   sourceSet by the way also has source target and/or release version.
   Thi

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-583753281
 
 
   Here the forbiddenapis example how to setup a task per sourceSet: https://github.com/policeman-tools/forbidden-apis/blob/master/src/main/resources/de/thetaphi/forbiddenapis/gradle/plugin-init.groovy#L42

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] mocobeta closed pull request #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
mocobeta closed pull request #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584671311
 
 
   > I'd rewrite the whole thing - should I work on it? (I don't have much time, but spending too much time here in explaining what to do costs more time). IMHO, the current setup is very gradle-unlike. You'd never do it like that, feels like Ant. :-)
   
   > ECJ only task: new name sourceLint, defined for each sourceSet separately; check task depends on it.
   
   OK, should I close this PR and create another ones. It may takes time to make good ones for me, so the work would be suitable for someone who is familiar with gradle than me.
   
   > Creating the all-in-one documentation folder in lucene and solr root projects (including the index.html listing all projects).
   
   Just for your information, this is also discussed in JIRA: https://issues.apache.org/jira/browse/LUCENE-9201

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584631168
 
 
   Hi @mocobeta, 
   what's the problem with the source folder? Here it is https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSet.html#org.gradle.api.tasks.SourceSet:java
   
   The second problem is the "joning" classpath: This won't work on windows (":" is only valid on Linux). With sourceSets. Use the following method: `getAsPath()` https://docs.gradle.org/current/javadoc/org/gradle/api/file/FileCollection.html#getAsPath-- (also the complieClassPath for the test sourceset really also contains the classes from the main sourceset). The problem may only be incorrect dependencies. The bug is that you have to define a separate task per sourceset. So don't add a global lintJavadocs path and instead just register a new task for each project named "sourceLint" (remove Javadocs from it, the javadocs in the ECJ call is obsolete, its just from former times. We now primarily use it to find obsolete imports) that depends on 
   
   If you then execute sourceLint from top-level it will execute the task for every project separately. You should also be able to call it separately for a single unit. Ideally that task should then be depended on each project's "check".
   
   I'd rewrite the whole thing - should I work on it? (I don't have much time, but spending too much time here in explaining what to do costs more time). IMHO, the current setup is very gradle-unlike. You'd never do it like that, feels like Ant. :-)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-583751044
 
 
   The task should just be defined for each sourceSet. Then tests and compile works automatically. Grafles will automatically add 2 tasks (one for each sourceSet): ecjLintMain and ecjLintTest (if you call it ecjLint base name). To set this up ask Gradle for current sourceSets and generate a task with automatic name based on SourceSet name. Classpath is provided gratis.
   
   See e.g Gradle internal tasks or forbiddenapis source code how those tasks should be declared. This easy seen here is not in line with the model behind Gradle (you define tasks per sourceSet, so it's extensible, e.g. if we add new sourceSets when building multi-release jars for some modules).
   
   sourceSet by the way also has source target and/or release version.
   Thi

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dweiss commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-585103020
 
 
   Ok, sure thing. I'll create a sub-task on this issue and maybe try to push the ecj linter forward so that it is there as an example to copy from. Many things in gradle are not so obvious (although they are fairly clear once you soak in the basic concepts). If you have doubts or questions about how the code in the patch works please don't hesitate to ask.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584635945
 
 
   In short: We should split this PR:
   - ECJ only task: new name `sourceLint`, defined for each sourceSet separately, depending on `check` task.
   - Creating the all-in-one documentation folder in `lucene` and `solr` root projects (including the `index.html` listing all projects).
   - Adding the link-checker python on the all-in-one output.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584631168
 
 
   Hi @mocobeta, 
   what's the problem with the source folder? Here it is https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSet.html#org.gradle.api.tasks.SourceSet:java
   
   The second problem is the "joning" classpath: This won't work on windows (":" is only valid on Linux). With sourceSets. Use the following method: `getAsPath()` https://docs.gradle.org/current/javadoc/org/gradle/api/file/FileCollection.html#getAsPath-- (also the complieClassPath for the test sourceset really also contains the classes from the main sourceset). The problem may only be incorrect dependencies. The bug is that you have to define a separate task per sourceset. So don't add a global lintJavadocs path and instead just register a new task for each project named "sourceLint" (remove Javadocs from it, the javadocs in the ECJ call is obsolete, its just from former times. We now primarily use it to find obsolete imports) that depends on 
   
   If you then execute sourceLint from top-level it will execute the task for every project separately. You should also be able to call it separately for a single unit. Ideally that task should then be depended on each project's "check".
   
   I'd rewrite the whole thing, should I work on it. The current setup is very gradle-unlike. You'd never do it like that, feels like Ant. :-)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584635945
 
 
   In short: We should split this PR:
   - ECJ only task: new name `sourceLint`, defined for each sourceSet separately; `check` task depends on it.
   - Creating the all-in-one documentation folder in `lucene` and `solr` root projects (including the `index.html` listing all projects).
   - Adding the link-checker python on the all-in-one output.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584633315
 
 
   The `copyAllJavadocs` should be placed outside of linter.
   
   We will need this anyways, as the whole Javadocs are not structured by modules at the moment, we also copy them together in Ant (because it's published as one huge folder layout) on the Lucene and Solr web pages.
   
   We should add another task to collect all Javadocs for the lucene and also for the solr root projects, add the XSL-based index.html and so allow it to be published on website or Jenkins.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build

Posted by GitBox <gi...@apache.org>.
mocobeta commented on issue #1242: LUCENE-9201: Port documentation-lint task to Gradle build
URL: https://github.com/apache/lucene-solr/pull/1242#issuecomment-584574521
 
 
   Thanks @uschindler for your comments. I rewrote the task to use sourceSets instead of relying the assumption the target projects (for linting) have "src/java".
   However there is another problem, the Java source directory isn't available from SourceSet as far as I know. Actual source directory path is required when executing the ECJ.
   ```groovy
       // excerpt from the custom ECJ lint task
       project.plugins.withId('java', {
         project.sourceSets.each { sourceSet ->
           project.javaexec {
             classpath {
               project.rootProject.configurations.ecj.asPath
             }
             main = "org.eclipse.jdt.internal.compiler.batch.Main"
             args += [
               // Unfortunately, 'testCompileClasspath' is not available from sourceSet, so without the second term test classes cannot be compiled.
               "-classpath", sourceSet.compileClasspath.toList().join(':') + project.configurations.testCompileClasspath.findAll().join(":"),
               "-d", dstDir,
               "-encoding", "UTF-8",
               "-source", "11", // How this can be obtained from sourceSet or project?
               "-target", "11",
               "-nowarn",
               "-enableJavadoc",
               "-properties", "${project.rootProject.rootDir}/lucene/tools/javadoc/ecj.javadocs.prefs",
               "src/java" // ... or "src/test". How this can be obtained from sourceSet or project?
             ]
           }
         }
       })
   ```
   
   SourceSet has a property `allJava` that contains all Java source file, this is no help here.
   https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSet.html#org.gradle.api.tasks.SourceSet
   I might misses something, or another hack is required to identify the actual source directory path?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org