Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 30, 2018
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c25fd92
[ML] Include 3rd party C++ component notices
droberts195 Apr 25, 2018
e6b73fd
Add a sanity check to the tests
droberts195 Apr 25, 2018
146b09c
Avoid empty tasks for OSS and test projects
droberts195 Apr 26, 2018
d935441
Merge branch 'master' into include_ml_cpp_notices
droberts195 Apr 30, 2018
a2ef1cb
generateNotice should not be up-to-date if extractNativeLicenses isn't
droberts195 Apr 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions distribution/archives/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,24 @@ subprojects {
}
check.dependsOn checkNotice

task checkMlCppNotice {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
+
 }
 
 /*****************************************************************************

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I changed that in the 3rd commit.

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")
}
}
}
}
}
check.dependsOn checkMlCppNotice
}

/*****************************************************************************
Expand Down
16 changes: 15 additions & 1 deletion x-pack/plugin/ml/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ artifacts {
testArtifacts testJar
}

task extractNativeLicenses(type: Copy) {
dependsOn configurations.nativeBundle
into "${buildDir}"
from {
project.zipTree(configurations.nativeBundle.singleFile)
}
include 'platform/licenses/**'
}
project.afterEvaluate {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these need to be in an afterEvaluate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Add an extra licenses directory to the combined notices
project.tasks.findByName('generateNotice').dependsOn extractNativeLicenses
project.tasks.findByName('generateNotice').licensesDir new File("${project.buildDir}/platform/licenses")
}

run {
plugin xpackModule('core')
}
Expand All @@ -85,7 +99,7 @@ task internalClusterTest(type: RandomizedTestingTask,
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test

// also add an "alias" task to make typing on the command line easier
Expand Down