zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
2.89k stars 124 forks source link

feat: call make uninstall when removing a plugin/snippet #468

Closed psprint closed 6 months ago

psprint commented 1 year ago

Description

If the uninstall target is there in the Makefile of the removed plugin/snippet, it'll be run as: make -C {plugin/snippet dir} uninstall before removing the dir (with zinit delete).

Motivation and Context

@vladdoster mentioned (#333) that --prefix=$ZPFX installed plugins aren't cleaned up lik --prefix=$PWD ones, so I added make uninstall support.

Related Issue(s)

333

Usage examples

# Will first uninstall ctags from the --prefix=($ZPFX,$PWD,etc.) path
zinit delete -y unversal-ctags/ctags

2023-01-29-122000_1901x884_scrot

How Has This Been Tested?

Types of changes

Checklist:

vladdoster commented 1 year ago

DRAFT RESPONSE

This is a good idea. However, I still favor using plugin source directory instead of $ZPFX as a prefix for the following reasons.

psprint commented 1 year ago

What are the reasons?

czw., 2 lut 2023, 03:54 użytkownik vladislav doster < @.***> napisał:

DRAFT RESPONSE

This is a good idea. However, I still favor using plugin source directory instead of $ZPFX as a prefix for the following reasons.

-

— Reply to this email directly, view it on GitHub https://github.com/zdharma-continuum/zinit/pull/468#issuecomment-1413078083, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOE4CD565JB6YP3UP5G6X3WVMOW3ANCNFSM6AAAAAAUKGFKCQ . You are receiving this because you authored the thread.Message ID: @.***>

psprint commented 1 year ago

Ping?

vladdoster commented 1 year ago

@psprint,

As I have said before, I will not review PRs with failing CI checks except when the CI tests are broken or you can't get a check to pass.

So, is there an issue generating the documentation?

psprint commented 1 year ago

Yes I cannot generate docs because there is no docker for PCLinuxOS. I've tried RPM for Fedora but it wasn't compatible.

vladdoster commented 1 year ago

@psprint,

This doesn't work for an example you gave in #458

zi id-as as"null" from"gitlab.matrix.org" configure make'install' for matrix-org/olm

I tried your commit before I changed it. Seems to be since it uses cmake?

Screenshot 2023-04-28 at 23 18 46
psprint commented 1 year ago

The run hooks function requires a parameter on $5 - the path to the plugin. It also needs a Makefile with uninstall target.

sob., 29 kwi 2023, 06:21 użytkownik vladislav doster < @.***> napisał:

This doesn't work for example you gave in #458 https://github.com/zdharma-continuum/zinit/pull/458

zi id-as as"null" from"gitlab.matrix.org" configure make'install' for matrix-org/olm

I tried your commit before I changed it. Seems to be since it uses cmake?

[image: Screenshot 2023-04-28 at 23 18 46] https://user-images.githubusercontent.com/10052309/235283128-0a8e60e7-d71a-4a59-9c74-7be33961a1a5.png

— Reply to this email directly, view it on GitHub https://github.com/zdharma-continuum/zinit/pull/468#issuecomment-1528646664, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOE4CACMNCFOZTHUJVXMXDXDSJLXANCNFSM6AAAAAAUKGFKCQ . You are receiving this because you were mentioned.Message ID: @.***>

psprint commented 1 year ago

@vladdoster It turns out that CMake doesn't support uninstall target like Autotools do. However, one can simply rm -f files in install_manifest.txt. So this could be added to the PR. https://stackoverflow.com/questions/41471620/cmake-support-make-uninstall

psprint commented 1 year ago

@vladdoster: I've resolved conflicts (fresh branch) and added CMake support. The commit is all lowercase and has the prefix, I cannot generate zshelldocs because there's no docker for my OS.

vladdoster commented 1 year ago

@vladdoster: I've resolved conflicts (fresh branch) and added CMake support. The commit is all lowercase and has the prefix, and I cannot generate zshelldocs because there's no docker for my OS.

@psprint,

FWIW, You don't need Docker to generate the documentation. I don't use Docker, either, because it is painfully slow (on macOS atleast) due to virtualization.

OS time
macOS 1m 41s
Docker 6m 15s

I use the following:

zi for id-as lbin'!build/zsd*' light-mode make'--always-make' null @zdharma-continuum/zshelldoc
Screenshot 2023-04-30 at 22 33 56

Note If working on zshelldoc, add --always-make, so zinit update --urge will recreate the files.

Screenshot 2023-04-30 at 22 28 54
vladdoster commented 6 months ago

Closing due to the changes were merged in #616 (code found here).