ronantakizawa/codereview-bench huggingface dataset:
Repository: GoogleContainerTools/jib
File: jib-core/src/main/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilder.java (Java)

Code before:
              layerEntry.getSourceFile(), layerEntry.getExtractionPath().toString());

      // Sets the entry's permissions by masking out the permission bits from the entry's mode (the
      // lowest 9 bits) then using a bitwise OR to set them to the layerEntry's permissions.
      entry.setMode((entry.getMode() & ~0777) | layerEntry.getPermissions().getPermissionBits());
      entry.setModTime(layerEntry.getModificationTime().toEpochMilli());
      setUserAndGroup(entry, layerEntry);

      uniqueTarArchiveEntries.add(entry);
    }

    // Gets the entries sorted by extraction path.
    List<TarArchiveEntry> sortedFilesystemEntries = uniqueTarArchiveEntries.getSortedEntries();

    Set<String> names = new HashSet<>();

    // Adds all the files to a tar stream.
    TarStreamBuilder tarStreamBuilder = new TarStreamBuilder();
    for (TarArchiveEntry entry : sortedFilesystemEntries) {
      Verify.verify(!names.contains(entry.getName()));
      names.add(entry.getName());

      tarStreamBuilder.addTarArchiveEntry(entry);
    }

    return Blobs.from(tarStreamBuilder::writeAsTarArchiveTo, true);
  }
}


Review comment (suggestion):
Just noticed that generally speaking `TarStreamBuilder::writeAsTarArchiveTo` is not retryable, as non-retryable `Blob`s [can be added](https://github.com/GoogleContainerTools/jib/blob/1fd6b62c90bb96a53fdc719265c4224157810361/jib-core/src/main/java/com/google/cloud/tools/jib/tar/TarStreamBuilder.java#L92) to it.
```suggestion
    return Blobs.from(tarStreamBuilder::writeAsTarArchiveTo, false);
```

Code after:
              layerEntry.getSourceFile(), layerEntry.getExtractionPath().toString());

      // Sets the entry's permissions by masking out the permission bits from the entry's mode (the
      // lowest 9 bits) then using a bitwise OR to set them to the layerEntry's permissions.
      entry.setMode((entry.getMode() & ~0777) | layerEntry.getPermissions().getPermissionBits());
      entry.setModTime(layerEntry.getModificationTime().toEpochMilli());
      setUserAndGroup(entry, layerEntry);

      uniqueTarArchiveEntries.add(entry);
    }

    // Gets the entries sorted by extraction path.
    List<TarArchiveEntry> sortedFilesystemEntries = uniqueTarArchiveEntries.getSortedEntries();

    Set<String> names = new HashSet<>();

    // Adds all the files to a tar stream.
    TarStreamBuilder tarStreamBuilder = new TarStreamBuilder();
    for (TarArchiveEntry entry : sortedFilesystemEntries) {
      Verify.verify(!names.contains(entry.getName()));
      names.add(entry.getName());

      tarStreamBuilder.addTarArchiveEntry(entry);
    }

    return Blobs.from(tarStreamBuilder::writeAsTarArchiveTo, false);
  }
}
Repository: GoogleContainerTools/jib
File: jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Build.java (Java)

Code before:
      if (!Files.isReadable(buildFile)) {
        logger.log(
            LogEvent.Level.ERROR,
            "The Build File YAML either does not exist or cannot be opened for reading: "
                + buildFile);
        return 1;
      }
      if (!Files.isRegularFile(buildFile)) {
        logger.log(LogEvent.Level.ERROR, "Build File YAML path is a not a file: " + buildFile);
        return 1;
      }

      CacheDirectories cacheDirectories = CacheDirectories.from(commonCliOptions, contextRoot);
      Containerizer containerizer = Containerizers.from(commonCliOptions, logger, cacheDirectories);

      JibContainerBuilder containerBuilder =
          BuildFiles.toJibContainerBuilder(contextRoot, buildFile, this, commonCliOptions, logger);

      // Enable registry mirrors
      Multimaps.asMap(globalConfig.getRegistryMirrors()).forEach(containerizer::addRegistryMirrors);

      JibContainer jibContainer = containerBuilder.containerize(containerizer);
      JibCli.writeImageJson(commonCliOptions.getImageJsonPath(), jibContainer);
    } catch (Exception ex) {
      JibCli.logTerminatingException(logger, ex, commonCliOptions.isStacktrace());
      Thread.currentThread().interrupt();
      return 1;
    } finally {
      JibCli.finishUpdateChecker(logger, updateCheckFuture);
      executor.shutDownAndAwaitTermination(Duration.ofSeconds(3));
    }
    return 0;
  }
}


