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.

Sign up for GitHub

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

Jump to bottom

Smithy models with subdirectories no-longer resolve correctly #1859

Closed
Xtansia opened this issue Jul 17, 2023 · 4 comments · Fixed by #1860
Closed

Smithy models with subdirectories no-longer resolve correctly #1859

Xtansia opened this issue Jul 17, 2023 · 4 comments · Fixed by #1860
Labels
bug This issue is a bug.

Comments

@Xtansia
Copy link
Contributor

Xtansia commented Jul 17, 2023

After the changes in 5e864a1, when using a directory as the source (ie. via the gradle plugin), any smithy files that were in subdirectories are no longer resolved.

Is this an intended removal of this behaviour, as I'm unsure if the recursive resolution was intended/documented originally?

We are relying on this behaviour for organisational purposes in our repo here for example: https://github.com/opensearch-project/opensearch-api-specification/tree/main/model
@Xtansia
Copy link
Contributor Author

Xtansia commented Jul 17, 2023

Actually after double checking the smithy-build.json docs for sources, it states:

Provides a list of relative files or directories that contain the models that are considered the source models of the build. When a directory is encountered, all files in the entire directory tree are added as sources. Sources are relative to the configuration file.

entire directory tree to me implies a recursive search, so I believe this is truly a regression in behaviour.

@Xtansia
Copy link
Contributor Author

Xtansia commented Jul 17, 2023

Clarifying after looking closer, it's not the resolution that's failing as such. The OpenAPI plugin/projection works correctly, however the changes means not all the smithy files are copied into the JAR for the project, resulting in the validate JAR task failing. This also means the sources projection is incomplete.

@milesziemer
Copy link
Contributor

milesziemer commented Jul 17, 2023 •
edited
Loading

Could you provide steps to reproduce this? I can't reproduce with a local copy of your codebase updated to smithy 1.34, running ./gradlew build.

Edit: I didn't realize this was using latest changes in git, rather than 1.34.0 release, I am able to reproduce.

@milesziemer milesziemer added the bug This issue is a bug. label Jul 17, 2023
milesziemer added a commit to milesziemer/smithy that referenced this issue Jul 17, 2023
@milesziemer
Fix building models in subdirs
e7076e1 
Fixes smithy-lang#1859

Modifies the changes in smithy-lang#1851,
specifically `SmithyBuild::addSources`, to not check subdirectories for
Smithy files. The previous behavior was to just add whatever path was
given to the method, *not* everything in subdirectories. However, the
change didn't recursively search subdirectores, so you could get build
errors due to missing shapes when running the sources plugin.

This change reverts the behavior of `SmithyBuild::addSource` to only
add whichever path it was given, but still checks to see if the path
is a valid Smithy model.
@milesziemer milesziemer mentioned this issue Jul 17, 2023
Merged
@Xtansia
Copy link
Contributor Author

Xtansia commented Jul 17, 2023

@milesziemer Yes, I probably could have been clearer about this referring to unreleased changes. I see you've now been able to reproduce and PR a fix.

For posterity I've still created a minimal repro here: https://github.com/Xtansia/smithy-1859-repro

milesziemer added a commit that referenced this issue Jul 18, 2023
@milesziemer
Fix building models in subdirs (#1860)
e76d3a0 
Fixes #1859

Modifies the changes in #1851,
specifically `SmithyBuild::addSources`, to not check subdirectories for
Smithy files. The previous behavior was to just add whatever path was
given to the method, *not* everything in subdirectories. However, the
change didn't recursively search subdirectores, so you could get build
errors due to missing shapes when running the sources plugin.

This change reverts the behavior of `SmithyBuild::addSource` to only
add whichever path it was given, but still checks to see if the path
is a valid Smithy model.
syall pushed a commit to Xtansia/smithy that referenced this issue Aug 11, 2023
@milesziemer
Fix building models in subdirs (smithy-lang#1860)
6e444f5 
Fixes smithy-lang#1859

Modifies the changes in smithy-lang#1851,
specifically `SmithyBuild::addSources`, to not check subdirectories for
Smithy files. The previous behavior was to just add whatever path was
given to the method, *not* everything in subdirectories. However, the
change didn't recursively search subdirectores, so you could get build
errors due to missing shapes when running the sources plugin.

This change reverts the behavior of `SmithyBuild::addSource` to only
add whichever path it was given, but still checks to see if the path
is a valid Smithy model.
alextwoods pushed a commit to alextwoods/smithy that referenced this issue Sep 15, 2023
@milesziemer @alextwoods
Fix building models in subdirs (smithy-lang#1860)
9b0a396 
Fixes smithy-lang#1859

Modifies the changes in smithy-lang#1851,
specifically `SmithyBuild::addSources`, to not check subdirectories for
Smithy files. The previous behavior was to just add whatever path was
given to the method, *not* everything in subdirectories. However, the
change didn't recursively search subdirectores, so you could get build
errors due to missing shapes when running the sources plugin.

This change reverts the behavior of `SmithyBuild::addSource` to only
add whichever path it was given, but still checks to see if the path
is a valid Smithy model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue is a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix building models in subdirs milesziemer/smithy
2 participants
@Xtansia @milesziemer