Skip to content

Add Anchor hooks#1796

Closed
losman0s wants to merge 2 commits intosolana-foundation:masterfrom
mrgnlabs:post-build-hook
Closed

Add Anchor hooks#1796
losman0s wants to merge 2 commits intosolana-foundation:masterfrom
mrgnlabs:post-build-hook

Conversation

@losman0s
Copy link
Copy Markdown
Contributor

@losman0s losman0s commented Apr 15, 2022

Problem

I have to repeatedly copy pasta the IDL json/types like an idiot while developing my programs.

Proposed solution

Recognize a special "post_build" script from Anchor.toml which triggers automatically after build, so that I can stick to using the simple and beautiful anchor build with no particular option.

I can make a companion PR to the book if this PR seems useful to you.

@paul-schaaf
Copy link
Copy Markdown
Contributor

this is definitely something that would be nice to have and we have a more general issue to add hooks #219

Id like to accept a PR that adds support for all the hooks. Would you be interested in doing the other hooks too? Or just adding the hooks setting and then I add the other hooks? let me know!

the following hooks would be a good start: pre-build, post-build, pre-test, post-test, pre-deploy, post-deploy

@losman0s
Copy link
Copy Markdown
Contributor Author

@paul-schaaf I can do it all, nw. Just want to make sure I get it, you want a separate hooks entry in Anchor.toml, separate from scripts and throwing error for unsupported hooks? Or the current approach with "reserved" script names is fine?

@paul-schaaf
Copy link
Copy Markdown
Contributor

you want a separate hooks entry in Anchor.toml, separate from scripts and throwing error for unsupported hooks?

yes

@paul-schaaf paul-schaaf marked this pull request as draft April 20, 2022 22:22
@losman0s
Copy link
Copy Markdown
Contributor Author

Ok I did not exactly add an error when providing an invalid hook, as it is not immediately apparent to me how to do that cleanly.
Current behaviour is that unsupported entries to the [hooks] map are simply ignored (I'm guessing discarded during toml deserialization), which does not shock me more than that.
Lmk what you think.

@losman0s losman0s marked this pull request as ready for review April 21, 2022 08:39
provider: Provider,
workspace: Option<WorkspaceConfig>,
scripts: Option<ScriptsConfig>,
hooks: Option<HooksConfig>,
Copy link
Copy Markdown
Contributor

@paul-schaaf paul-schaaf Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can make this a Hashmap/BTreeMap and then fail during conversion from _Config to Config (which happens in Config's FromStr implementation) if you encounter an unknown hook

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paul-schaaf I actually started with this and switched to a struct since we know in advance the hooks we're supporting, compared to scripts which is arbitrary. I can switch to Hashmap/BTreeMap if you feel rejecting invalid ones is more valuable though, yes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it's not too ugly implementation-wise and provides a better UX.

and just to be clear what I mean is: Hashmap/BTreeMap in _Config and Option<HooksConfig> in Config

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it. WIll give that a go tmr. Eager to get that merged 😄

Copy link
Copy Markdown
Contributor Author

@losman0s losman0s Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, providing unsupported hooks now results in an error:

thread 'main' panicked at 'Error reading hooks. Hooks ["WRONG"] are not part of valid hooks ["pre_build", "post_build", "pre_test", "post_test", "pre_deploy", "post_deploy"].', cli/src/config.rs:336:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Lmk if you have any other remarks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paul-schaaf bump, in case this fell through the cracks

@losman0s losman0s changed the title Add optional post-build hook Add Anchor hooks May 18, 2022
@tomlinton
Copy link
Copy Markdown
Contributor

Needs an entry in CHANGELOG.md

@losman0s losman0s force-pushed the post-build-hook branch 2 times, most recently from 1b538c2 to 6fc994a Compare May 18, 2022 03:08
@losman0s
Copy link
Copy Markdown
Contributor Author

losman0s commented Jun 7, 2022

@armaniferrante Could you help with this?

@Henry-E
Copy link
Copy Markdown
Collaborator

Henry-E commented Dec 6, 2022

yeh, i feel unqualified to review this. Not sure what hooks are or what they're used for yet 🤷

@Henry-E Henry-E added the help wanted Extra attention is needed label Dec 6, 2022
@Aursen
Copy link
Copy Markdown
Collaborator

Aursen commented Nov 8, 2023

@acheroncrypto can you review this one please?

@jamie-osec
Copy link
Copy Markdown
Collaborator

Due to this becoming stale and conflicts, I'm going to close in favor of #3862

@jamie-osec jamie-osec closed this Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants