Generate py from protos and correctly mangle python modules (#2667)

Fixes #1073

buildSrc/src/main/groovy/io/deephaven/tools/docker/DiffTask.groovy

@@ -0,0 +1,118 @@
+package io.deephaven.tools.docker
+
+import groovy.transform.CompileStatic
+import org.gradle.api.Action
+import org.gradle.api.DefaultTask
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileVisitDetails
+import org.gradle.api.internal.file.FileLookup
+import org.gradle.api.provider.Property
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.util.PatternFilterable
+import org.gradle.api.tasks.util.PatternSet
+
+import javax.inject.Inject
+import java.nio.file.Path
+
+/**
+ * Compares the files in two sets of contents, failing if there are differences. Intended as a
+ * counterpart to the Synx task, to allow failing the build if certain sources don't match their
+ * expected values.
+ */
+@CompileStatic
+abstract class DiffTask extends DefaultTask {
+    // This is an Object because Gradle doesn't define an interface for getAsFileTree(), so we
+    // will resolve it when we execute the task. This allows us to read from various sources,
+    // such as configurations.
+    @Input
+    abstract Property<Object> getExpectedContents()
+    // In contrast, this is assumed to be a source directory, to easily allow some Sync action
+    // to easily be the "fix this mistake" counterpart to this task
+    @Input
+    abstract DirectoryProperty getActualContents()
+
+    private final PatternSet ignoreInActual = new PatternSet();
+
+    /**
+     * Marks some contents as not needing to be diff'd, like Sync's "preserve". Counter-intuitively,
+     * this usually requires an "exclude" to specify only a specific subdirectory should be diff'd.
+     */
+    DiffTask ignore(Action<? super PatternFilterable> action) {
+        action.execute(this.ignoreInActual);
+        return this;
+    }
+
+    /**
+     * Human readable name of the task that should be run to correct any errors detected by this task.
+     */
+    @Input
+    abstract Property<String> getGenerateTask()
+
+    @Inject
+    protected FileLookup getFileLookup() {
+        throw new UnsupportedOperationException();
+    }
+
+    @TaskAction
+    void diff() {
+        def resolver = getFileLookup().getFileResolver(getActualContents().asFile.get())
+        // for each file in the generated go output, make sure it exists and matches contents
+        Set<Path> changed = []
+        Set<Path> missing = []
+        // build this list before we traverse, then remove as we go, to represent files that shouldn't exist
+        Set<Path> existingFiles = []
+
+        def ignoreSpec = ignoreInActual.getAsSpec()
+        getActualContents().asFileTree.visit { FileVisitDetails details ->
+            if (ignoreSpec.isSatisfiedBy(details)) {
+                return;
+            }
+            if (details.isDirectory()) {
+                return;
+            }
+            existingFiles.add(details.file.toPath());
+        }
+
+        project.files(getExpectedContents().get()).asFileTree.visit { FileVisitDetails details ->
+            if (details.isDirectory()) {
+                return;
+            }
+
+            // note the relative path of each generated file
+            def pathString = details.relativePath.pathString
+
+            def sourceFile = resolver.resolve(pathString)
+            // if the file does not exist in our source dir, add an error
+            if (!sourceFile.exists()) {
+                missing.add(sourceFile.toPath())
+            } else {
+                // remove this from the "existing" collection so we can detect extra files later
+                existingFiles.remove(sourceFile.toPath())
+
+                // verify that the contents match
+                if (sourceFile.text != details.file.text) {
+                    changed.add(sourceFile.toPath())
+                }
+            }
+        }
+        if (!changed.isEmpty() || !missing.isEmpty() || !existingFiles.isEmpty()) {
+            logger.error("Sources do not match expected files:")
+            changed.each {
+                logger.error("File has changes: $it")
+            }
+            missing.each {
+                logger.error("File is missing: $it")
+            }
+            existingFiles.each {
+                logger.error("File should not exist: $it")
+            }
+            if (generateTask.isPresent()) {
+                throw new RuntimeException("Sources do not match expected, re-run ${generateTask.get()}")
+            } else {
+                throw new RuntimeException("Sources do not match expected");
+            }
+
+        }
+    }
+}

proto/proto-backplane-grpc/build.gradle

@@ -17,6 +17,9 @@ configurations {
 
   // automatically pick up the standard junit version
   testCompile.extendsFrom junit
+
+  js {}
+  python {}
 }
 
 dependencies {
@@ -76,6 +79,18 @@ TaskProvider<Task> generateProtobuf = Docker.registerDockerTask(project, 'genera
   }
 }
 
+// Provide js, go, python output as distinct artifacts - the java output will be consumed in this project by
+// declaring it as a new sourceset, see below. This could definitely be more precisely stated, but given that
+// all are generated in the same docker image creation step, we don't need to get too clever here.
+artifacts {
+  js(layout.buildDirectory.dir('generated/source/proto/main/js')) {
+    builtBy generateProtobuf
+  }
+  python(layout.buildDirectory.dir('generated/source/proto/main/python')) {
+    builtBy generateProtobuf
+  }
+}
+
 
 compileJava.dependsOn generateProtobuf
 

proto/raw-js-openapi/build.gradle

@@ -3,16 +3,21 @@ plugins {
     id 'io.deephaven.project.register'
 }
 
-evaluationDependsOn ':proto'
-evaluationDependsOn ':proto:proto-backplane-grpc'
 evaluationDependsOn Docker.registryProject('node')
 
-def backplaneProject = project(':proto:proto-backplane-grpc')
+configurations {
+    js
+}
+
+dependencies {
+    js project(path: ':proto:proto-backplane-grpc', configuration: 'js')
+}
+
 def webpackSourcesLocation = layout.buildDirectory.dir("${buildDir}/dhapi")
 
 Docker.registerDockerTask(project, 'webpackSources') {
     copyIn {
-        from(backplaneProject.tasks.findByName('generateProtobuf').outputs.files) {
+        from(configurations.js) {
             // note: we are only copying the JS and not TS files.
             include '**/*.js'
             into 'raw-js-openapi/build/js-src'
@@ -41,10 +46,6 @@ Docker.registerDockerTask(project, 'webpackSources') {
     }
 }
 
-configurations {
-    js
-}
-
 artifacts {
     js(webpackSourcesLocation) {
         builtBy webpackSources

py/client/build.gradle

@@ -4,18 +4,78 @@ import com.bmuschko.gradle.docker.tasks.container.DockerRemoveContainer
 import com.bmuschko.gradle.docker.tasks.network.DockerCreateNetwork
 import com.bmuschko.gradle.docker.tasks.network.DockerRemoveNetwork
 import io.deephaven.tools.docker.WaitForHealthyContainer
+import io.deephaven.tools.docker.DiffTask
 
 plugins {
     id 'com.bmuschko.docker-remote-api'
     id 'io.deephaven.python-wheel'
 }
 
+evaluationDependsOn Docker.registryProject('python')
+
 wheel {
     src 'pydeephaven'
     src 'examples'
     src 'tests'
 }
 
+configurations {
+    python {}
+}
+
+dependencies {
+    python project(path: ':proto:proto-backplane-grpc', configuration: 'python')
+}
+
+def generatedProtoDir = layout.buildDirectory.dir('generatedProto')
+
+def updatePyNamespaces = tasks.register('updatePyNamespaces', Sync) {
+    from(configurations.python) {
+        // change directory from deephaven to pydeephaven
+        eachFile { it.path = 'pydeephaven/' + it.path.substring('deephaven/'.length()) }
+        // rewrite specific imports/namespaces from deephaven to pydeephaven
+        filter { contents ->
+            def scanner = new Scanner(contents)
+            def lines = []
+            while (scanner.hasNextLine()) {
+                def line = scanner.nextLine();
+                if (line.trim().startsWith('\'__module__\' :')) {
+                    line = line.replace('\'deephaven.', '\'pydeephaven.')
+                } else if (line.startsWith('from deephaven.proto')) {
+                    line = line.replace('from deephaven.proto', 'from pydeephaven.proto')
+                }
+                lines.add(line)
+            }
+            return lines.join(System.lineSeparator());
+        }
+    }
+
+    into generatedProtoDir
+}
+
+def compare = tasks.register('compareProtobuf', DiffTask) {
+    dependsOn updatePyNamespaces
+
+    expectedContents.set generatedProtoDir.get().dir('pydeephaven/proto')
+    actualContents.set layout.projectDirectory.dir('pydeephaven/proto')
+    generateTask.set ':py-client:updateProto'
+    ignore {
+        include '__init__.py'
+    }
+}
+// fail a "check" build if these are out of date
+tasks.getByName('quick').dependsOn(compare)
+
+def updateProtobuf = tasks.register('updateProtobuf', Sync) {
+    dependsOn(updatePyNamespaces)
+    finalizedBy compare
+    from generatedProtoDir.get().dir('pydeephaven/proto')
+    into layout.projectDirectory.dir('pydeephaven/proto')
+    preserve {
+        include '__init__.py'
+    }
+}
+
 // Start up a docker container for the grpc server, then run pydeephaven test
 evaluationDependsOn(':docker-server')
 String randomSuffix = UUID.randomUUID().toString();