Skip to content
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

Make git revision loading lazy #45358

Merged
merged 1 commit into from
Aug 9, 2019
Merged

Make git revision loading lazy #45358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 32 additions & 15 deletions buildSrc/src/main/java/org/elasticsearch/gradle/info/GlobalBuildInfoPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;

import static java.nio.charset.StandardCharsets.UTF_8;
Expand All @@ -45,6 +46,8 @@ public void apply(Project project) {
File compilerJavaHome = findCompilerJavaHome();
File runtimeJavaHome = findRuntimeJavaHome(compilerJavaHome);

Object gitRevisionResolver = createGitRevisionResolver(project);

final List<JavaHome> javaVersions = new ArrayList<>();
for (int version = 8; version <= Integer.parseInt(minimumCompilerVersion.getMajorVersion()); version++) {
if (System.getenv(getJavaHomeEnvVarName(Integer.toString(version))) != null) {
Expand Down Expand Up @@ -92,7 +95,7 @@ public void apply(Project project) {
ext.set("minimumCompilerVersion", minimumCompilerVersion);
ext.set("minimumRuntimeVersion", minimumRuntimeVersion);
ext.set("gradleJavaVersion", Jvm.current().getJavaVersion());
ext.set("gitRevision", gitRevision(project));
ext.set("gitRevision", gitRevisionResolver);
ext.set("buildDate", ZonedDateTime.now(ZoneOffset.UTC));
});
}
Expand Down Expand Up @@ -203,21 +206,35 @@ private static int findDefaultParallel(Project project) {
return _defaultParallel;
}

private String gitRevision(final Project project) {
final ByteArrayOutputStream stdout = new ByteArrayOutputStream();
final ByteArrayOutputStream stderr = new ByteArrayOutputStream();
final ExecResult result = project.exec(spec -> {
spec.setExecutable("git");
spec.setArgs(Arrays.asList("rev-parse", "HEAD"));
spec.setStandardOutput(stdout);
spec.setErrorOutput(stderr);
spec.setIgnoreExitValue(true);
});
private Object createGitRevisionResolver(final Project project) {
return new Object() {
private final AtomicReference<String> gitRevision = new AtomicReference<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better instead of making this an AtomicReference to simply synchronize these calls so we can avoid to possibility shelling to git multiple times if this is called concurrently? I think as it is we could do that and simply the first one would win.

Copy link
Member Author

Choose a reason for hiding this comment

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

While this is a possibility, I wasn't sure it was necessary to avoid. I figured it was better to make concurrent calls fast, if they happen (but it seems unlikely?). By synchronizing the method, we would still be single threading the method even after we've retrieved the hash.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's hard to say either way. I'm fine with leaving as is.


@Override
public String toString() {
if (gitRevision.get() == null) {
final ByteArrayOutputStream stdout = new ByteArrayOutputStream();
final ByteArrayOutputStream stderr = new ByteArrayOutputStream();
final ExecResult result = project.exec(spec -> {
spec.setExecutable("git");
spec.setArgs(Arrays.asList("rev-parse", "HEAD"));
spec.setStandardOutput(stdout);
spec.setErrorOutput(stderr);
spec.setIgnoreExitValue(true);
});

final String revision;
if (result.getExitValue() != 0) {
revision = "unknown";
} else {
revision = stdout.toString(UTF_8).trim();
}
this.gitRevision.compareAndSet(null, revision);
}
return gitRevision.get();
}
};

if (result.getExitValue() != 0) {
return "unknown";
}
return stdout.toString(UTF_8).trim();
}

}
9 changes: 5 additions & 4 deletions distribution/docker/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import org.elasticsearch.gradle.BuildPlugin
import org.elasticsearch.gradle.LoggedExec
import org.elasticsearch.gradle.MavenFilteringHack
import org.elasticsearch.gradle.VersionProperties
import org.elasticsearch.gradle.testfixtures.TestFixturesPlugin
import org.elasticsearch.gradle.testfixtures.TestFixturesPlugin

apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.test.fixtures'
Expand Down Expand Up @@ -58,15 +57,17 @@ project.ext {
}

from(project.projectDir.toPath().resolve("src/docker/Dockerfile")) {
MavenFilteringHack.filter(it, expansions(oss, local))
expand(expansions(oss, local))
}
}
}
}

void addCopyDockerContextTask(final boolean oss) {
task(taskName("copy", oss, "DockerContext"), type: Sync) {
inputs.properties(expansions(oss, true))
expansions(oss, true).each { k, v ->
inputs.property(k, { v.toString() })
}
into files(oss)

with dockerBuildContext(oss, true)
Expand Down
8 changes: 4 additions & 4 deletions distribution/docker/src/docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

FROM centos:7 AS builder

ENV PATH /usr/share/elasticsearch/bin:$PATH
ENV PATH /usr/share/elasticsearch/bin:\$PATH

RUN groupadd -g 1000 elasticsearch && \
adduser -u 1000 -g 1000 -d /usr/share/elasticsearch elasticsearch
Expand Down Expand Up @@ -41,8 +41,8 @@ ENV ELASTIC_CONTAINER true

RUN for iter in {1..10}; do yum update -y && \
yum install -y nc && \
yum clean all && exit_code=0 && break || exit_code=$? && echo "yum error: retry $iter in 10s" && sleep 10; done; \
(exit $exit_code)
yum clean all && exit_code=0 && break || exit_code=\$? && echo "yum error: retry \$iter in 10s" && sleep 10; done; \
(exit \$exit_code)

RUN groupadd -g 1000 elasticsearch && \
adduser -u 1000 -g 1000 -G 0 -d /usr/share/elasticsearch elasticsearch && \
Expand All @@ -57,7 +57,7 @@ COPY --from=builder --chown=1000:0 /usr/share/elasticsearch /usr/share/elasticse
# REF: https://github.com/elastic/elasticsearch-docker/issues/171
RUN ln -sf /etc/pki/ca-trust/extracted/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts

ENV PATH /usr/share/elasticsearch/bin:$PATH
ENV PATH /usr/share/elasticsearch/bin:\$PATH

COPY --chown=1000:0 bin/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh

Expand Down