Review comment (suggestion):
I think we should interrupt only when catching `InterruptedException`, not for all exceptions.

Code after:
      if (!Files.isReadable(buildFile)) {
        logger.log(
            LogEvent.Level.ERROR,
            "The Build File YAML either does not exist or cannot be opened for reading: "
                + buildFile);
        return 1;
      }
      if (!Files.isRegularFile(buildFile)) {
        logger.log(LogEvent.Level.ERROR, "Build File YAML path is a not a file: " + buildFile);
        return 1;
      }

      CacheDirectories cacheDirectories = CacheDirectories.from(commonCliOptions, contextRoot);
      Containerizer containerizer = Containerizers.from(commonCliOptions, logger, cacheDirectories);

      JibContainerBuilder containerBuilder =
          BuildFiles.toJibContainerBuilder(contextRoot, buildFile, this, commonCliOptions, logger);

      // Enable registry mirrors
      Multimaps.asMap(globalConfig.getRegistryMirrors()).forEach(containerizer::addRegistryMirrors);

      JibContainer jibContainer = containerBuilder.containerize(containerizer);
      JibCli.writeImageJson(commonCliOptions.getImageJsonPath(), jibContainer);
    } catch (InterruptedException ex) {
      JibCli.logTerminatingException(logger, ex, commonCliOptions.isStacktrace());
      Thread.currentThread().interrupt();
      return 1;
    } catch (Exception ex) {
      JibCli.logTerminatingException(logger, ex, commonCliOptions.isStacktrace());
      return 1;
    } finally {
      JibCli.finishUpdateChecker(logger, updateCheckFuture);
      executor.shutDownAndAwaitTermination(Duration.ofSeconds(3));
    }
    return 0;
  }
}
Repository: GoogleContainerTools/jib
File: jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java (Java)

Code before:
    return exposedPorts;
  }

  @Nullable
  public ImmutableSet<AbsoluteUnixPath> getVolumes() {
    return volumes;
  }

  @Nullable
  public String getUser() {
    return user;
  }

  @Nullable
  public ImmutableMap<String, String> getLabels() {
    return labels;
  }

  @Nullable
  public AbsoluteUnixPath getWorkingDirectory() {
    return workingDirectory;
  }

  public boolean isPlatformTag() {
    return platformTag;
  }

  @Override
  @VisibleForTesting
  public boolean equals(Object other) {
    if (this == other) {
      return true;
    }
    if (!(other instanceof ContainerConfiguration)) {
      return false;
    }
    ContainerConfiguration otherContainerConfiguration = (ContainerConfiguration) other;
    return platforms.equals(otherContainerConfiguration.platforms)
        && creationTime.equals(otherContainerConfiguration.creationTime)
        && Objects.equals(entrypoint, otherContainerConfiguration.entrypoint)
        && Objects.equals(programArguments, otherContainerConfiguration.programArguments)
        && Objects.equals(environmentMap, otherContainerConfiguration.environmentMap)
        && Objects.equals(exposedPorts, otherContainerConfiguration.exposedPorts)
        && Objects.equals(labels, otherContainerConfiguration.labels)
        && Objects.equals(user, otherContainerConfiguration.user)
        && Objects.equals(workingDirectory, otherContainerConfiguration.workingDirectory);
  }

  @Override
  @VisibleForTesting
  public int hashCode() {

Review comment (suggestion):
```suggestion
  public boolean isPlatformTagEnabled() {
    return platformTag;
  }
```

Code after:
  }

  @Nullable
  public AbsoluteUnixPath getWorkingDirectory() {
    return workingDirectory;
  }

  @Override
  @VisibleForTesting
  public boolean equals(Object other) {
    if (this == other) {
      return true;
    }
    if (!(other instanceof ContainerConfiguration)) {
      return false;
    }
    ContainerConfiguration otherContainerConfiguration = (ContainerConfiguration) other;
    return platforms.equals(otherContainerConfiguration.platforms)
        && creationTime.equals(otherContainerConfiguration.creationTime)
        && Objects.equals(entrypoint, otherContainerConfiguration.entrypoint)
        && Objects.equals(programArguments, otherContainerConfiguration.programArguments)
        && Objects.equals(environmentMap, otherContainerConfiguration.environmentMap)
        && Objects.equals(exposedPorts, otherContainerConfiguration.exposedPorts)
        && Objects.equals(labels, otherContainerConfiguration.labels)
        && Objects.equals(user, otherContainerConfiguration.user)
        && Objects.equals(workingDirectory, otherContainerConfiguration.workingDirectory);
  }

  @Override
  @VisibleForTesting
  public int hashCode() {
    return Objects.hash(
        platforms,
        creationTime,
        entrypoint,
        programArguments,
        environmentMap,
        exposedPorts,
        labels,
        user,
        workingDirectory);
  }
}
Repository: GoogleContainerTools/jib
File: jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/JibPluginConfiguration.java (Java)

