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

Migrate build steps to Gradle build #46

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Conversation

pkwarren
Copy link
Member

Currently, certain Gradle tasks (like 'build', 'check') don't work without running 'make generate' first. Instead of splitting logic in between Gradle and the Makefile, migrate logic for generating code, running license-header, and other details to the Gradle build so they can happen in the correct order automatically. This has the side effect that new users can use Gradle commands they are familiar with ('./gradlew build') and not have to learn the makefile conventions separately.

Currently, certain Gradle tasks (like 'build', 'check') don't work
without running 'make generate' first. Instead of splitting logic in
between Gradle and the Makefile, migrate logic for generating code,
running license-header, and other details to the Gradle build so they
can happen in the correct order automatically. This has the side effect
that new users can use Gradle commands they are familiar with
('./gradlew build') and not have to learn the makefile conventions
separately.
@@ -24,7 +24,5 @@ jobs:
distribution: 'temurin'
java-version: '17'
cache: 'gradle'
- name: Generate
run: make checkgenerate
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to add this step in the previous PR because it would fail unless the generation step is done before. This would also break the release flow. By moving the logic and dependencies to the Gradle build, we can ensure that prior to compiling Java code we run necessary code generation steps first.

@@ -33,32 +25,19 @@ checkgenerate: generate ## Checks if `make generate` produces a diff.

.PHONY: clean
clean: ## Delete intermediate build artifacts
@# -X only removes untracked files, -d recurses into directories, -f actually removes files/dirs
git clean -Xdf
Copy link
Member Author

Choose a reason for hiding this comment

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

This was removing the .idea/ directory used to work on this project too. By switching to Gradle (and ensuring all created files are created in build/), we can just use the normal clean step.

commandLine(bufCLIPath, "generate", "--template", "buf.gen.yaml", "src/main/resources")
}

tasks.register("generate") {
Copy link
Member Author

Choose a reason for hiding this comment

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

This replaces the steps from the make generate target in the Makefile.

tasks.withType<JavaCompile> {
if (JavaVersion.current().isJava9Compatible) doFirst {
options.compilerArgs = mutableListOf("--release", "8")
dependsOn("generateTestSources")
Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures that we've generated the source code necessary to compile the tests prior to compiling code.

targetExclude("src/main/java/build/buf/validate/**/*.java")
targetExclude("src/main/java/build/buf/validate/**/*.java", "build/generated/test-sources/bufgen/**/*.java")
}
kotlinGradle {
Copy link
Member Author

Choose a reason for hiding this comment

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

Now running spotless to auto format the Gradle build files.

@@ -4,13 +4,38 @@ import net.ltgt.gradle.errorprone.errorprone
plugins {
`version-catalog`

application
Copy link
Member Author

Choose a reason for hiding this comment

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

By adding this plugin, a distribution is automatically created (with a wrapper script to run the executable jar), so we don't need conformance.sh any longer.

)
}

tasks.register<Exec>("conformance") {
Copy link
Member Author

Choose a reason for hiding this comment

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

This logic was specific to this subdirectory (and project) but was included in the top-level makefile before. It helps to move it closer to where it is used.

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package build.buf;
package build.buf.protovalidate.conformance;
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this to a better location specific to this project.

Copy link
Member

@stefanvanburen stefanvanburen left a comment

Choose a reason for hiding this comment

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

looks reasonable to me, one nit and one question

build.gradle.kts Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
@pkwarren pkwarren merged commit 6530bef into main Oct 11, 2023
4 checks passed
@pkwarren pkwarren deleted the pkw/makefile-to-gradle branch October 11, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants