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

CMakeDeps: remove warning about existing alias #14644

Merged

Conversation

BobIsOnFire
Copy link
Contributor

@BobIsOnFire BobIsOnFire commented Sep 1, 2023

Changelog: Fix: CMakeDeps: Remove "Target name ... already exists" warning about duplicating aliases.
Docs: omit

This PR benefits #14560 (and #12958)

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.

@memsharded
Copy link
Member

Thanks for your contribution @BobIsOnFire

I think I'd advocate for removing the warning completely, I think it is there to protect against colliding aliases from different packages, but that check should better be done globally, I'll check with the team.

@memsharded
Copy link
Member

memsharded commented Sep 5, 2023

It seems this warning is more problematic than helpful for most users. Lets do the following:

  • Please remove the warnings completely from CMake code (do you want to do it in this same PR @BobIsOnFire ?). Note: there is a test test_collide_component_alias that explicitly checks for this warning, this test can remove the check, and add a comment that this is possible (alias collision), and it will not warn at this moment.
  • As a future thing, we could consider checking in the CMakeDeps python code possible aliases colisions and warn, or even error from there.

@BobIsOnFire BobIsOnFire force-pushed the fix/silence-alias-warning branch from 22f9683 to c4fb6ed Compare September 5, 2023 08:39
@BobIsOnFire BobIsOnFire force-pushed the fix/silence-alias-warning branch from c4fb6ed to 099b32e Compare September 5, 2023 08:40
@BobIsOnFire BobIsOnFire changed the title Silence warning about existing alias if QUIET is passed CMakeDeps: remove warning about existing alias Sep 5, 2023
@BobIsOnFire
Copy link
Contributor Author

Hi @memsharded

Thanks! I have updated branch to remove warning / adjust tests.

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.

3 participants