Add Maven dependency resolution to the CLI #1526
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kstich
reviewed
Dec 9, 2022
smithy-cli/src/main/java/software/amazon/smithy/cli/commands/WarmupCommand.java
Outdated
Show resolved
Hide resolved
smithy-cli/src/main/java/software/amazon/smithy/cli/commands/WarmupCommand.java
Outdated
Show resolved
Hide resolved
This commit adds the ability to resolve dependencies in the Smithy CLI. This allows end users to author models, generate code, and other activities from the Smithy CLI without needing to use tools like Gradle or Maven. This lowers the up-front learning curve of most Smithy use cases to author models and generate code. Writing Java library code, model build plugins, or transforms is out of scope of the Smithy CLI. Such use cases require the use of tools like Gradle or Maven. To support dependency resolution in the CLI, this commit also introduces the ``--version`` option to display the current version. The ``smithy clean`` command was added since it was very useful to test dependency resolution by removing the classpath-cache file to force them to be re-resolved. Using the Smithy CLI directly means that tools like Gradle aren't automatically detecting the model directory and adding sources. To account for this, the `sources` property was added to smithy-build.json. It acts like `imports`, but the models contained in `sources` are considered part of the model being built rather than just a dependeny (i.e., the sources plugin will contain these models). To actually test maven resolution, this commit also adds integration tests for the CLI (./gradlew :smithy-cli:integ). Other changes: 1. The use of @SmithyUnstableApi and @SmithyInternalApi was removed from all smithy-cli classes because there is a blanket @SmithyUnstableApi annotation on the entire package, making the individual usage unnecessary. 2. Moved --severity setting from StandardOptions to BuildOptions. This setting is only needed when building Smithy models. 3. Made Smithy CLI commands and related options classes package-private since they aren't referenced outside of the commands package. 4. This commit also prints out more useful exceptions when --stacktrace is on since wrapping things in a DependencyRunnably obscured the underlying issue. 5. Command line options related to model discovery and discovery classpath are now hidden from help output. They are much less relevant now that dependency resolution is baked into the CLI, and may not even be necessary if Smithy's Ant and Gradle integration aren't using it (need to verify).
mtdowling
force-pushed
the
cli-dependencies-rebase
branch
from
41b1beb to
03c55c4
Compare
We previously used Paths to de-dupe model sources in the ModelAssembler, but weren't using absolute paths to normalize paths before de-duping. This caused issues when smithy-build.json files defined imports or sources and sources were provided to the Smithy CLI as positional arguments. We now normalize files provided to the model assembler using toAbsolutePath to ensure that relative and absolute paths are de-duped. Further, this commit adds the same treatment to SmithyBuild sources. This isn't needed to de-dupe for the assembler, but rather to de-dupe files when generating the sources plugin. Without this, the sources plugin would fail due to conflicting files that look different because one might be absolute and another might be relative, but are the same. Integration test cases were added to the Smithy CLI to test this because it does not appear reliable to change the current working directory.
Printing from the CLI previously often resulted in hard to read output because messages were flushed on each newline rather than each log message/write to stdout/stderr, causing contiguous strings of text that contain newlines (like model validation events) to be interleaved between threads. This change updates the CLI to use a PrintWriter instead of a PrintStream to fix this. Additionally, added a new API for creating grouped sections of text from a CliPrinter that includes styling methods. Forcing color/no_color is now done only through environment variables (FORCE_COLOR, NO_COLOR). This matches lots of other CLI tools, and removes the clutter of the old options from help output. If the old options are used, a warning is logged and they are ignored. The way in which colors are detected and rendered is updated too. This change introduces an Ansi enum that contains AUTO, FORCE_COLOR, and NO_COLOR members. AUTO will auto-detect whether colors are supported using a heuristic, and dispatch to FORCE_COLOR and NO_COLOR based on if colors are supported. CliPrinter instances have a reference to Ansi and use it to style text + provide a public API to get the Ansi color setting. The hueristic used to detect colors is now: 1. Is FORCE_COLOR set? colors 2. Is NO_COLOR set? no colors 3. Is TERM set and set to "dumb"? no colors 4. Is TERM null and the OS is Windows? no colors 5. Is a System console detected (i.e., interactive CLI that isn't redirected)? colors 6. no colors I also updated checkstyle to allow for empty methods with braces on the same line to accomodate some of the implementations I was added. I then went and made similar changes to classes in the CLI package to use this approach.
mtdowling
force-pushed
the
cli-dependencies-rebase
branch
from
a6f02a9 to
85a1aef
Compare
Per request, adding back support for these arguments. This was achieved by separating the ColorFormatter from the CliPrinter. Because CLI arguments aren't fully parsed when bootstrapping the CLI and there needs to be a resolved ColorFormatter to create an Environment to give to a comment, I added a ColorFormatter interface that is implemented by AnsiColorFormatter (formally Ansi). The CLI will create an instance of a ColorFormatter that defers whether color is supported or not by creating a custom ColorFormatter that queries StandardOptions#colorSetting before each write.
mtdowling
force-pushed
the
cli-dependencies-rebase
branch
from
85a1aef to
b1e733b
Compare
JordonPhillips
requested changes
Dec 12, 2022
smithy-cli/src/main/java/software/amazon/smithy/cli/commands/CleanCommand.java
Outdated
Show resolved
Hide resolved
smithy-cli/src/main/java/software/amazon/smithy/cli/commands/WarmupCommand.java
Outdated
Show resolved
Hide resolved
JordonPhillips
approved these changes
Dec 13, 2022
When using Gradle, Gradle should handle dependency resolution, not the CLI. If we detect that Gradle is wrapping the CLI, dependency resolution is ignored. This helps ensure that previous versions of the Gradle plugin do not allow dependency resolution to occur, which would not work well with Gradle and could break users when they upgrade to a newer version of the Gradle plugin.
sugmanue
approved these changes
Dec 13, 2022
Some applications might not want to automatically load a detected smithy-build.json file. For example, if you're validating that a JAR built from a package correctly defines all of its dependencies correctly, then loading "sources" alongiside this JAR can lead to conflicts. --no-config can be passed to disable automatically loading a detected smithy-build.json file. Passing this option and a -c parameter will fail validation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds the ability to resolve dependencies in the Smithy CLI. This allows end users to author models, generate code, and other activities from the Smithy CLI without needing to use tools like Gradle or Maven. This lowers the up-front learning curve of most Smithy use cases to author models and generate code.
Writing Java library code, model build plugins, or transforms is out of scope of the Smithy CLI. Such use cases require the use of tools like Gradle or Maven.
To support dependency resolution in the CLI, this commit also introduces the
--versionoption to display the current version.The
smithy cleancommand was added since it was very useful to test dependency resolution by removing the classpath-cache file to force them to be re-resolved.Using the Smithy CLI directly means that tools like Gradle aren't automatically detecting the model directory and adding sources. To account for this, the
sourcesproperty was added to smithy-build.json. It acts likeimports, but the models contained insourcesare considered part of the model being built rather than just a dependeny (i.e., the sources plugin will contain these models).To actually test maven resolution, this commit also adds integration tests for the CLI (./gradlew :smithy-cli:integ).
Other changes:
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.