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

Facilitate sharing specs across multi-module builds. #432

Merged
merged 24 commits into from
Sep 13, 2022

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented Sep 2, 2022

This adds the necessary mechanisms and logic to facilitate sharing smithy specs in multi-module settings.

The first important part is the addition of a resource generation mechanism that adds the local smithy files to the output resources, and synthesise smithy files containing only metadata, keeping track of the namespaces that have been generated by smithy4s. Because this extends the openapi generation to be able to generate more resources, I've renamed/revamped the arguments in CodegenArgs and CodegenCommand to be more fitting.

Because the metadata in question is an array node, it is monoidal in nature and the values are aggregated when loading models. These values are used to skip code generation in downstream Smithy4s calls, as the downstream modules can trust that the upstream one will have generated the relevant package.

Then, the SBT plugin changes to load internal dependency outputs as local jars. This has the effect of pulling the models from upstream modules in the current model assembler, allowing users to depend on the shapes it contains.

resolves #420
resolves #391

TODOS :

  • Documentation
  • Opt-out SBT setting
  • [ ] Think about whether some other metadata should be added to track the version of Smithy4s that generated the sources, to try and protect against incompatibilities, as this mechanism could be extended to use Smithy4s in the same style as protobuf, where specs are packaged alongside the generated code. in a subsequent PR
  • Think really hard about the implications of all of this before committing : let's get it out there and collect feedback

This adds necessary the necessary mechanisms and logic to facilitate
sharing smithy specs in multi-module settings.

The first important part is the addition of a resource generation
mechanism that adds the local smithy files to the output resources,
and synthetise smithy files containing only metadata, keeping track
of the namespaces that have been generated by smithy4s.

Because the metadata in question is an array node, it is monoidal in
nature and the values are aggregated when loading models. These values
are used to skip code generation in downstream Smithy4s calls, as the
downstream modules can trust that the upstream one will have generated
the relevant package.

Then, the SBT plugin changes to load internal dependency outputs as
local jars. This has the effect of pulling the models from upstream
modules in the current model assembler, allowing users to depend on
the shapes it contains.
@Baccata Baccata marked this pull request as draft September 2, 2022 12:37
@Baccata Baccata added this to the 0.16 milestone Sep 2, 2022
@Baccata Baccata added the ‼️ usage breaking ‼️ Usage breaking (source or build) label Sep 2, 2022
Copy link
Member

@kubukoz kubukoz left a comment

Choose a reason for hiding this comment

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

I'm too braindead to read through this today, but might take a peek later

scala-steward and others added 12 commits September 3, 2022 13:10
Update nscplugin, sbt-scala-native, ... to 0.4.7
This exhaustively captures the codegenArgs as a way to invalidate
SBT's cache to retrigger codegen every time something changes.

This uses SBT's JsonFormat, converting os-lib's paths into file hashes.
In theory, when a local file changes (say, a dependency snapshot), the
cache should now be invalidated and trigger a regeneration.
This allows people who experiment with snapshot jars (published locally)
to get a better developer experience, by not requiring them to restart
SBT between spec changes.
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2022

CLA assistant check
All committers have signed the CLA.

@Baccata Baccata marked this pull request as ready for review September 9, 2022 07:08
Copy link
Contributor

@daddykotex daddykotex left a comment

Choose a reason for hiding this comment

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

I pushed a commit to address a doc issue, but otherwise it LGTM

modules/docs/src/01-overview/05-sharing-specs.md Outdated Show resolved Hide resolved
@@ -823,7 +825,7 @@ def genSmithyImpl(config: Configuration) = Def.task {
val outputDir = (config / genSmithyOutput).?.value
.getOrElse((config / sourceManaged).value)
.getAbsolutePath()
val openapiOutputDir =
val resourceOutputDir =
(config / genSmithyOpenapiOutput).?.value
Copy link
Contributor

Choose a reason for hiding this comment

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

working on the mill version: should we rename this to genResourcesOutput ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can always address this on my mill PR

did my comment make sense to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure you can go ahead and rename in your PR :)

@Baccata Baccata merged commit 000343c into 0.16.0 Sep 13, 2022
@Baccata Baccata deleted the multimodule-codegen branch September 13, 2022 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ usage breaking ‼️ Usage breaking (source or build)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants