From 6d8419844371d735ca7a4cec3d8708997219e26e Mon Sep 17 00:00:00 2001 From: Hunter Mellema <124718352+hpmellema@users.noreply.github.com> Date: Fri, 12 Jan 2024 08:17:19 -0700 Subject: [PATCH] Move smithyCli configuration and correct issue with conventions (#112) Corrects issue where conventions were causing custom defined tasks used without the java plugin applied to fail and further simplify the usage of custom build tasks by separating the definition of the smithyCli configuration from the main source set to make sure that the cli configuration is available even when no sourceSets exist (i.e. the java base plugin has not been applied). --- .../smithy-build-task/build.gradle.kts | 4 -- .../smithy/gradle/SmithyBuildTaskTest.java | 1 - .../smithy/gradle/SmithyBasePlugin.java | 53 ++++++++++++------- .../amazon/smithy/gradle/SmithyUtils.java | 18 +------ .../internal/CliDependencyResolver.java | 33 ++++++------ .../gradle/tasks/AbstractSmithyCliTask.java | 5 +- .../smithy/gradle/tasks/BaseSmithyTask.java | 33 ++++++++---- .../smithy/gradle/tasks/SmithyBuildTask.java | 3 +- .../smithy/gradle/tasks/SmithyFormatTask.java | 1 - .../gradle/tasks/SmithyValidateTask.java | 14 +---- 10 files changed, 80 insertions(+), 85 deletions(-) diff --git a/examples/base-plugin/smithy-build-task/build.gradle.kts b/examples/base-plugin/smithy-build-task/build.gradle.kts index b84a58c..e1319d3 100644 --- a/examples/base-plugin/smithy-build-task/build.gradle.kts +++ b/examples/base-plugin/smithy-build-task/build.gradle.kts @@ -6,13 +6,9 @@ import software.amazon.smithy.gradle.tasks.SmithyBuildTask // and the classpath used when building. plugins { - id("java-library") id("software.amazon.smithy.gradle.smithy-base").version("0.9.0") } -tasks["jar"].enabled = false -tasks["smithyBuild"].enabled = false - tasks.create("doit") { models.set(files("model/")) smithyBuildConfigs.set(files("smithy-build.json")) diff --git a/smithy-base/src/it/java/software/amazon/smithy/gradle/SmithyBuildTaskTest.java b/smithy-base/src/it/java/software/amazon/smithy/gradle/SmithyBuildTaskTest.java index 8f4ae58..47a41e4 100644 --- a/smithy-base/src/it/java/software/amazon/smithy/gradle/SmithyBuildTaskTest.java +++ b/smithy-base/src/it/java/software/amazon/smithy/gradle/SmithyBuildTaskTest.java @@ -30,7 +30,6 @@ public void testCustomBuild() { .withProjectDir(buildDir) .withArguments("build", "--stacktrace") .build(); - Utils.assertSmithyBuildDidNotRun(result); Utils.assertArtifactsCreated( buildDir, diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyBasePlugin.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyBasePlugin.java index d4bd289..a653865 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyBasePlugin.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyBasePlugin.java @@ -30,6 +30,7 @@ public final class SmithyBasePlugin implements Plugin { * Default name to use for the build task created by this plugin. */ public static final String SMITHY_BUILD_TASK_NAME = "smithyBuild"; + public static final String SMITHY_CLI_CONFIG = "smithyCli"; private static final GradleVersion MINIMUM_GRADLE_VERSION = GradleVersion.version("8.2.0"); private static final GradleVersion MIN_SMITHY_FORMAT_VERSION = GradleVersion.version("1.33.0"); @@ -51,6 +52,7 @@ public void apply(@Nonnull Project project) { // Add smithy source set extension SmithyExtension smithyExtension = project.getExtensions().create("smithy", SmithyExtension.class); + configureSmithyCliConfig(project); configureSourceSetDefaults(project, smithyExtension); } @@ -63,6 +65,14 @@ private static void validateGradleVersion() { } } + private static void configureSmithyCliConfig(Project project) { + // Set up Smithy-specific configurations + Configuration smithyCliConfiguration = project.getConfigurations() + .maybeCreate(SmithyUtils.SMITHY_CLI_CONFIGURATION_NAME); + smithyCliConfiguration.setVisible(false); + smithyCliConfiguration.setDescription("Configuration for Smithy CLI and associated dependencies."); + } + private void configureSourceSetDefaults(Project project, SmithyExtension extension) { project.getExtensions().getByType(SourceSetContainer.class).all(sourceSet -> { createConfigurations(sourceSet, project.getConfigurations()); @@ -70,15 +80,23 @@ private void configureSourceSetDefaults(Project project, SmithyExtension extensi TaskProvider buildTaskTaskProvider = addBuildTaskForSourceSet(sourceSet, sds, extension); // Ensure smithy-build is executed as part of building the "main" feature if (SourceSet.isMain(sourceSet)) { + // The CLI configuration should extend the main runtimeClasspath config, so we can + // resolve the cli version to use based on dependencies. + Configuration runtimeClasspathConfig = project.getConfigurations() + .getByName(sourceSet.getRuntimeClasspathConfigurationName()); + SmithyUtils.getCliConfiguration(project).extendsFrom(runtimeClasspathConfig); project.getTasks().getByName("build").dependsOn(buildTaskTaskProvider); } + }); - project.afterEvaluate(p -> { - // Resolve the Smithy CLI artifact to use - String cliVersion = CliDependencyResolver.resolve(project, sourceSet); + project.afterEvaluate(p -> { + // Resolve the Smithy CLI artifact to use + String cliVersion = CliDependencyResolver.resolve(project); + project.getExtensions().getByType(SourceSetContainer.class).all(sourceSet -> { // Add format task for source set if enabled and the CLI version supports it if (extension.getFormat().get() && cliVersionSupportsFormat(cliVersion, sourceSet)) { + SmithySourceDirectorySet sds = sourceSet.getExtensions().getByType(SmithySourceDirectorySet.class); addFormatTaskForSourceSet(sourceSet, sds, extension); } }); @@ -104,30 +122,20 @@ private boolean cliVersionSupportsFormat(String cliVersion, SourceSet sourceSet) /** - * Creates and configures smithy configurations if they do not already exist. - ** + * Creates and configures smithy-specific configurations if they do not already exist. + * * @param sourceSet source set to add configurations to * @param configurations configuration container */ private void createConfigurations(SourceSet sourceSet, ConfigurationContainer configurations) { - String runtimeClasspathConfigurationName = sourceSet.getRuntimeClasspathConfigurationName(); String compileOnlyConfigurationName = sourceSet.getCompileOnlyConfigurationName(); - String sourceSetName = sourceSet.toString(); - - // Set up Smithy-specific configurations - Configuration smithyCliConfiguration = configurations.maybeCreate( - SmithyUtils.getSmithyCliConfigurationName(sourceSet)); - smithyCliConfiguration.extendsFrom(configurations.getByName(runtimeClasspathConfigurationName)); - smithyCliConfiguration.setVisible(false); - smithyCliConfiguration.setDescription("Configuration for Smithy CLI and associated dependencies for the " - + sourceSetName + " sourceSet."); Configuration smithyBuildConfig = configurations.maybeCreate( SmithyUtils.getSmithyBuildConfigurationName(sourceSet)); smithyBuildConfig.setVisible(false); smithyBuildConfig.setTransitive(true); smithyBuildConfig.extendsFrom(configurations.getByName(compileOnlyConfigurationName)); - smithyBuildConfig.setDescription("Build-time smithy dependencies for " + sourceSetName + "."); + smithyBuildConfig.setDescription("Build-time smithy dependencies for " + sourceSet + "."); } /** @@ -173,7 +181,10 @@ private TaskProvider addBuildTaskForSourceSet(SourceSet sourceS SmithyExtension extension ) { String taskName = SmithyUtils.getRelativeSourceSetName(sourceSet, SMITHY_BUILD_TASK_NAME); - TaskProvider buildTaskTaskProvider = project.getTasks() + String buildConfigName = SmithyUtils.getSmithyBuildConfigurationName(sourceSet); + String runtimeConfigName = sourceSet.getRuntimeClasspathConfigurationName(); + + return project.getTasks() .register(taskName, SmithyBuildTask.class, build -> { // Configure basic extension settings @@ -186,10 +197,14 @@ private TaskProvider addBuildTaskForSourceSet(SourceSet sourceS build.getProjectionSourceTags().set(extension.getProjectionSourceTags()); build.getOutputDir().set(extension.getOutputDirectory()); + // Add smithy configurations as classpaths for build task + build.getCliClasspath().set(project.getConfigurations().getByName(SMITHY_CLI_CONFIG)); + build.getBuildClasspath().set(project.getConfigurations().getByName(buildConfigName)); + build.getModelDiscoveryClasspath().set( + project.getConfigurations().getByName(runtimeConfigName)); + // this allows the main smithy build task to show up when running `gradle tasks` build.setGroup(LifecycleBasePlugin.BUILD_GROUP); }); - - return buildTaskTaskProvider; } } diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyUtils.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyUtils.java index 3b896ec..19aa172 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyUtils.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/SmithyUtils.java @@ -46,16 +46,6 @@ public final class SmithyUtils { private SmithyUtils() {} - /** - * Gets the {@code SmithyExtension} extension of a {@code Project}. - * - * @param project Project to query. - * @return Returns the extension. - */ - public static SmithyExtension getSmithyExtension(Project project) { - return project.getExtensions().getByType(SmithyExtension.class); - } - /** * Gets the path to a projection plugins output. * @@ -236,10 +226,6 @@ private static RuntimeException unwrapException(Throwable current) { } } - static String getSmithyCliConfigurationName(SourceSet sourceSet) { - return getRelativeSourceSetName(sourceSet, SMITHY_CLI_CONFIGURATION_NAME); - } - static String getSmithyBuildConfigurationName(SourceSet sourceSet) { return getRelativeSourceSetName(sourceSet, SMITHY_BUILD_CONFIGURATION_NAME); @@ -249,8 +235,8 @@ static String getRelativeSourceSetName(SourceSet sourceSet, String name) { return SourceSet.isMain(sourceSet) ? name : sourceSet.getName() + StringUtils.capitalize(name); } - public static Configuration getCliConfiguration(Project project, SourceSet sourceSet) { - return project.getConfigurations().getByName(getSmithyCliConfigurationName(sourceSet)); + public static Configuration getCliConfiguration(Project project) { + return project.getConfigurations().getByName(SMITHY_CLI_CONFIGURATION_NAME); } } diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/internal/CliDependencyResolver.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/internal/CliDependencyResolver.java index 8e7760e..4aa4fc4 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/internal/CliDependencyResolver.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/internal/CliDependencyResolver.java @@ -11,9 +11,10 @@ import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.FileCollection; -import org.gradle.api.tasks.SourceSet; +import org.gradle.api.plugins.JavaPlugin; import software.amazon.smithy.gradle.SmithyUtils; import software.amazon.smithy.utils.SmithyInternalApi; @@ -44,16 +45,15 @@ private CliDependencyResolver() {} * detected is used for the CLI. * * @param project Project to add dependencies to. - * @param sourceSet sourceset to use to find configurations. * * @return version of the cli that was resolved. */ - public static String resolve(Project project, SourceSet sourceSet) { - Configuration cli = SmithyUtils.getCliConfiguration(project, sourceSet); + public static String resolve(Project project) { + Configuration cli = SmithyUtils.getCliConfiguration(project); // Prefer explicitly set dependency first. Optional explicitCliDepOptional = cli.getAllDependencies().stream() - .filter(d -> isMatchingDependency(d, SMITHY_CLI_DEP_NAME)) + .filter(CliDependencyResolver::isMatchingDependency) .findFirst(); if (explicitCliDepOptional.isPresent()) { project.getLogger().info("(using explicitly configured Smithy CLI)"); @@ -64,14 +64,14 @@ public static String resolve(Project project, SourceSet sourceSet) { failIfRunningInMainSmithyRepo(project); // If no explicit dependency was found, find the CLI version by scanning and set this as a dependency - String cliVersion = getCliVersion(project, sourceSet); + String cliVersion = getCliVersion(project); project.getDependencies().add(cli.getName(), String.format(DEPENDENCY_NOTATION, cliVersion)); return cliVersion; } - private static String getCliVersion(Project project, SourceSet sourceSet) { - String cliVersion = detectCliVersionInRuntimeDependencies(project, sourceSet); + private static String getCliVersion(Project project) { + String cliVersion = detectCliVersionInRuntimeDependencies(project.getConfigurations()); if (cliVersion != null) { project.getLogger().info("(detected Smithy CLI version {})", cliVersion); } else { @@ -85,14 +85,17 @@ private static String getCliVersion(Project project, SourceSet sourceSet) { /** * Check if there's a dependency on smithy-model somewhere, and assume that version. * - * @param project configuration to search for CLI version - * @param sourceSet SourceSet to get runtime configuration for + * @param configurations Configuration contain to use to search for configurations * * @return version of cli available in configuration */ - public static String detectCliVersionInRuntimeDependencies(Project project, SourceSet sourceSet) { - Configuration runtimeClasspath = project.getConfigurations().getByName( - sourceSet.getRuntimeClasspathConfigurationName()); + public static String detectCliVersionInRuntimeDependencies(ConfigurationContainer configurations) { + // If the runtimeClasspath configuration does not exist we cannot scan runtime deps + if (configurations.findByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME) == null) { + return null; + } + Configuration runtimeClasspath = configurations.getByName( + JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME); return runtimeClasspath.getResolvedConfiguration().getResolvedArtifacts().stream() .filter(ra -> ra.getName().equals("smithy-model")) .map(ra -> ra.getModuleVersion().getId().getVersion()) @@ -100,9 +103,9 @@ public static String detectCliVersionInRuntimeDependencies(Project project, Sour .orElse(null); } - private static boolean isMatchingDependency(Dependency dependency, String name) { + private static boolean isMatchingDependency(Dependency dependency) { return Objects.equals(dependency.getGroup(), "software.amazon.smithy") - && dependency.getName().equals(name); + && dependency.getName().equals(SMITHY_CLI_DEP_NAME); } private static String scanForSmithyCliVersion(Project project) { diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java index c29a9ec..68456e6 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java @@ -55,15 +55,14 @@ abstract class AbstractSmithyCliTask extends BaseSmithyTask { final void executeCliProcess(String command, List additionalArgs, FileCollection sources, - FileCollection modelDiscoveryClasspath, boolean disableModelDiscovery ) { List args = new ArrayList<>(); args.add(command); - if (modelDiscoveryClasspath != null) { + if (!getModelDiscoveryClasspath().get().isEmpty()) { args.add("--discover-classpath"); - args.add(modelDiscoveryClasspath.getAsPath()); + args.add(getModelDiscoveryClasspath().get().getAsPath()); } else if (!disableModelDiscovery) { args.add("--discover"); } diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java index 3c81bb6..1bc20c8 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java @@ -23,7 +23,6 @@ import org.gradle.work.DisableCachingByDefault; import org.gradle.workers.WorkerExecutor; - /** * Base class for all Smithy tasks. */ @@ -35,33 +34,45 @@ abstract class BaseSmithyTask extends DefaultTask { BaseSmithyTask() { getFork().convention(false); getShowStackTrace().convention(ShowStacktrace.INTERNAL_EXCEPTIONS); - getResolvedCliClasspath().convention(getProject().getConfigurations().getByName("smithyCli")); - getBuildClasspath().convention(getProject().getConfigurations().getByName("smithyBuild")); - getRuntimeClasspath().convention(getProject().getConfigurations().getByName("runtimeClasspath")); + + // By default, the build classpath and model discovery classpaths are empty file collections. + getBuildClasspath().set(getProject().files()); + getModelDiscoveryClasspath().set(getProject().files()); + + // if the smithyCli configuration exists use it by default + if (getProject().getConfigurations().findByName("smithyCli") != null) { + getCliClasspath().convention(getProject().getConfigurations() + .getByName("smithyCli")); + } + startParameter = getProject().getGradle().getStartParameter(); } /** * Base classpath used for executing the smithy cli. + * + *

Note: this classpath must contain the smithy-cli jar. */ @Classpath - @Optional - public abstract Property getResolvedCliClasspath(); + public abstract Property getCliClasspath(); /** - * Classpath to use for build dependencies. + * Classpath used for discovery of additional Smithy models during cli execution. + * + *

Defaults to an empty collection. */ @Classpath @Optional - public abstract Property getBuildClasspath(); + public abstract Property getModelDiscoveryClasspath(); /** - * Classpath for runtime dependencies. + * Classpath to use for build dependencies. */ @Classpath @Optional - public abstract Property getRuntimeClasspath(); + public abstract Property getBuildClasspath(); + /** * Read-only property that returns the classpath used to determine the @@ -71,7 +82,7 @@ abstract class BaseSmithyTask extends DefaultTask { */ @Internal Provider getCliExecutionClasspath() { - return getResolvedCliClasspath().zip(getBuildClasspath(), FileCollection::plus); + return getCliClasspath().zip(getBuildClasspath(), FileCollection::plus); } /** diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java index ea4e72c..c2b432b 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java @@ -40,7 +40,6 @@ public SmithyBuildTask(ObjectFactory objectFactory) { getOutputDir().convention(SmithyUtils.getProjectionOutputDirProperty(getProject())); } - /** * Tags that are searched for in classpaths when determining which * models are projected into the created JAR. @@ -112,7 +111,7 @@ public void execute() { BuildParameterBuilder builder = new BuildParameterBuilder(); // Model discovery classpath - builder.libClasspath(getRuntimeClasspath().get().getAsPath()); + builder.libClasspath(getModelDiscoveryClasspath().get().getAsPath()); builder.buildClasspath(getCliExecutionClasspath().get().getAsPath()); builder.projectionSourceTags(getProjectionSourceTags().get()); builder.allowUnknownTraits(getAllowUnknownTraits().get()); diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java index 777f394..f1bb941 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java @@ -41,7 +41,6 @@ public void execute() { executeCliProcess("format", ListUtils.of(), objectFactory.fileCollection().from(file), - null, true ); } diff --git a/smithy-jar/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java b/smithy-jar/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java index fae3b46..1553221 100644 --- a/smithy-jar/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java +++ b/smithy-jar/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java @@ -10,7 +10,6 @@ import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; import org.gradle.api.provider.Provider; -import org.gradle.api.tasks.Classpath; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.Internal; @@ -36,7 +35,6 @@ public SmithyValidateTask(ObjectFactory objectFactory) { super(objectFactory); getAllowUnknownTraits().convention(false); getDisableModelDiscovery().convention(false); - getResolvedCliClasspath().convention(getProject().getConfigurations().getByName("smithyCli")); setDescription(DESCRIPTION); } @@ -57,15 +55,6 @@ public SmithyValidateTask(ObjectFactory objectFactory) { @InputFiles public abstract Property getJarToValidate(); - /** - * Classpath used for discovery of additional Smithy models during cli execution. - * - *

Defaults to an empty collection. - */ - @Classpath - @Optional - public abstract Property getModelDiscoveryClasspath(); - /** * Disable model discovery. * @@ -90,7 +79,7 @@ public SmithyValidateTask(ObjectFactory objectFactory) { @Internal @Override Provider getCliExecutionClasspath() { - return getResolvedCliClasspath(); + return getCliClasspath(); } @TaskAction @@ -100,7 +89,6 @@ public void execute() { // Set models to an empty collection so source models are not included in validation path. executeCliProcess("validate", ListUtils.of(), getJarToValidate().get(), - getModelDiscoveryClasspath().getOrNull(), getDisableModelDiscovery().get() ); }