Skip to content

Command plugin should skip irrelevant targets when run without --target argument #798

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

Conversation

simonjbeaumont
Copy link
Collaborator

Motivation

In #795 we adjusted the logic of the command plugin because errors were not propagating properly. As a result, we regressed behaviour when the command plugin was run without any --target argument. In this mode, the plugin runs on all targets. When it detects a misconfigured target it errors, as it should. However, the classification of misconfigured target included targets that were not intended for code generation at all, resulting in errors running the command plugin on packages with others targets.

Modifications

  • Add command plugin generation step to integration test
  • If the command plugin is running without a --target argument and cannot find any of the required files, skip that target.

Result

Test Plan

This PR includes two commits. The first adds a step to the integration test, which runs the command plugin on the integration test package. This will fail in CI. Then the second commit will provide the fix, which should pass in CI. The commits will then be squash merged.

@simonjbeaumont simonjbeaumont added the 🔨 semver/patch No public API change. label Jul 23, 2025
@simonjbeaumont
Copy link
Collaborator Author

Good... the canary commit shows the failure:

error: Issues with required files:
- No config file found in the target named 'MockTransportClient'. Add a file called 'openapi-generator-config.yaml' or 'openapi-generator-config.yml' to the target's source directory. See documentation for details.
- No OpenAPI document found in the target named 'MockTransportClient'. Add a file called 'openapi.yaml', 'openapi.yml' or 'openapi.json' (can also be a symlink) to the target's source directory. See documentation for details..

— source: https://github.com/apple/swift-openapi-generator/actions/runs/16471435605/job/46561409290?pr=798#step:4:752

But... annoyingly we get badly ordered output since there's a mix of stderr and stdout logging from the plugin and tool. A well behaved tool should log to stderr and reserve stdout for actual output that could e.g. be piped to other processes.

I'll add one more commit to fix that before adding the commit for the actual fix.

@simonjbeaumont simonjbeaumont force-pushed the sb/command-plugin-gracefully-skip-targts branch from e496d38 to 8eaf620 Compare July 23, 2025 13:22
@simonjbeaumont
Copy link
Collaborator Author

OK, great. This is now failing as we expect:

...
- ✅ OpenAPI code generation for target 'Server' successfully completed.
Considering target 'MockTransportClient':
- Trying OpenAPI code generation.
- OpenAPI code generation failed with error.

— source: https://github.com/apple/swift-openapi-generator/actions/runs/16471883278/job/46562960013?pr=798#step:4:818

I'll add the fix.

@simonjbeaumont simonjbeaumont changed the title Add command plugin generation step to integration test Command plugin should skip irrelevant targets when run without --target argument Jul 23, 2025
@simonjbeaumont
Copy link
Collaborator Author

And, now things are working as expected:

...
- ✅ OpenAPI code generation for target 'Server' successfully completed.
Considering target 'MockTransportClient':
- Trying OpenAPI code generation.
- Skipping because target isn't configured for OpenAPI code generation.
Considering target 'MockTransportServer':
- Trying OpenAPI code generation.
- Skipping because target isn't configured for OpenAPI code generation.
** ✅ Successfully built integration test package.

— source: https://github.com/apple/swift-openapi-generator/actions/runs/16472015174/job/46563435636?pr=798#step:4:804

@simonjbeaumont simonjbeaumont requested a review from czechboy0 July 23, 2025 13:32
@simonjbeaumont simonjbeaumont marked this pull request as ready for review July 23, 2025 13:36
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Approving, feel free to decide what to do with the two comments

@simonjbeaumont simonjbeaumont enabled auto-merge (squash) July 23, 2025 14:25
@simonjbeaumont simonjbeaumont merged commit f4f6f9b into apple:main Jul 23, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manually generating code fails if package includes additional targets
2 participants