From 1614bd6d9a2e8312423e7157a4a66e1ee768df10 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Tue, 19 Mar 2024 18:52:25 +0000 Subject: [PATCH 1/2] GH-132: Fix race condition when referencing the same archived dependencies in multiple places --- .gitignore | 2 +- .../invoker.properties | 17 +++ .../pom.xml | 108 ++++++++++++++++++ .../some-dependency-1/pom.xml | 47 ++++++++ .../some-dependency-2/pom.xml | 41 +++++++ .../some-dependency-3/pom.xml | 47 ++++++++ .../some-dependency-4/pom.xml | 41 +++++++ .../some-project/invoker.properties | 17 +++ .../some-project/pom.xml | 106 +++++++++++++++++ .../src/main/protobuf/helloworld.proto | 27 +++++ .../org/example/helloworld/ProtobufTest.java | 63 ++++++++++ .../source/ProtoSourceResolver.java | 2 + 12 files changed, 517 insertions(+), 1 deletion(-) create mode 100644 src/it/gh-132-duplicate-maven-dependencies/invoker.properties create mode 100644 src/it/gh-132-duplicate-maven-dependencies/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-dependency-1/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-dependency-2/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-dependency-3/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-dependency-4/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-project/invoker.properties create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-project/pom.xml create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-project/src/main/protobuf/helloworld.proto create mode 100644 src/it/gh-132-duplicate-maven-dependencies/some-project/src/test/java/org/example/helloworld/ProtobufTest.java diff --git a/.gitignore b/.gitignore index c94c5d02..914810cd 100644 --- a/.gitignore +++ b/.gitignore @@ -9,7 +9,7 @@ target/ .attach* # Stuff that I generate when I have lost all hope in the world -/build.log +/*.log temp/ # Version backups after running ./mvnw versions:set diff --git a/src/it/gh-132-duplicate-maven-dependencies/invoker.properties b/src/it/gh-132-duplicate-maven-dependencies/invoker.properties new file mode 100644 index 00000000..542474dd --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/invoker.properties @@ -0,0 +1,17 @@ +# +# 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. +# + +invoker.goals=clean verify --projects some-project --also-make diff --git a/src/it/gh-132-duplicate-maven-dependencies/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/pom.xml new file mode 100644 index 00000000..a072bcec --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/pom.xml @@ -0,0 +1,108 @@ + + + + 4.0.0 + + org.example + some-parent + 0.0.1-SNAPSHOT + pom + + + some-dependency-1 + some-dependency-2 + some-dependency-3 + some-dependency-4 + some-project + + + + 5.10.1 + 4.26.0 + 2.37.0 + + 3.11.0 + 3.2.2 + + UTF-8 + + + + + + com.google.protobuf + protobuf-java + ${protobuf.version} + compile + + + + com.google.api.grpc + proto-google-common-protos + ${proto-google-common-protos.version} + compile + + + + org.junit.jupiter + junit-jupiter + ${junit.version} + test + + + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + ${maven-compiler-plugin.version} + + + + org.apache.maven.plugins + maven-surefire-plugin + ${maven-surefire-plugin.version} + + + + @project.groupId@ + @project.artifactId@ + @project.version@ + + + ${protobuf.version} + + + + + + generate + + + + + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-dependency-1/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-1/pom.xml new file mode 100644 index 00000000..35940611 --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-1/pom.xml @@ -0,0 +1,47 @@ + + + + 4.0.0 + + + org.example + some-parent + 0.0.1-SNAPSHOT + ../pom.xml + + + some-dependency-1 + + + + com.google.protobuf + protobuf-java + compile + + + + com.google.api.grpc + proto-google-common-protos + compile + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-dependency-2/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-2/pom.xml new file mode 100644 index 00000000..82f975e3 --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-2/pom.xml @@ -0,0 +1,41 @@ + + + + 4.0.0 + + + org.example + some-parent + 0.0.1-SNAPSHOT + ../pom.xml + + + some-dependency-2 + + + + com.google.protobuf + protobuf-java + compile + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-dependency-3/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-3/pom.xml new file mode 100644 index 00000000..9350066c --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-3/pom.xml @@ -0,0 +1,47 @@ + + + + 4.0.0 + + + org.example + some-parent + 0.0.1-SNAPSHOT + ../pom.xml + + + some-dependency-3 + + + + com.google.protobuf + protobuf-java + compile + + + + com.google.api.grpc + proto-google-common-protos + compile + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-dependency-4/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-4/pom.xml new file mode 100644 index 00000000..fc584c60 --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-dependency-4/pom.xml @@ -0,0 +1,41 @@ + + + + 4.0.0 + + + org.example + some-parent + 0.0.1-SNAPSHOT + ../pom.xml + + + some-dependency-4 + + + + com.google.protobuf + protobuf-java + compile + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-project/invoker.properties b/src/it/gh-132-duplicate-maven-dependencies/some-project/invoker.properties new file mode 100644 index 00000000..71cb16da --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-project/invoker.properties @@ -0,0 +1,17 @@ +# +# 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. +# + +invoker.goals = clean package diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-project/pom.xml b/src/it/gh-132-duplicate-maven-dependencies/some-project/pom.xml new file mode 100644 index 00000000..3fb4673e --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-project/pom.xml @@ -0,0 +1,106 @@ + + + + 4.0.0 + + + org.example + some-parent + 0.0.1-SNAPSHOT + ../pom.xml + + + some-project + + + 5.10.1 + 4.26.0 + + 3.11.0 + 3.2.2 + + UTF-8 + + + + + ${project.parent.groupId} + some-dependency-1 + ${project.parent.version} + compile + + + + ${project.parent.groupId} + some-dependency-2 + ${project.parent.version} + compile + + + + ${project.parent.groupId} + some-dependency-3 + ${project.parent.version} + compile + + + + ${project.parent.groupId} + some-dependency-4 + ${project.parent.version} + compile + + + + com.google.protobuf + protobuf-java + compile + + + + org.junit.jupiter + junit-jupiter + test + + + + + + + @project.groupId@ + @project.artifactId@ + @project.version@ + + + ${protobuf.version} + + + + + + generate + + + + + + + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-project/src/main/protobuf/helloworld.proto b/src/it/gh-132-duplicate-maven-dependencies/some-project/src/main/protobuf/helloworld.proto new file mode 100644 index 00000000..8d91946c --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-project/src/main/protobuf/helloworld.proto @@ -0,0 +1,27 @@ +// +// 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. +// + +syntax = "proto3"; + +option java_multiple_files = true; +option java_package = "org.example.helloworld"; + +package org.example.helloworld; + +message GreetingRequest { + string name = 1; +} + diff --git a/src/it/gh-132-duplicate-maven-dependencies/some-project/src/test/java/org/example/helloworld/ProtobufTest.java b/src/it/gh-132-duplicate-maven-dependencies/some-project/src/test/java/org/example/helloworld/ProtobufTest.java new file mode 100644 index 00000000..21038f9a --- /dev/null +++ b/src/it/gh-132-duplicate-maven-dependencies/some-project/src/test/java/org/example/helloworld/ProtobufTest.java @@ -0,0 +1,63 @@ +/* + * 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 org.example.helloworld; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import org.junit.jupiter.api.Test; + +class ProtobufTest { + @Test + void generatedProtobufSourcesAreFullMessages() throws Throwable { + // When + var superClasses = new ArrayList(); + Class superClass = GreetingRequest.class; + + do { + superClasses.add(superClass.getName()); + superClass = superClass.getSuperclass(); + } while (superClass != null); + + // Then + // GeneratedMessageV3 for protobuf-java 3.25.3 and older. + // GeneratedMessage for protobuf-java 4.26.0 and newer. + assertTrue(superClasses.contains("com.google.protobuf.GeneratedMessage")); + } + + @Test + void generatedProtobufSourcesAreValid() throws Throwable { + // Given + var expectedGreetingRequest = GreetingRequest + .newBuilder() + .setName("Ashley") + .build(); + + // When + var baos = new ByteArrayOutputStream(); + expectedGreetingRequest.writeTo(baos); + var actualGreetingRequest = GreetingRequest.parseFrom(baos.toByteArray()); + + assertNotEquals(0, baos.toByteArray().length); + + // Then + assertEquals(expectedGreetingRequest.getName(), actualGreetingRequest.getName()); + } +} diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java b/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java index ca7d4880..80f080c5 100644 --- a/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java +++ b/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java @@ -115,6 +115,8 @@ public Collection createProtoFileListings( originalPaths .stream() + // GH-132: Avoid running multiple times on the same location. + .distinct() .map(this::submitProtoFileListingTask) // terminal operation to ensure all are scheduled prior to joining. .collect(Collectors.toList()) From d4775a61cefa53c4bf8215560abc7f6e6e9d0714 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Tue, 19 Mar 2024 18:55:22 +0000 Subject: [PATCH 2/2] GH-132: Add an extra case to work around using un-normalized paths that are duplicated --- .../protobufmavenplugin/source/ProtoSourceResolver.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java b/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java index 80f080c5..2b454643 100644 --- a/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java +++ b/src/main/java/io/github/ascopes/protobufmavenplugin/source/ProtoSourceResolver.java @@ -18,6 +18,8 @@ import static java.util.function.Predicate.not; +import io.github.ascopes.protobufmavenplugin.platform.FileUtils; +import io.github.ascopes.protobufmavenplugin.platform.HostSystem; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -115,6 +117,9 @@ public Collection createProtoFileListings( originalPaths .stream() + // GH-132: Normalize to ensure different paths to the same file do not + // get duplicated across more than one extraction site. + .map(FileUtils::normalize) // GH-132: Avoid running multiple times on the same location. .distinct() .map(this::submitProtoFileListingTask)