Code before:
  protected Set<JibMavenPluginExtension<?>> getInjectedPluginExtensions() {
    return injectedPluginExtensions;
  }

  protected MavenSession getSession() {
    return Preconditions.checkNotNull(session);
  }

  protected MavenProject getProject() {
    return Preconditions.checkNotNull(project);
  }

  protected void checkJibVersion() throws MojoExecutionException {
    Preconditions.checkNotNull(descriptor);
    MojoCommon.checkJibVersion(descriptor);
  }

  /**
   * Gets the specified platforms.
   *
   * @return the specified platforms
   */
  List<PlatformParameters> getPlatforms() {
    String property = getProperty(PropertyNames.FROM_PLATFORMS);
    if (property != null) {
      return Stream.of(property.split(","))
          .map(PlatformParameters::ofString)
          .collect(Collectors.toList());
    }
    return from.platforms;
  }

  /**
   * Gets the base image reference.
   *
   * @return the configured base image reference
   */
  @Nullable
  String getBaseImage() {
    String property = getProperty(PropertyNames.FROM_IMAGE);
    if (property != null) {
      return property;
    }
    return from.image;
  }

  /**
   * Gets the base image credential helper.
   *
   * @return the configured base image credential helper name
   */

Review comment (suggestion):
For consistency, let's use `ConfigurationPropertyValidator.parseListProperty(property).map(...)...`.

