Feature: Option to skip hooks #1127

noirbizarre · 2022-06-10T09:16:12Z

Pull Request Check List

A news fragment is added in news/ describing what is new.
Test cases added for changed code.

Describe what you have changed in this PR.

This PR is a proposal at implementing #948.
If provides 2 parameters:

--skip-hooks that will skip any hook (or pre/post script) involved in the execution
--skip which can take any arbitrary task or hook name(s) and skip it/them

Those 2 parameters have been added to the following commands

run
install
lock
build
init

The --skip can be used many times. The following commands are all equivalents

--skip=pre_task1,task2
--skip pre_task1,task2
--skip=pre_task1 --skip=task2
--skip pre_task1 --skip task2

The both options can be used together:

pdm run --skip-hooks --skip=taskn my-composite-task

frostming · 2022-06-10T09:35:04Z

Those 2 parameters have been added to the following commands

run

install

lock

build

init

That is not complete, the following commands also call hooks:

add
update
sync
remove
publish (new in 2.0)

Besides, can we merge the two options --skip-hooks and --skip into one, something like --skip=:all? And ideally, the hooks should also be able to control with env vars.

noirbizarre · 2022-06-10T10:04:40Z

I'll add the option to those extra commands and I think documentation needs an update on this (they are not listen) or at least an explanation that signals does not match the commands but the actions executed by commands (if I understand well, install can trigger pre_lock and post_lock...). I'll try adding some lifecycle documentation somewhere (or update it if it already exists)

For the :all keyword, I'll do as well as :pre and :post with:

:all: skip all hooks
:pre: skip all pre hooks
:post: skip all post hooks

frostming · 2022-06-10T10:07:45Z

I'll try adding some lifecycle documentation somewhere (or update it if it already exists)

A Lifecycle documentation would be superb!

For the :all keyword, I'll do as well as :pre and :post with:

:all: skip all hooks
:pre: skip all pre hooks
:post: skip all post hooks

Yeah, I love the shortcuts

codecov-commenter · 2022-06-11T21:24:29Z

Codecov Report

Attention: Patch coverage is 97.97980% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.58%. Comparing base (4563185) to head (0dc1ed8).

Files with missing lines	Patch %	Lines
pdm/cli/options.py	86.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1127      +/-   ##
==========================================
+ Coverage   84.36%   84.58%   +0.21%     
==========================================
  Files          79       80       +1     
  Lines        6958     7030      +72     
  Branches     1652     1663      +11     
==========================================
+ Hits         5870     5946      +76     
+ Misses        733      731       -2     
+ Partials      355      353       -2

