Skip to content

Conversation

bripeticca
Copy link
Contributor

Fixes #8398

Used package dependencies of a manifest were considered trait-guarded if a target dependency in the manifest:

  1. used a product from said package and
  2. was trait-guarded

regardless as to whether the same package dependency was used elsewhere (whether in the same target or another in the same manifest) and was not guarded by traits.

This change ensures that this is considered when doing the calculations for package dependencies that should be omitted if every target dependency that uses said package dependency is guarded.

- Modify the calculation done for target dependencies that
  are trait-guarded, and consider cases where target deps
  that reference the same package could have both instances
  of being trait-guarded and not trait-guarded.
- Expand on test case that checks for the appropriate
  trait-guarded target deps

TODO:
- update method descriptions for trait-guarded target deps
- add more test cases to ensure correctness
- Add more checks in Manifest tests to assure that
  dependencies are only considered guarded/unused if
  every instance of a target dependency referring to
  a package is guarded/unused
- Add description to new traitGuardedTargetDependencies
  methods/implementations.
@bripeticca
Copy link
Contributor Author

@swift-ci please test

- Fix the formatting for `Manifest.swift`
- Clarify what the return values represent for the `traitGuardedTargetDependencies` methods
- Cleanup + renaming
@bripeticca
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix! Just one super minor formatting nit

@bripeticca
Copy link
Contributor Author

@swift-ci please test

@bripeticca
Copy link
Contributor Author

@swift-ci please test windows

1 similar comment
@bripeticca
Copy link
Contributor Author

@swift-ci please test windows

@MaxDesiatov
Copy link
Contributor

Good to merge?

@bripeticca bripeticca merged commit 5b30b32 into swiftlang:main Mar 26, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Traits] Used package dependency omitted when traits guard some target dependencies

2 participants