Compare existing and generated output 07/104807/3
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 9 Mar 2023 13:08:20 +0000 (14:08 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 9 Mar 2023 14:12:37 +0000 (15:12 +0100)
Before we overwrite a particular file we need to make sure that update
is actually needed. This adds the smarts to suppress such modifications
and notify BuildContext only when things change.

JIRA: YANGTOOLS-745
Change-Id: I17760f3342112343cd67f248d6374fd37710284d
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
plugin/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/GeneratorTask.java
plugin/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/YangToSourcesProcessor.java

index d5714545ae02931b7f971e12aae79c0177fc8726..eb5aa8cbc1da505ccd1ba21f8090e618a100feb3 100644 (file)
@@ -15,10 +15,12 @@ import com.google.common.collect.MultimapBuilder;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
 import java.util.List;
 import java.util.stream.Collectors;
 import org.apache.maven.project.MavenProject;
-import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.plugin.generator.api.FileGenerator;
 import org.opendaylight.yangtools.plugin.generator.api.FileGeneratorException;
@@ -27,8 +29,8 @@ import org.opendaylight.yangtools.plugin.generator.api.GeneratedFile;
 import org.opendaylight.yangtools.yang.parser.api.YangParserConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.sonatype.plexus.build.incremental.BuildContext;
 
-@NonNullByDefault
 final class GeneratorTask implements Identifiable<String> {
     private static final Logger LOG = LoggerFactory.getLogger(GeneratorTask.class);
 
@@ -49,15 +51,15 @@ final class GeneratorTask implements Identifiable<String> {
         return arg.getIdentifier();
     }
 
-    YangParserConfiguration parserConfig() {
+    @NonNull YangParserConfiguration parserConfig() {
         return parserConfig;
     }
 
-    FileGeneratorArg arg() {
+    @NonNull FileGeneratorArg arg() {
         return arg;
     }
 
-    String generatorName() {
+    @NonNull String generatorName() {
         return gen.getClass().getName();
     }
 
@@ -66,13 +68,14 @@ final class GeneratorTask implements Identifiable<String> {
      * model held in specified {@link ContextHolder}.
      *
      * @param project current Maven Project
+     * @param buildContext Incremental BuildContext
      * @param context model generation context
      * @return {@link FileState} for every generated file
      * @throws FileGeneratorException if the underlying generator fails
      * @throws IOException when a generated file cannot be written
      */
-    List<FileState> execute(final MavenProject project, final ContextHolder context)
-            throws FileGeneratorException, IOException {
+    @NonNull List<FileState> execute(final MavenProject project, final BuildContext buildContext,
+            final ContextHolder context) throws FileGeneratorException, IOException {
         final var access = new ProjectFileAccess(project, getIdentifier());
 
         // Step one: determine what files are going to be generated
@@ -119,13 +122,30 @@ final class GeneratorTask implements Identifiable<String> {
 
         // Step four: submit all code generation tasks (via parallelStream()) and wait for them to complete
         sw.reset().start();
-        final var result = dirs.values().parallelStream()
-                .map(WriteTask::generateFile)
-                .collect(Collectors.toList());
-        LOG.debug("Generated {} files in {}", result.size(), sw);
-
+        final var outputFiles = dirs.values().parallelStream()
+            .map(task -> {
+                try {
+                    return task.generateFile();
+                } catch (IOException e) {
+                    throw new IllegalStateException("Failed to generate " + task.file, e);
+                }
+            })
+            .collect(Collectors.toList());
+        LOG.debug("Generated {} files in {}", outputFiles.size(), sw);
+
+        // Step five: update maven project to include top-level directories
         access.updateMavenProject();
-        return result;
+
+        // Step six: extract FileState objects while notifying BuildContext of any files which have been changed
+        return outputFiles.stream()
+            .map(output -> {
+                final var state = output.state();
+                if (output.changed()) {
+                    buildContext.refresh(new File(state.path()));
+                }
+                return state;
+            })
+            .collect(Collectors.toList());
     }
 
     @Override
@@ -133,6 +153,16 @@ final class GeneratorTask implements Identifiable<String> {
         return MoreObjects.toStringHelper(this).add("generator", generatorName()).add("argument", arg).toString();
     }
 
+    /**
+     * A single file produced by {@link GeneratorTask}. It contains a {@link FileState} and an indication whether the
+     * file has been changed.
+     */
+    private record OutputFile(@NonNull FileState state, boolean changed) {
+        OutputFile {
+            requireNonNull(state);
+        }
+    }
+
     private static final class WriteTask {
         private final GeneratedFile file;
         private final File target;
@@ -142,14 +172,29 @@ final class GeneratorTask implements Identifiable<String> {
             this.file = requireNonNull(file);
         }
 
-        FileState generateFile() {
-            final FileState ret;
-            try {
-                ret = FileState.ofWrittenFile(target, file::writeBody);
-            } catch (IOException e) {
-                throw new IllegalStateException("Failed to generate file " + target, e);
+        OutputFile generateFile() throws IOException {
+            if (!target.isFile()) {
+                // Target file does not exist: just output body
+                return new OutputFile(FileState.ofWrittenFile(target, file::writeBody), true);
             }
-            return ret;
+
+            // Acquire existing file state
+            final var existingFile = FileState.ofFile(target);
+
+            // Write out the new output into a temporary file
+            final var tmpFile = FileState.ofWrittenFile(File.createTempFile("gen", null, target.getParentFile()),
+                file::writeBody);
+
+            // If file size and checksum matches just delete our output
+            final var tmpPath = Path.of(tmpFile.path());
+            if (existingFile.size() == tmpFile.size() && existingFile.crc32() == tmpFile.crc32()) {
+                Files.delete(tmpPath);
+                return new OutputFile(existingFile, false);
+            }
+
+            // Mismatch: move the temp file into place
+            Files.move(tmpPath, target.toPath(), StandardCopyOption.REPLACE_EXISTING);
+            return new OutputFile(new FileState(existingFile.path(), tmpFile.size(), tmpFile.crc32()), true);
         }
     }
 }
index 669825f6e2b44cf55b1996269d9fca6a9799ef2e..86f536b16263eaa077cee238903bb38b2c2179ad 100644 (file)
@@ -266,14 +266,12 @@ class YangToSourcesProcessor {
                 final var genSw = Stopwatch.createStarted();
                 final List<FileState> files;
                 try {
-                    files = factory.execute(project, holder);
+                    files = factory.execute(project, buildContext, holder);
                 } catch (FileGeneratorException | IOException e) {
                     throw new MojoFailureException(LOG_PREFIX + " Generator " + factory + " failed", e);
                 }
 
-                files.forEach(state -> buildContext.refresh(new File(state.path())));
                 outputFiles.addAll(files);
-
                 LOG.info("{} Sources generated by {}: {} in {}", LOG_PREFIX, factory.generatorName(), files.size(),
                     genSw);
             }