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.

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

GH-481: Fix bug where large builds crash on Windows #482

Merged
merged 4 commits into from
Dec 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
<maven-assembly-plugin.version>3.7.1</maven-assembly-plugin.version>
<maven-checkstyle-plugin.version>3.6.0</maven-checkstyle-plugin.version>
<maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
<maven-dependency-plugin.version>3.8.1</maven-dependency-plugin.version>
<maven-gpg-plugin.version>3.2.7</maven-gpg-plugin.version>
<maven-install-plugin.version>3.1.3</maven-install-plugin.version>
<maven-invoker-plugin.version>3.8.1</maven-invoker-plugin.version>
Expand Down Expand Up @@ -323,6 +324,13 @@
<version>${maven-assembly-plugin.version}</version>
</plugin>

<plugin>
<!-- Used for Mockito agent support in JDK21+. We have to know the JAR path of the dependency. -->
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>${maven-dependency-plugin.version}</version>
</plugin>

<plugin>
<!-- Checking style of code matches the Google code style. -->
<groupId>org.apache.maven.plugins</groupId>
Expand Down
19 changes: 19 additions & 0 deletions protobuf-maven-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,21 @@
<artifactId>maven-compiler-plugin</artifactId>
</plugin>

<plugin>
<!-- Used for Mockito agent support in JDK21+. We have to know the JAR path of the dependency. -->
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>

<executions>
<execution>
<phase>initialize</phase>
<goals>
<goal>properties</goal>
</goals>
</execution>
</executions>
</plugin>

<plugin>
<!-- Runs integration tests. -->
<groupId>org.apache.maven.plugins</groupId>
Expand Down Expand Up @@ -219,6 +234,10 @@
<!-- Unit testing. -->
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>

<configuration>
<argLine>@{argLine} -javaagent:${org.mockito:mockito-core:jar}</argLine>
</configuration>
</plugin>

<plugin>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#
# Copyright (C) 2023 - 2024, Ashley Scopes.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

# Package twice to trigger incremental compilation on the second run. This
# allows us to test passing large numbers of files through the incremental compilation
# cache as well.
invoker.goals.0 = clean
invoker.goals.1 = package -Dmaven.jar.skip -Dmaven.test.skip
invoker.goals.2 = package -Dmaven.jar.skip -Dmaven.test.skip
76 changes: 76 additions & 0 deletions protobuf-maven-plugin/src/it/gh-481-command-length/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--

Copyright (C) 2023 - 2024, Ashley Scopes.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

-->
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>@project.groupId@.it</groupId>
<artifactId>integration-test-parent</artifactId>
<version>@project.version@</version>
<relativePath>../setup/pom.xml</relativePath>
</parent>

<groupId>gh-481-command-length</groupId>
<artifactId>gh-481-command-length</artifactId>

<dependencies>
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
<scope>compile</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>

<configuration>
<!-- Speed up the builds a little bit -->
<debug>false</debug>
<fork>true</fork>
<meminitial>256</meminitial>
<maxmem>256</maxmem>
<proc>none</proc>
</configuration>
</plugin>

<plugin>
<groupId>@project.groupId@</groupId>
<artifactId>@project.artifactId@</artifactId>

<configuration>
<incrementalCompilation>true</incrementalCompilation>
</configuration>

<executions>
<execution>
<goals>
<goal>generate</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
146 changes: 146 additions & 0 deletions protobuf-maven-plugin/src/it/gh-481-command-length/prebuild.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright (C) 2023 - 2024, Ashley Scopes.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import groovy.transform.Field

import java.nio.charset.StandardCharsets
import java.nio.file.Files
import java.nio.file.Path
import java.util.concurrent.Executors
import java.util.concurrent.ExecutorService
import java.util.concurrent.Future

static final @Field String ALPHA = "abcdefghijklmnopqrstuvwxyz"
static final @Field String ALPHA_NUMERIC = ALPHA + "0123456789"
static final @Field String ALPHA_NUMERIC_UNDERSCORE = ALPHA_NUMERIC + "_"
static final @Field List<String> PROTO_TYPES = [
"double", "float", "int32", "int64", "uint32", "uint64", "sint32", "sint64",
"fixed32", "fixed64", "sfixed32", "sfixed64", "bool", "string", "bytes",
]

@Field Random rng = new Random()

int randomInt(int min, int max) {
return min + rng.nextInt(max - min)
}

char randomChar(String dictionary) {
return dictionary[rng.nextInt(dictionary.length())]
}

void addRandomMessageName(Appendable sb) {
int length = randomInt(5, 10)
sb.append(Character.toUpperCase(randomChar(ALPHA)))
for (int i = 1; i < length; ++i) {
sb.append(randomChar(ALPHA_NUMERIC))
}
}

void addRandomIdentifier(Appendable sb) {
int length = randomInt(5, 10)
sb.append(randomChar(ALPHA))
for (int i = 1; i < length; ++i) {
sb.append(randomChar(ALPHA_NUMERIC_UNDERSCORE))
}
}

void addRandomMessage(Appendable sb) {
sb.append("\nmessage ")
addRandomMessageName(sb)
sb.append(" {\n")
int fieldCount = randomInt(1, 5)
for (int i = 1; i <= fieldCount; ++i) {
sb.append(" ")
.append(PROTO_TYPES[rng.nextInt(PROTO_TYPES.size())])
.append(" ")
addRandomIdentifier(sb)
sb.append(" = ")
.append(i.toString())
.append(";\n")
}
sb.append("}\n")
}

