Skip to content

add uninstall target for cmake #2265

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
Merged
merged 2 commits into from
Jul 31, 2020
Merged

add uninstall target for cmake #2265

merged 2 commits into from
Jul 31, 2020

Conversation

acxz
Copy link
Contributor

@acxz acxz commented Jun 25, 2020

Resolves #2264

@EricCousineau-TRI EricCousineau-TRI mentioned this pull request Jul 1, 2020
Closed
@acxz
Copy link
Contributor Author

acxz commented Jul 26, 2020

@wjakob can you take a look at this PR?

@acxz
Copy link
Contributor Author

acxz commented Jul 26, 2020

based on disc on #2160 gonna close/reopen this PR

@acxz acxz closed this Jul 26, 2020
@acxz acxz reopened this Jul 26, 2020
@henryiii
Copy link
Collaborator

henryiii commented Jul 30, 2020 •
edited
Loading

One tiny change - could you add a check to make sure this only gets added when built as the main project? PYBIND11_MASTER_PROJECT, IIRC. If a parent project wants to add this, they should do it themselves. Also, target names are global, so this addition would break projects that have an uninstall target defined later, unless we add this check. Uninstall targets are a little tricky (since they take a list of files and delete them), which is why CMake doesn't have built in support for it even now.

I'll help if it conflicts with #2338.

@henryiii henryiii self-requested a review July 30, 2020 00:17
henryiii
henryiii requested changes Jul 30, 2020
View reviewed changes
@acxz
Copy link
Contributor Author

acxz commented Jul 30, 2020

The CI errors seem unrelated

acxz and others added 2 commits July 30, 2020 20:30
@acxz @henryiii
add uninstall target for cmake
d9b31c5
@acxz @henryiii
only add target when built as master project
590bc91 
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
@henryiii
Copy link
Collaborator

Rebased after #2338 and force-pushed.

@henryiii henryiii merged commit 6f6e939 into pybind:master Jul 31, 2020
@henryiii
Copy link
Collaborator

Thanks!

@acxz acxz deleted the uninstall_target branch July 31, 2020 01:34
@henryiii henryiii mentioned this pull request Jul 31, 2020
Merged
@rwgk rwgk mentioned this pull request Feb 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henryiii henryiii henryiii approved these changes

Assignees
No one assigned
Labels
build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Create uninstall target for CMake
2 participants
@acxz @henryiii