[ML] Include 3rd party C++ component notices

[droberts195]

The overall NOTICE file for the ML X-Pack module should
include the notices from the 3rd party C++ components as
well as the 3rd party Java components.

[jasontedor]

I left a suggestion. Also this build fails for me with:

> Task :distribution:archives:tar:checkMlCppNotice FAILED

FAILURE: Build failed with an exception.

* Where:
Build file '/home/jason/src/elastic/elasticsearch/distribution/archives/build.gradle' line: 233

* What went wrong:
Execution failed for task ':distribution:archives:tar:checkMlCppNotice'.
> expected [/home/jason/src/elastic/elasticsearch/distribution/archives/tar/build/tar-extracted/elasticsearch-7.0.0-alpha1-SNAPSHOT/modules/x-pack/x-pack-ml/NOTICE.txt] to contain [Apache log4cxx] but it did not

[jasontedor]

I think we can avoid adding this task to the task graph if the project is not tar nor zip? That way we do not have no-op tasks for oss-tar and oss-zip. Something like:

diff --git a/distribution/archives/build.gradle b/distribution/archives/build.gradle
index 5cfbaf95f09..278d79712ae 100644
--- a/distribution/archives/build.gradle
+++ b/distribution/archives/build.gradle
@@ -219,24 +219,25 @@ subprojects {
   }
   check.dependsOn checkNotice
 
-  task checkMlCppNotice {
-    dependsOn buildDist, checkExtraction
-    onlyIf toolExists
-    doLast {
-      if (project.name == 'zip' || project.name == 'tar') {
-        // this is just a small sample from the C++ notices, the idea being that if we've added these lines we've probably added all the required lines
-        final List<String> expectedLines = Arrays.asList("Apache log4cxx", "Boost Software License - Version 1.0 - August 17th, 2003")
-        final Path noticePath = archiveExtractionDir.toPath().resolve("elasticsearch-${VersionProperties.elasticsearch}/modules/x-pack/x-pack-ml/NOTICE.txt")
-        final List<String> actualLines = Files.readAllLines(noticePath)
-        for (final String expectedLine : expectedLines) {
-          if (actualLines.contains(expectedLine) == false) {
-            throw new GradleException("expected [${noticePath}] to contain [${expectedLine}] but it did not")
+  if (project.name == 'tar' || project.name == 'zip') {
+    task checkMlCppNotice {
+      dependsOn buildDist, checkExtraction
+      onlyIf toolExists
+      doLast {
+          // this is just a small sample from the C++ notices, the idea being that if we've added these lines we've probably added all the required lines
+          final List<String> expectedLines = Arrays.asList("Apache log4cxx", "Boost Software License - Version 1.0 - August 17th, 2003")
+          final Path noticePath = archiveExtractionDir.toPath().resolve("elasticsearch-${VersionProperties.elasticsearch}/modules/x-pack/x-pack-ml/NOTICE.txt")
+          final List<String> actualLines = Files.readAllLines(noticePath)
+          for (final String expectedLine : expectedLines) {
+            if (actualLines.contains(expectedLine) == false) {
+              throw new GradleException("expected [${noticePath}] to contain [${expectedLine}] but it did not")
+            }
           }
-        }
       }
     }
+    check.dependsOn checkMlCppNotice
   }
-  check.dependsOn checkMlCppNotice
+
 }
 
 /*****************************************************************************

[droberts195]

Thanks. I changed that in the 3rd commit.

[elasticmachine]

Pinging @elastic/es-core-infra

[rjernst]

Why do these need to be in an afterEvaluate?

[rjernst]

Hrm, I see why (generateNotice is added as a task in an afterEvaluate), but this is incredibly fragile (depends on the order afterEvaluates are run in). I will work on a change to simplify this using gradle's new Provider stuff.

[droberts195]

Also this build fails for me

This is because this line in the ML build.gradle is not having the intended effect:

    project.tasks.findByName('generateNotice').dependsOn extractNativeLicenses

I was hoping it would mean that if extractNativeLicenses was not up-to-date then generateNotice would also not be considered up-to-date. However, this is not what happens:

:x-pack:plugin:ml:extractNativeLicenses (Thread[Daemon worker,5,main]) started.

> Task :x-pack:plugin:ml:extractNativeLicenses
Task ':x-pack:plugin:ml:extractNativeLicenses' is not up-to-date because:
  Output property 'destinationDir' file /Users/dave/elasticsearch/x-pack/plugin/ml/build/platform/licenses has been removed.
  Output property 'destinationDir' file /Users/dave/elasticsearch/x-pack/plugin/ml/build/platform/licenses/apr-iconv-INFO.csv has been removed.
  Output property 'destinationDir' file /Users/dave/elasticsearch/x-pack/plugin/ml/build/platform/licenses/apr-iconv-LICENSE.txt has been removed.
:x-pack:plugin:ml:extractNativeLicenses (Thread[Daemon worker,5,main]) completed. Took 0.137 secs.
:x-pack:plugin:ml:generateNotice (Thread[Daemon worker,5,main]) started.

> Task :x-pack:plugin:ml:generateNotice UP-TO-DATE
Skipping task ':x-pack:plugin:ml:generateNotice' as it is up-to-date.
:x-pack:plugin:ml:generateNotice (Thread[Daemon worker,5,main]) completed. Took 0.0 secs.

This has been a problem since the code was originally introduced into 5.4, but we get away with it in the case where everything is built from clean, so release builds were OK.

@rjernst will you have time to switch the notice generation to use Gradle's new Provider stuff in time for the final 6.3.0 build candidate? If so then I'll wait for that before trying to sort out this dependency problem.

[rjernst]

@rjernst will you have time to switch the notice generation to use Gradle's new Provider stuff in time for the final 6.3.0 build candidate?

I will try to do this today.

[droberts195]

@jasontedor @rjernst I pushed a2ef1cb to get the overall notice to be regenerated if the ML C++ notices change. Since this is a blocker for 6.3 I thought it was worth putting an interim solution in place even if there will be something better in the long term. Please can you take another look?

[jasontedor]

This LGTM; you can merge as-is and we can follow-up later.

[jasontedor]

run gradle build tests

[rjernst]

That's a tricky hack, but ok for now. Simplifying the notice tasks to not be created in afterEvaluate has been difficult, sorry I haven't got it working yet. I will fix this more when I come up with a solution there.

[droberts195]

Thanks for the reviews @rjernst and @jasontedor. I agree it’s a hack but at least we’ll be including the necessary notice files in 6.3 and hopefully it can be replaced with something more elegant in the future.