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

Fix false positive duplicate static products lint rule #2201

Conversation

kwridan
Copy link
Collaborator

@kwridan kwridan commented Dec 27, 2020

Resolves #2197

Short description πŸ“

The duplicate static products linter was incorrectly flagging duplicate static products between host app targets and their additional bundled products (such as watch apps, app clips, extensions, etc...).

Those additional targets can safely link the same static products as their host app due them being independent products.

Checklist βœ…

  • The code architecture and patterns are consistent with the rest of the codebase.
  • A developer other than the author has verified that the changes work as expected.
  • The changes have been tested following the documented guidelines.
  • The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
  • In case the PR introduces changes that affect users, the documentation has been updated.

Test Plan πŸ› 

  • Run swift build && swift run tuist generate --path fixtures/ios_app_with_watchapp2/
  • Verify no duplicate static products lint warning is produced

Resolved: tuist#2197

- The duplicate static products linter didn't take into account extensions, watch apps and app clips
- Those can safely link the same static products linked by a host app as they are independent products
- Updated the watch app fixture to include a local switch package to reproduce the previous false positive scenario

Test Plan:

- Run `swift build && swift run tuist generate --path fixtures/ios_app_with_watchapp2/`
- Verify no duplicate static products lint warning is produced
Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this so quickly @kwridan

@kwridan kwridan force-pushed the fixes/false-positive-duplicate-static-products-in-extensions branch from 6c4ed9a to 5102339 Compare December 27, 2020 08:22
- explicitly specifying the `-sdk` can yield build errors when building a scheme that contains multiple products for different platforms (e.g. App with a bundled watch app)
@kwridan
Copy link
Collaborator Author

kwridan commented Dec 27, 2020

I've had to update the acceptance test build commands (the ones in xcode.rb) as explicitly specifying -sdk when building schemes that contain multiple products of different platforms (like in the fixture I modified) can yield build errors. Instead the destination platform is specified to mitigate this. Did we encounter issues in the past that required explicitly specifying -sdk?

I did a sanity check on a few acceptance tests and that seemed to have worked, I guess we'll see how all of them fair on CI πŸ˜… - if that fails, will revert the fixture change or add a different one not run by acceptance tests :/

@kwridan
Copy link
Collaborator Author

kwridan commented Dec 27, 2020

looks like the change works for all fixtures on CI too πŸŽ‰ - the only CI failure is Rubocop but those look unrelated to this change :/

Screenshot 2020-12-27 at 10 24 39

@pepicrft pepicrft merged commit 55b669c into tuist:main Dec 28, 2020
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.

Importing Multi-Platform Packages In Extensions Throwing Lint Error
2 participants