void addRandomProtoFile(int index, Appendable sb) {
// File header
sb.append('syntax = "proto3";\n')
.append('option java_multiple_files = true;\n')
.append('option java_package = "')

sb.append("javapackage_").append(index.toString()).append(".")
int javaPackageLength = randomInt(1, 5)
addRandomIdentifier(sb)
for (int i = 1; i < javaPackageLength; ++i) {
sb.append(".")
addRandomIdentifier(sb)
}

sb.append('";\n\n')
.append("package ")

sb.append("protopackage_").append(index.toString()).append(".")
int protoPackageLength = randomInt(1, 5)
addRandomIdentifier(sb)
for (int i = 1; i < protoPackageLength; ++i) {
sb.append(".")
addRandomIdentifier(sb)
}
sb.append(';\n')

// File contents, one to two messages each.
int messageCount = randomInt(1, 2)
for (int i = 0; i < messageCount; ++i) {
addRandomMessage(sb)
}
}

String randomFileName(int index) {
StringBuilder fileName = new StringBuilder()
// Ensure the file names remain unique.
fileName.append(Character.toUpperCase(randomChar(ALPHA)))
int length = randomInt(1, 10)
for (int i = 1; i < length; ++i) {
fileName.append(randomChar(ALPHA_NUMERIC))
}
return fileName.append("_${index}.proto").toString()
}

@SuppressWarnings('GroovyAssignabilityCheck')
Future<Void> generateRandomFile(int index, Path baseDir, ExecutorService executor) {
executor.submit {
Path path = baseDir.resolve(randomFileName(index))
try (BufferedWriter writer = Files.newBufferedWriter(path, StandardCharsets.UTF_8)) {
addRandomProtoFile(index, writer)
}
return null
}
}

void generateRandomFiles(int count, ExecutorService executor) {
println("Generating ${count} random proto files files for this test...")
Path baseDir = basedir.toPath().resolve("src").resolve("main").resolve("protobuf")
Files.createDirectories(baseDir)
List<Future<Void>> futures = []
for (int i = 0; i < count; ++i) {
futures.add(generateRandomFile(i, baseDir, executor))
}
for (Future<Void> future : futures) {
future.get()
}
}

ExecutorService executor = Executors.newFixedThreadPool(30)
generateRandomFiles(500, executor)
executor.shutdown()
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import static io.github.ascopes.protobufmavenplugin.sources.SourceListing.flattenSourceProtoFiles;

import io.github.ascopes.protobufmavenplugin.plugins.ProjectPluginResolver;
import io.github.ascopes.protobufmavenplugin.protoc.ArgLineBuilder;
import io.github.ascopes.protobufmavenplugin.protoc.CommandLineExecutor;
import io.github.ascopes.protobufmavenplugin.protoc.ProtocArgumentFileBuilderBuilder;
import io.github.ascopes.protobufmavenplugin.protoc.ProtocResolver;
import io.github.ascopes.protobufmavenplugin.sources.ProjectInputListing;
import io.github.ascopes.protobufmavenplugin.sources.ProjectInputResolver;
Expand Down Expand Up @@ -109,34 +109,13 @@ public boolean generate(GenerationRequest request) throws ResolutionException, I

createOutputDirectories(request);

var argLineBuilder = new ArgLineBuilder(protocPath)
.fatalWarnings(request.isFatalWarnings())
.importPaths(projectInputs.getCompilableSources()
.stream()
.map(SourceListing::getSourceRoot)
.collect(Collectors.toUnmodifiableList()))
.importPaths(projectInputs.getDependencySources()
.stream()
.map(SourceListing::getSourceRoot)
.collect(Collectors.toUnmodifiableList()));

request.getEnabledLanguages()
.forEach(language -> argLineBuilder.generateCodeFor(
language,
request.getOutputDirectory(),
request.isLiteEnabled()
));

// GH-269: Add the plugins after the enabled languages to support generated code injection
argLineBuilder.plugins(resolvedPlugins, request.getOutputDirectory());

// GH-438: We now register the source roots before generating anything. This ensures we still
// call Javac with the sources even if we incrementally compile with zero changes.
registerSourceRoots(request);
// Determine the sources we need to regenerate. This will be all of the sources usually but

// Determine the sources we need to regenerate. This will be all the sources usually but
// if incremental compilation is enabled then we will only output the files that have changed
// unless we deem a full rebuild necesarry.
// unless we deem a full rebuild necessary.
var compilableSources = computeActualSourcesToCompile(request, projectInputs);
if (compilableSources.isEmpty()) {
// Nothing to compile. If we hit here, then we likely received inputs but were using
Expand All @@ -145,9 +124,25 @@ public boolean generate(GenerationRequest request) throws ResolutionException, I
return true;
}

var argLine = argLineBuilder.compile(compilableSources);
var argumentFileBuilder = new ProtocArgumentFileBuilderBuilder()
.addLanguages(
request.getEnabledLanguages(),
request.getOutputDirectory(),
request.isLiteEnabled())
.addImportPaths(projectInputs.getCompilableSources()
.stream()
.map(SourceListing::getSourceRoot)
.collect(Collectors.toUnmodifiableList()))
.addImportPaths(projectInputs.getDependencySources()
.stream()
.map(SourceListing::getSourceRoot)
.collect(Collectors.toUnmodifiableList()))
.addPlugins(resolvedPlugins, request.getOutputDirectory())
.addSourcePaths(compilableSources)
.setFatalWarnings(request.isFatalWarnings())
.build();

if (!commandLineExecutor.execute(argLine)) {
if (!commandLineExecutor.execute(protocPath, argumentFileBuilder)) {
return false;
}

Expand Down
Loading
Loading