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

More flexible hooks #254

Closed
jrosiek opened this issue Aug 2, 2024 · 3 comments · Fixed by #258
Closed

More flexible hooks #254

jrosiek opened this issue Aug 2, 2024 · 3 comments · Fixed by #258
Assignees

Comments

@jrosiek
Copy link

jrosiek commented Aug 2, 2024

Feature suggestion

Current state

  • Order of hook execution is fixed and arbitrary. Specifically, builtin hooks always execute before entry-point hooks.
  • The first hook method (initialize) is already used to execute business logic in builtin hooks (version hook). That means user cannot execute any logic nor capture Context before builtin hook.
  • format_version in SCM version hook does not have access to Context

Alternative proposals

  • Ability to specify hook execution order (integer) that is used to sort hooks before execution
  • Provide additional hook method that must not contain any business logic and can only be used to capture context (eg. pdm_set_context)
  • Provide context to format_version to enable more complex logic (eg. additional calls to git)
@frostming
Copy link
Contributor

frostming commented Aug 2, 2024

Can you explain more about "capture context", what do you want to achieve and why the current execution order prevents you from doing this?

@jrosiek
Copy link
Author

jrosiek commented Aug 2, 2024

Sure. I would like to make some additional calls to git from format_version. I need context.root to do that. I could get it as an argument - arguably the easiest solution.

While I was looking for a workaround, I considered creating a hook which would store Context provided in pdm_build_initialize as a class field, which could be later used by format_version (also a method of the same hook). Unfortunately, I learned that it won't work, because SCM version hook runs first and does it's magic (a call to format_version) in its pdm_build_initialize - the first hook method ever called. From this exercise I came up with some ideas how hook could be made more flexible to allow such trickery.

@frostming
Copy link
Contributor

frostming commented Aug 2, 2024

Okay, so could it be solved if the context is passed to format_version, any other problem remaining?

  • format_version in SCM version hook does not have access to Context

I made this order so that users can read the resolved scm version from context when executing their initialize hook.

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 a pull request may close this issue.

2 participants