Flag	Coverage Δ
unittests	`84.36% <97.97%> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

frostming

@noirbizarre Thanks for your work on that feature. The code is of high quality and your Python skills are awesome. I've made some comments and improvement suggestions. Also, would you like to write a new documentation page about hooks?

pdm/cli/commands/run.py

pdm/cli/hooks.py

pdm/cli/options.py

news/1127.feature.md

tests/cli/test_hooks.py

Hnasar · 2022-06-12T11:00:33Z

@frostming I've been meaning to add a bug for a while that it seems like the relevant hooks aren't called from pdm-pep517 (e.g. if you add a pre_build hook it doesn't get called from python -m build).

If hooks are a fundamental part of PDM (and not just a part of the CLI) would it make sense to move HookManager into pdm-pep517 and then import it here?

frostming · 2022-06-12T11:05:25Z

@frostming I've been meaning to add a bug for a while that it seems like the relevant hooks aren't called from pdm-pep517 (e.g. if you add a pre_build hook it doesn't get called from python -m build).

If hooks are a fundamental part of PDM (and not just a part of the CLI) would it make sense to move HookManager into pdm-pep517 and then import it here?

I have a different opinion on that. Lifecycle scripts should be considered part of the package manager PDM, i.e. the frontend, rather than pdm-pep517, the backend. Like in node.js world, Yarn and npm support different lifecycle scripts. If you use other frontends like build, they may not call the hooks.

Besides, pdm-pep517, as a backend, only participates in the build process, it has nothing to do with install/lock/init.

…project#1127) fix pdm-project#948

… skip list

noirbizarre · 2022-06-13T09:44:30Z

@noirbizarre Thanks for your work on that feature. The code is of high quality and your Python skills are awesome. I've made some comments and improvement suggestions. Also, would you like to write a new documentation page about hooks?

👆🏼 Thanks 🙏🏼 🙏🏼

I applied changes/suggestions and rebased on the latest dev to resolve conflicts and squashed some commits to make history more consistent.
Do you want the hooks documentation in this PR or in a separate one (because it might take some times to write a good one and holding the PR might lead to more conflict solving).

Another question: do you me to add a pre_publish and post_publish hook ? Seems to be some legit and useful hooks

frostming · 2022-06-13T09:53:03Z

I applied changes/suggestions and rebased on the latest dev to resolve conflicts and squashed some commits to make history more consistent.

Thanks, hope my updates to the dev branch don't cause too much trouble. The CI seems passing.

Do you want the hooks documentation in this PR or in a separate one

A separate PR would be okay.

do you me to add a pre_publish and post_publish hook ? Seems to be some legit and useful hooks

Sure, go ahead for it.

noirbizarre · 2022-06-13T10:03:06Z

I applied changes/suggestions and rebased on the latest dev to resolve conflicts and squashed some commits to make history more consistent.

Thanks, hope my updates to the dev branch don't cause too much trouble. The CI seems passing.

As soon as I understood dev has been rebased, no problem, just a rebase on my side too 👍🏼

Do you want the hooks documentation in this PR or in a separate one

A separate PR would be okay.

OK, let's go this way 👍🏼

do you me to add a pre_publish and post_publish hook ? Seems to be some legit and useful hooks

Sure, go ahead for it.

Great, I'll add them in another PR too

So the PR should be good to merge and I'll submit extra changes (pre_publish/post_publish and extended hooks documentation) in separate pull-requests 👍🏼

(And thanks for the reactivity, it's a pleasure contributing with quick feedback)

frostming

LGTM

noirbizarre · 2022-06-13T12:34:08Z

Great 🎊
I'll let you merge, I don't have the proper permissions to do so.

* fix(scripts): allow pdm test command to receive path arguments * feat(hooks): added a `--skip` option to skipp scripts and hooks (#1127) fix #948 * feat(hooks): use PDM_SKIP_HOOKS environement variable as fallback for skip list

* feat(core): Use tomllib on Python 3.11 (#1072) * docs: 📝 Fix typo in `pip install pdm` description (#1061) * Use tomllib on Python 3.11 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * news * use a compatibility module * missed one import Co-authored-by: t106362512 <[email protected]> Co-authored-by: hauntsaninja <> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(core): Replace halo, click, and termcolor with rich (#1091) * feat(core): Use `unearth` as the backend to find and download packages (#1096) * perf(resolver): Speed up the resolution with lazy find_matches (#1098) * Improve the output of installing packages * Use confirm instead of ask * feat(core): New command: pdm publish (#1107) * Use rich handler for logging to stderr * feat(scripts): added composite tasks support (#1117) * feat(core): Add option to skip hooks (#1127) * fix(scripts): allow pdm test command to receive path arguments * feat(hooks): added a `--skip` option to skipp scripts and hooks (#1127) fix #948 * feat(hooks): use PDM_SKIP_HOOKS environement variable as fallback for skip list * feat(core): Support setup.py import (#1137) * Update completion script * fix(resolution): fix a bug that versions with local part can't be found and installed Close #1093 * feat(core): forbid editable depenencies in project table (#1140) * Make the error message more friendly * doc: improve the docs about dependencies * doc: add CLI reference doc * doc: use asciiart as the program description * chore: remove remaining artifacts from #1127 (#1152) * Feature: complete lifecycle signals and documentation (#1147) * feat(hooks): Added pre-publish hook * refactor(hooks): dynamic signal/hooks listing avoiding double declaration * feat(hooks): added (pre|post)_script and (pre|post)_run hooks * doc(hooks): added lifecycle and hooks documentation * review fix * fix(tests): add and use the _echo fixture for cross-plateform and concise test echos * refactor(hooks): automatically register the script handler for all hooks * feat: Update pdm-pep517 to 1.0 (#1153) * fix(scripts): merge the Script and Description field from listing (#1151) * feat: fetch the candidate hashes concurrently (#1154) * feat: fetch the candidate hashes concurrently * add news * Feat/respect-source-order (#1155) * doc: restructure the docs about project metadata and build configuration * parse pep 621 metadata to avoid build (#1156) * feat: Remove the compatible support for pdm legacy metadata (#1157) * fix(config): use platform standard directories for all PDM directories (#1161) Fixes #1150 * fix(#1156): only trust parsing result when all are static * New build configuration table * chore: added a tox.ini file for easier local testing against all Python versions (#1160) * feat(CLI): Yarn-like root scripts fallback (#1159) * feat(hooks): added a post_use hook (#1163) Co-authored-by: Shantanu <[email protected]> Co-authored-by: t106362512 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Axel H <[email protected]>

* fix(scripts): allow pdm test command to receive path arguments * feat(hooks): added a `--skip` option to skipp scripts and hooks (#1127) fix #948 * feat(hooks): use PDM_SKIP_HOOKS environement variable as fallback for skip list

noirbizarre added a commit to noirbizarre/pdm that referenced this pull request Jun 10, 2022

Add the news file for pdm-project#1127

aa10c04

noirbizarre force-pushed the feature/skip branch from bbee0bc to d830175 Compare June 10, 2022 10:03

frostming added this to the Release 2.0 milestone Jun 11, 2022

noirbizarre force-pushed the feature/skip branch from 970a4e3 to d3e8337 Compare June 11, 2022 21:14

frostming requested changes Jun 12, 2022

View reviewed changes

pdm/cli/commands/run.py Outdated Show resolved Hide resolved

pdm/cli/hooks.py Outdated Show resolved Hide resolved

pdm/cli/options.py Outdated Show resolved Hide resolved

frostming changed the title ~~Feature/skip~~ Feature: Option to skip hooks Jun 12, 2022

frostming reviewed Jun 12, 2022

View reviewed changes

news/1127.feature.md Outdated Show resolved Hide resolved

Hnasar reviewed Jun 12, 2022

View reviewed changes

tests/cli/test_hooks.py Outdated Show resolved Hide resolved

frostming force-pushed the dev branch from 1c376a8 to 4563185 Compare June 13, 2022 08:32

noirbizarre added 3 commits June 13, 2022 11:37

fix(scripts): allow pdm test command to receive path arguments

d7f8275

feat(hooks): added a --skip option to skipp scripts and hooks (pdm-…

4a2888f

…project#1127) fix pdm-project#948

feat(hooks): use PDM_SKIP_HOOKS environement variable as fallback for…

0dc1ed8

… skip list

noirbizarre force-pushed the feature/skip branch from f1c9569 to 0dc1ed8 Compare June 13, 2022 09:38

frostming approved these changes Jun 13, 2022

View reviewed changes

frostming merged commit f276147 into pdm-project:dev Jun 13, 2022

noirbizarre deleted the feature/skip branch June 13, 2022 14:35

noirbizarre mentioned this pull request Jun 20, 2022

Feature: complete lifecycle signals and documentation #1147

Merged

2 tasks

noirbizarre added a commit to noirbizarre/pdm that referenced this pull request Jun 22, 2022

chore: remove remaining artifacts from pdm-project#1127

ffaef8d

noirbizarre mentioned this pull request Jun 22, 2022

chore: remove remaining artifacts from #1127 #1152

Merged

frostming pushed a commit that referenced this pull request Jun 22, 2022

chore: remove remaining artifacts from #1127 (#1152)

4d5c9cf

frostming pushed a commit that referenced this pull request Jun 24, 2022

chore: remove remaining artifacts from #1127 (#1152)

e1eab12

frostming pushed a commit that referenced this pull request Jun 24, 2022

chore: remove remaining artifacts from #1127 (#1152)

21ef866

frostming pushed a commit that referenced this pull request Jun 28, 2022

chore: remove remaining artifacts from #1127 (#1152)

45c66a8

frostming pushed a commit that referenced this pull request Jun 28, 2022

chore: remove remaining artifacts from #1127 (#1152)

b5834c6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Option to skip hooks #1127

Feature: Option to skip hooks #1127

noirbizarre commented Jun 10, 2022 •

edited

Loading

frostming commented Jun 10, 2022 •

edited

Loading

noirbizarre commented Jun 10, 2022 •

edited

Loading

frostming commented Jun 10, 2022

codecov-commenter commented Jun 11, 2022 •

edited by codecov bot

Loading

frostming left a comment •

edited

Loading

Hnasar commented Jun 12, 2022

frostming commented Jun 12, 2022 •

edited

Loading

noirbizarre commented Jun 13, 2022 •

edited

Loading

frostming commented Jun 13, 2022

noirbizarre commented Jun 13, 2022 •

edited

Loading

frostming left a comment

noirbizarre commented Jun 13, 2022

Feature: Option to skip hooks #1127

Feature: Option to skip hooks #1127

Conversation

noirbizarre commented Jun 10, 2022 • edited Loading

Pull Request Check List

Describe what you have changed in this PR.

frostming commented Jun 10, 2022 • edited Loading

noirbizarre commented Jun 10, 2022 • edited Loading

frostming commented Jun 10, 2022

codecov-commenter commented Jun 11, 2022 • edited by codecov bot Loading

Codecov Report

frostming left a comment • edited Loading

Choose a reason for hiding this comment

Hnasar commented Jun 12, 2022

frostming commented Jun 12, 2022 • edited Loading

noirbizarre commented Jun 13, 2022 • edited Loading

frostming commented Jun 13, 2022

noirbizarre commented Jun 13, 2022 • edited Loading

frostming left a comment

Choose a reason for hiding this comment

noirbizarre commented Jun 13, 2022

noirbizarre commented Jun 10, 2022 •

edited

Loading

frostming commented Jun 10, 2022 •

edited

Loading

noirbizarre commented Jun 10, 2022 •

edited

Loading

codecov-commenter commented Jun 11, 2022 •

edited by codecov bot

Loading

frostming left a comment •

edited

Loading

frostming commented Jun 12, 2022 •

edited

Loading

noirbizarre commented Jun 13, 2022 •

edited

Loading

noirbizarre commented Jun 13, 2022 •

edited

Loading