Code after:
  @Inject private Set<JibMavenPluginExtension<?>> injectedPluginExtensions = Collections.emptySet();

  protected Set<JibMavenPluginExtension<?>> getInjectedPluginExtensions() {
    return injectedPluginExtensions;
  }

  protected MavenSession getSession() {
    return Preconditions.checkNotNull(session);
  }

  protected MavenProject getProject() {
    return Preconditions.checkNotNull(project);
  }

  protected void checkJibVersion() throws MojoExecutionException {
    Preconditions.checkNotNull(descriptor);
    MojoCommon.checkJibVersion(descriptor);
  }

  /**
   * Gets the specified platforms.
   *
   * @return the specified platforms
   */
  List<PlatformParameters> getPlatforms() {
    String property = getProperty(PropertyNames.FROM_PLATFORMS);
    if (property != null) {
      return ConfigurationPropertyValidator.parseListProperty(property).stream()
          .map(PlatformParameters::of)
          .collect(Collectors.toList());
    }
    return from.platforms;
  }

  /**
   * Gets the base image reference.
   *
   * @return the configured base image reference
   */
  @Nullable
  String getBaseImage() {
    String property = getProperty(PropertyNames.FROM_IMAGE);
    if (property != null) {
      return property;
    }
    return from.image;
  }

  /**
   * Gets the base image credential helper.
   *
Repository: GoogleContainerTools/jib
File: kokoro/presubmit.sh (Shell)

Code before:
#!/bin/bash

set -o errexit
set -o xtrace

gcloud components install docker-credential-gcr

# Docker service does not run by default in Big Sur but can be started with the following commands.
if [ "${KOKORO_JOB_CLUSTER}" = "MACOS_EXTERNAL" ]; then
docker-machine ls
docker-machine start default
docker-machine ssh default "echo '{ \"insecure-registries\":[\"192.168.99.104:5000\"] }' | sudo tee /etc/docker/daemon.json "
docker-machine ssh default "echo 'DOCKER_OPTS=\"--config-file=/etc/docker/daemon.json\"' | sudo tee -a /var/lib/boot2docker/profile "
docker-machine ssh default "sudo /etc/init.d/docker restart"
docker-machine env default
eval "$(docker-machine env default)"
export DOCKER_IP="$(docker-machine ip default)"
echo $DOCKER_IP

export JAVA_HOME="/Library/Java/JavaVirtualMachines/jdk-8-latest/Contents/Home"
export PATH=$JAVA_HOME/bin:$PATH
echo $JAVA_HOME
fi

# Stops any left-over containers.
docker stop $(docker ps --all --quiet) || true
docker kill $(docker ps --all --quiet) || true

cd github/jib

# we only run integration tests on jib-core for presubmit
./gradlew clean build :jib-core:integrationTest --info --stacktrace


Review comment (suggestion):
In the end, this should probably be
```suggestion
docker-machine ssh default "echo '{ \"insecure-registries\":[\"$DOCKER_IP:5000\"] }' | sudo tee /etc/docker/daemon.json "
```
and come after the `export DOCKER_IP` below.

Also, 5000 is the usual port for a local registry, but I remember we also use 6000. I wonder if we can omit the port part or have a way to specify any ports.

Code after:
#!/bin/bash

set -o errexit
set -o xtrace

gcloud components install docker-credential-gcr

# Docker service does not run by default in Big Sur but can be started with the following commands.
if [ "${KOKORO_JOB_CLUSTER}" = "MACOS_EXTERNAL" ]; then
source github/jib/kokoro/docker_setup.sh
fi

# Stops any left-over containers.
docker stop $(docker ps --all --quiet) || true
docker kill $(docker ps --all --quiet) || true

cd github/jib

# we only run integration tests on jib-core for presubmit
./gradlew clean build :jib-core:integrationTest --info --stacktrace
Repository: GoogleContainerTools/jib
File: kokoro/continuous.sh (Shell)

Code before:
docker-machine ssh default "echo '{ \"insecure-registries\":[\"$DOCKER_IP:5000\", \"$DOCKER_IP:8080\"] }' | sudo tee /etc/docker/daemon.json "
docker-machine ssh default "echo 'DOCKER_OPTS=\"--config-file=/etc/docker/daemon.json\"' | sudo tee -a /var/lib/boot2docker/profile "
docker-machine ssh default "sudo /etc/init.d/docker restart"
docker-machine env default
eval "$(docker-machine env default)"

export JAVA_HOME="/Library/Java/JavaVirtualMachines/jdk-8-latest/Contents/Home"
export PATH=$JAVA_HOME/bin:$PATH
echo $JAVA_HOME
fi

# docker-credential-gcr uses GOOGLE_APPLICATION_CREDENTIALS as the credentials key file
export GOOGLE_APPLICATION_CREDENTIALS=${KOKORO_KEYSTORE_DIR}/72743_jib_integration_testing_key

docker-credential-gcr configure-docker

# Stops any left-over containers.
docker stop $(docker ps --all --quiet) || true
docker kill $(docker ps --all --quiet) || true

# Sets the integration testing project.
export JIB_INTEGRATION_TESTING_PROJECT=jib-integration-testing

cd github/jib

./gradlew clean build :jib-cli:integrationTest --info --stacktrace


Review comment (suggestion):
We probably don't mean to just run `jib-cli` here?
AFAIK presubmit is `jib-core` only, but continuous should be everything.

Code after:
#!/bin/bash

set -o errexit
set -o xtrace

gcloud components install docker-credential-gcr

# Docker service does not run by default in Big Sur but can be started with the following commands.
if [ "${KOKORO_JOB_CLUSTER}" = "MACOS_EXTERNAL" ]; then
source github/jib/kokoro/docker_setup.sh
fi

# docker-credential-gcr uses GOOGLE_APPLICATION_CREDENTIALS as the credentials key file
export GOOGLE_APPLICATION_CREDENTIALS=${KOKORO_KEYSTORE_DIR}/72743_jib_integration_testing_key

docker-credential-gcr configure-docker

# Stops any left-over containers.
docker stop $(docker ps --all --quiet) || true
docker kill $(docker ps --all --quiet) || true

# Sets the integration testing project.
export JIB_INTEGRATION_TESTING_PROJECT=jib-integration-testing

cd github/jib

./gradlew clean build integrationTest --info --stacktrace
Repository: GoogleContainerTools/jib
File: jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ContainerParameters.java (Java)

Code before:

  public void setUser(String user) {
    this.user = user;
  }

  @Input
  @Nullable
  @Optional
  public String getWorkingDirectory() {
    if (System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY) != null) {
      return System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY);
    }
    return workingDirectory;
  }

  public void setWorkingDirectory(String workingDirectory) {
    this.workingDirectory = workingDirectory;
  }

  @Input
  @Optional
  public String getFilesModificationTime() {
    if (System.getProperty(PropertyNames.CONTAINER_FILES_MODIFICATION_TIME) != null) {
      return System.getProperty(PropertyNames.CONTAINER_FILES_MODIFICATION_TIME);
    }
    return filesModificationTime.getOrNull();
  }

  public void setFilesModificationTime(Provider<String> filesModificationTime) {
    this.filesModificationTime.set(filesModificationTime);
  }

  public void setFilesModificationTime(String filesModificationTime) {
    this.filesModificationTime.set(filesModificationTime);
  }

  @Input
  @Optional
  public String getCreationTime() {
    if (System.getProperty(PropertyNames.CONTAINER_CREATION_TIME) != null) {
      return System.getProperty(PropertyNames.CONTAINER_CREATION_TIME);
    }
    return creationTime.getOrNull();
  }

  public void setCreationTime(Provider<String> creationTime) {
    this.creationTime.set(creationTime);
  }

  public void setCreationTime(String creationTime) {
    this.creationTime.set(creationTime);

Review comment (suggestion):
`getOrElse` with the default value of `EPOCH_PLUS_SECOND`, perhaps? Realistically, it should not be null, unless the user sets `null` explicitly, but easier to be safe.

Code after:
  public void setUser(String user) {
    this.user = user;
  }

  @Input
  @Nullable
  @Optional
  public String getWorkingDirectory() {
    if (System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY) != null) {
      return System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY);
    }
    return workingDirectory;
  }

  public void setWorkingDirectory(String workingDirectory) {
    this.workingDirectory = workingDirectory;
  }

  @Input
  @Optional
  public Property<String> getFilesModificationTime() {
    String property = System.getProperty(PropertyNames.CONTAINER_FILES_MODIFICATION_TIME);
    if (property != null && !property.equals(filesModificationTime.get())) {
      filesModificationTime.set(property);
    }
    return filesModificationTime;
  }

  @Input
  @Optional
  public Property<String> getCreationTime() {
    String property = System.getProperty(PropertyNames.CONTAINER_CREATION_TIME);
    if (property != null && !property.equals(creationTime.get())) {
      creationTime.set(property);
    }
    return creationTime;
  }
}
Repository: GoogleContainerTools/jib
File: jib-cli/src/integration-test/java/com/google/cloud/tools/jib/cli/JarCommandTest.java (Java)

Code before:
    boolean success = task.call();
    assertThat(success).isTrue();

    // Create a manifest file
    Manifest manifest = new Manifest();
    Attributes attributes = new Attributes();
    attributes.putValue("Manifest-Version", "1.0");
    if (classPath != null) {
      attributes.putValue("Class-Path", classPath);
    }
    if (mainClass != null) {
      attributes.putValue("Main-Class", mainClass);
    }
    manifest.getMainAttributes().putAll(attributes);

    // Create JAR
    File jarFile = workingDir.resolve(name).toFile();
    jarFile.getParentFile().mkdirs();
    FileOutputStream fileOutputStream = new FileOutputStream(jarFile);
    JarOutputStream jarOutputStream = new JarOutputStream(fileOutputStream, manifest);
    ZipEntry zipEntry = new ZipEntry(className + ".class");
    jarOutputStream.putNextEntry(zipEntry);
    jarOutputStream.write(Files.readAllBytes(workingDir.resolve(className + ".class")));
    jarOutputStream.closeEntry();
    jarOutputStream.close();
    fileOutputStream.close();
  }
}


Review comment (suggestion):
Alternatively, a `try with resources` block could also help us with automatically closing these streams without an explicit call to close().

```
try (InputStream in = Files.newInputStream(source);
    OutputStream out = Files.newOutputStream(dest)) {
  // read from in, write to out
}
```

Code after:
    Iterable<? extends JavaFileObject> compilationUnits =
        fileManager.getJavaFileObjectsFromFiles(Collections.singletonList(javaFilePath.toFile()));
    Iterable<String> options = Arrays.asList("-source", "1.8", "-target", "1.8");
    JavaCompiler.CompilationTask task =
        compiler.getTask(null, fileManager, null, options, null, compilationUnits);
    boolean success = task.call();
    assertThat(success).isTrue();

    // Create a manifest file
    Manifest manifest = new Manifest();
    Attributes attributes = new Attributes();
    attributes.putValue("Manifest-Version", "1.0");
    if (classPath != null) {
      attributes.putValue("Class-Path", classPath);
    }
    if (mainClass != null) {
      attributes.putValue("Main-Class", mainClass);
    }
    manifest.getMainAttributes().putAll(attributes);

    // Create JAR
    File jarFile = workingDir.resolve(name).toFile();
    jarFile.getParentFile().mkdirs();
    try (FileOutputStream fileOutputStream = new FileOutputStream(jarFile);
        JarOutputStream jarOutputStream = new JarOutputStream(fileOutputStream, manifest)) {
      ZipEntry zipEntry = new ZipEntry(className + ".class");
      jarOutputStream.putNextEntry(zipEntry);
      jarOutputStream.write(Files.readAllBytes(workingDir.resolve(className + ".class")));
      jarOutputStream.closeEntry();
    }
  }
}