From fae5b1ca0c76522b62a75efea30ec12fcdb08b09 Mon Sep 17 00:00:00 2001 From: Peter Donovan Date: Sat, 23 Mar 2024 17:55:44 -0700 Subject: [PATCH 1/5] Fix docker error handling. We were always returning false for whether errors occurred in federated docker builds, but we should be checking the context to see whether an error was reported. --- .../org/lflang/federated/generator/FedGenerator.java | 6 ++++-- .../main/java/org/lflang/generator/c/CGenerator.java | 11 +++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/lflang/federated/generator/FedGenerator.java b/core/src/main/java/org/lflang/federated/generator/FedGenerator.java index 445b1dacfc..f3a60f15da 100644 --- a/core/src/main/java/org/lflang/federated/generator/FedGenerator.java +++ b/core/src/main/java/org/lflang/federated/generator/FedGenerator.java @@ -203,7 +203,7 @@ public boolean doGenerate(Resource resource, LFGeneratorContext context) throws }); context.finish(Status.COMPILED, codeMapMap); - return false; + return context.getErrorReporter().getErrorsOccurred(); } /** @@ -218,9 +218,11 @@ private void buildUsingDocker(LFGeneratorContext context, List subCo dockerGen.writeDockerComposeFile(createDockerFiles(context, subContexts)); if (dockerGen.build()) { dockerGen.createLauncher(); + } else { + context.getErrorReporter().nowhere().error("Docker build failed."); } } catch (IOException e) { - context.getErrorReporter().nowhere().error("Unsuccessful Docker build."); + context.getErrorReporter().nowhere().error("Docker build failed due to invalid file system state."); } } diff --git a/core/src/main/java/org/lflang/generator/c/CGenerator.java b/core/src/main/java/org/lflang/generator/c/CGenerator.java index d8ba272455..eb3a21dc26 100644 --- a/core/src/main/java/org/lflang/generator/c/CGenerator.java +++ b/core/src/main/java/org/lflang/generator/c/CGenerator.java @@ -523,7 +523,10 @@ public void doGenerate(Resource resource, LFGeneratorContext context) { context.getMode()); context.finish(GeneratorResult.Status.COMPILED, null); } else if (dockerBuild.enabled()) { - buildUsingDocker(); + boolean success = buildUsingDocker(); + if (!success) { + context.unsuccessfulFinish(); + } } else { var cleanCode = code.removeLines("#line"); var cCompiler = new CCompiler(targetConfig, fileConfig, messageReporter, cppMode); @@ -557,7 +560,7 @@ public void doGenerate(Resource resource, LFGeneratorContext context) { } /** Create Dockerfiles and docker-compose.yml, build, and create a launcher. */ - private void buildUsingDocker() { + private boolean buildUsingDocker() { // Create docker file. var dockerCompose = new DockerComposeGenerator(context); var dockerData = getDockerGenerator(context).generateDockerData(); @@ -568,9 +571,13 @@ private void buildUsingDocker() { throw new RuntimeException("Error while writing Docker files", e); } var success = dockerCompose.build(); + if (!success) { + messageReporter.nowhere().error("Docker-compose build failed."); + } if (success && mainDef != null) { dockerCompose.createLauncher(); } + return success; } private void generateCodeFor(String lfModuleName) throws IOException { From 9fff2a7f7016d0d85f3bc54b0575f85d4e94296d Mon Sep 17 00:00:00 2001 From: Peter Donovan Date: Sat, 23 Mar 2024 17:59:08 -0700 Subject: [PATCH 2/5] Update submodule to include Dockerfile fix --- core/src/main/resources/lib/c/reactor-c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/lib/c/reactor-c b/core/src/main/resources/lib/c/reactor-c index 7427d987d7..8158f7c01e 160000 --- a/core/src/main/resources/lib/c/reactor-c +++ b/core/src/main/resources/lib/c/reactor-c @@ -1 +1 @@ -Subproject commit 7427d987d725111141139819c5341921edd0fef1 +Subproject commit 8158f7c01ee3db93e7094297862b0d454f9cbc15 From f540df324eaee2c23ed3bcfd978140060e96e58a Mon Sep 17 00:00:00 2001 From: Peter Donovan Date: Sat, 23 Mar 2024 19:10:43 -0700 Subject: [PATCH 3/5] Fix code duplication problem --- .../federated/generator/FedGenerator.java | 8 ++++-- .../generator/docker/RtiDockerGenerator.java | 28 +++++++------------ core/src/main/resources/lib/c/reactor-c | 2 +- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/lflang/federated/generator/FedGenerator.java b/core/src/main/java/org/lflang/federated/generator/FedGenerator.java index f3a60f15da..2d06f21436 100644 --- a/core/src/main/java/org/lflang/federated/generator/FedGenerator.java +++ b/core/src/main/java/org/lflang/federated/generator/FedGenerator.java @@ -222,7 +222,10 @@ private void buildUsingDocker(LFGeneratorContext context, List subCo context.getErrorReporter().nowhere().error("Docker build failed."); } } catch (IOException e) { - context.getErrorReporter().nowhere().error("Docker build failed due to invalid file system state."); + context + .getErrorReporter() + .nowhere() + .error("Docker build failed due to invalid file system state."); } } @@ -239,8 +242,7 @@ private void prepareRtiBuildEnvironment(LFGeneratorContext context) { try { Files.createDirectories(dest); // 2. Copy reactor-c source files into it - FileUtil.copyFromClassPath("/lib/c/reactor-c/core", dest, true, false); - FileUtil.copyFromClassPath("/lib/c/reactor-c/include", dest, true, false); + FileUtil.copyFromClassPath("/lib/c/reactor-c", dest, true, true); // 3. Generate a Dockerfile for the rti new RtiDockerGenerator(context).generateDockerData(dest).writeDockerFile(); } catch (IOException e) { diff --git a/core/src/main/java/org/lflang/generator/docker/RtiDockerGenerator.java b/core/src/main/java/org/lflang/generator/docker/RtiDockerGenerator.java index fb863eed4c..d281cc896c 100644 --- a/core/src/main/java/org/lflang/generator/docker/RtiDockerGenerator.java +++ b/core/src/main/java/org/lflang/generator/docker/RtiDockerGenerator.java @@ -1,5 +1,9 @@ package org.lflang.generator.docker; +import java.io.BufferedReader; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.util.stream.Collectors; import org.lflang.generator.LFGeneratorContext; /** @@ -15,24 +19,12 @@ public RtiDockerGenerator(LFGeneratorContext context) { @Override protected String generateDockerFileContent() { - return """ - # Docker file for building the image of the rti - FROM %s - COPY core /reactor-c/core - COPY include /reactor-c/include - WORKDIR /reactor-c/core/federated/RTI - %s - RUN rm -rf build && \\ - mkdir build && \\ - cd build && \\ - cmake ../ && \\ - make && \\ - make install - - # Use ENTRYPOINT not CMD so that command-line arguments go through - ENTRYPOINT ["RTI"] - """ - .formatted(baseImage(), generateRunForBuildDependencies()); + InputStream stream = + RtiDockerGenerator.class.getResourceAsStream( + "/lib/c/reactor-c/core/federated/RTI/rti.Dockerfile"); + return new BufferedReader(new InputStreamReader(stream)) + .lines() + .collect(Collectors.joining("\n")); } @Override diff --git a/core/src/main/resources/lib/c/reactor-c b/core/src/main/resources/lib/c/reactor-c index 8158f7c01e..2a3c2575be 160000 --- a/core/src/main/resources/lib/c/reactor-c +++ b/core/src/main/resources/lib/c/reactor-c @@ -1 +1 @@ -Subproject commit 8158f7c01ee3db93e7094297862b0d454f9cbc15 +Subproject commit 2a3c2575be15f62b1229f8d8967b03d7fb16887a From c9fca22e8e6f0abb78cd03ce2913438f5beb7236 Mon Sep 17 00:00:00 2001 From: Peter Donovan Date: Sat, 23 Mar 2024 19:26:29 -0700 Subject: [PATCH 4/5] Fix unrelated bug uncovered by testing fix --- .../org/lflang/generator/docker/TSDockerGenerator.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/lflang/generator/docker/TSDockerGenerator.java b/core/src/main/java/org/lflang/generator/docker/TSDockerGenerator.java index cdb6ddcf4b..e22d9a3e86 100644 --- a/core/src/main/java/org/lflang/generator/docker/TSDockerGenerator.java +++ b/core/src/main/java/org/lflang/generator/docker/TSDockerGenerator.java @@ -17,11 +17,11 @@ public TSDockerGenerator(LFGeneratorContext context) { /** Return the content of the docker file for [tsFileName]. */ public String generateDockerFileContent() { return """ - |FROM %s - |WORKDIR /linguafranca/$name - |%s - |COPY . . - |ENTRYPOINT ["node", "dist/%s.js"] + FROM %s + WORKDIR /linguafranca/$name + %s + COPY . . + ENTRYPOINT ["node", "dist/%s.js"] """ .formatted(baseImage(), generateRunForBuildDependencies(), context.getFileConfig().name); } From 9296f07f4f8318462438e8f62ff7d7f9f636dcd7 Mon Sep 17 00:00:00 2001 From: Peter Donovan Date: Thu, 28 Mar 2024 16:18:33 -0700 Subject: [PATCH 5/5] Fix another unrelated bug uncovered by testing fix Note how non-obvious it is that this is a bug or even why the fix works. This is a consequence of reliance on implicit reliance on big chunks of shared mutable state. The resource is modified; the context is computed from the resource; therefore, the resource has to be modified before the context is computed. --- .../java/org/lflang/tests/TestBase.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/TestBase.java b/core/src/testFixtures/java/org/lflang/tests/TestBase.java index 6923149f07..fb57ea3691 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestBase.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestBase.java @@ -381,6 +381,13 @@ private void prepare(LFTest test, Transformer transformer, Configurator configur FileConfig.findPackageRoot(test.getSrcPath(), s -> {}) .resolve(FileConfig.DEFAULT_SRC_GEN_DIR) .toString()); + + // Update the test by applying the transformation. + if (transformer != null) { + if (!transformer.transform(resource)) { + throw new TestError("Test transformation unsuccessful.", Result.TRANSFORM_FAIL); + } + } var context = new MainContext( LFGeneratorContext.Mode.STANDALONE, @@ -391,13 +398,6 @@ private void prepare(LFTest test, Transformer transformer, Configurator configur fileAccess, fileConfig -> new DefaultMessageReporter()); - // Update the test by applying the transformation. - if (transformer != null) { - if (!transformer.transform(resource)) { - throw new TestError("Test transformation unsuccessful.", Result.TRANSFORM_FAIL); - } - } - // Reload the context because properties may have changed as part of the transformation. test.loadContext(context);