Skip to content

add uninstall target for cmake - #2265

Merged
henryiii merged 2 commits into
pybind:masterfrom
acxz:uninstall_target
Jul 31, 2020
Merged

add uninstall target for cmake#2265
henryiii merged 2 commits into
pybind:masterfrom
acxz:uninstall_target

Conversation

@acxz

@acxzacxz commented Jun 25, 2020

Copy link
Copy Markdown
Contributor

Resolves#2264

@acxz

acxz commented Jul 26, 2020

Copy link
Copy Markdown
ContributorAuthor

@wjakob can you take a look at this PR?

@acxz

acxz commented Jul 26, 2020

Copy link
Copy Markdown
ContributorAuthor

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

@acxzacxz closed this Jul 26, 2020
@acxzacxz reopened this Jul 26, 2020
@henryiii

henryiii commented Jul 30, 2020

Copy link
Copy Markdown
Collaborator

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
Comment threadCMakeLists.txt Outdated
@acxz

acxz commented Jul 30, 2020

Copy link
Copy Markdown
ContributorAuthor

The CI errors seem unrelated

acxzand others added 2 commits July 30, 2020 20:30
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
@henryiii

Copy link
Copy Markdown
Collaborator

Rebased after #2338 and force-pushed.

@henryiii
henryiii merged commit 6f6e939 into pybind:masterJul 31, 2020
@henryiii

Copy link
Copy Markdown
Collaborator

Thanks!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Create uninstall target for CMake

2 participants

@acxz@henryiii