Hooks stanza doesn't match parts and apps

[Ted Gould]

Working on adding some font hooks for the Inkscape package it was my first time using hooks, and I found it kinda confusing. I think this largely stemmed from the fact that the hooks section works differently than the parts and apps sections.

The hooks section is abstractly like this: hooks.$trigger.$parameter

Where apps and parts are mostly like this: apps.$name.$parameter

I think that the $name attribute is useful for organization and allows for more flexible hooks. For instance I'd suggest what is now:

hooks:
  configure:
    plugs: [ desktop ]

Would look more like this:

hooks:
  update-font-cache:
    trigger: [ configure ]
    command: update-font-cache.sh
    plugs: [ desktop ]

This would allow for better organization and documentation of hooks.

Of course this would require some translation as snapd expects the original format. But that could be achieved with Snapcraft building a shell script for each trigger calling the appropriate hooks.

[Kyle Fazzari (kyrofa)]

I very much support hooks mirroring apps. In fact, that's what the original proposal was[1]. The "trigger" idea is new though. Two questions come to mind:

1) Can you give me a use-case for where you'd want the exact same thing triggered upon multiple events?
2) If you define multiple hooks that are triggered by the same thing, how should snapcraft determine the order in which to execute them? Does it matter?

[1]: https://github.com/snapcore/snapcraft/pull/921

[Ted Gould (ted)]

> 1) Can you give me a use-case for where you'd want the exact same thing triggered upon multiple events?

Two thoughts there. One would be that you could have a configuration hook that you'd want to run on install for "zero configuration". Basically to init things. The other would be that you have an interface hook that would be on multiple interfaces.
> 2) If you define multiple hooks that are triggered by the same thing, how should snapcraft determine the order in which to execute them? Does it matter?
For the use-cases I was thinking of it wouldn't. But I could imagine that perhaps someone would need that. A solution there could be to include an "after" clause similar to how parts work in snapcraft. That seems to have worked well there. I think you'd want to keep it simple, for complex cases a user could build their own shell script.
On Jun 19 2020, at 12:47 pm, Kyle Fazzari <email address hidden> wrote:
> I very much support hooks mirroring apps. In fact, that's what the
> original proposal was[1]. The "trigger" idea is new though. Two
> questions come to mind:
>
> 1) Can you give me a use-case for where you'd want the exact same thing triggered upon multiple events?
> 2) If you define multiple hooks that are triggered by the same thing, how should snapcraft determine the order in which to execute them? Does it matter?
>
> [1]: https://github.com/snapcore/snapcraft/pull/921
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1877448
>
> Title:
> Hooks stanza doesn't match parts and apps
>
> Status in Snapcraft:
> New
>
> Bug description:
> Working on adding some font hooks for the Inkscape package it was my
> first time using hooks, and I found it kinda confusing. I think this
> largely stemmed from the fact that the hooks section works differently
> than the parts and apps sections.
>
> The hooks section is abstractly like this: hooks.$trigger.$parameter
> Where apps and parts are mostly like this: apps.$name.$parameter
> I think that the $name attribute is useful for organization and allows
> for more flexible hooks. For instance I'd suggest what is now:
>
> hooks:
> configure:
> plugs: [ desktop ]
>
> Would look more like this:
> hooks:
> update-font-cache:
> trigger: [ configure ]
> command: update-font-cache.sh
> plugs: [ desktop ]
>
> This would allow for better organization and documentation of hooks.
> Of course this would require some translation as snapd expects the
> original format. But that could be achieved with Snapcraft building a
> shell script for each trigger calling the appropriate hooks.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/snapcraft/+bug/1877448/+subscriptions
>

[Kyle Fazzari (kyrofa)]

> Two thoughts there. One would be that you could have a configuration hook that you'd want to run on install for "zero configuration". Basically to init things.

I feel that I must mention that the configure hook already runs at install time as well as configure time, but that doesn't really invalidate your point. Particularly because I really wish it didn't do that.

> The other would be that you have an interface hook that would be on multiple interfaces.

Typically those hooks are actually doing things specific to the interface though. That feels pretty hand-wavy.

I want to re-state that I'm on board with the generally idea presented here. However, I wonder if the added complexity of the triggers is worth it given that, if we assume the original proposal for a moment, you could easily have the same executable triggered on multiple events with the following:

hooks:
  configure:
    command: my-executable
  pre-refresh:
    command: my-executable

This also supports the more complex use-case you mention, where if someone needed to run a complex set of steps upon a given event, they could just write an executable for it and specify it there.

[Ted Gould (ted)]

I think the problem with using the trigger as the name is that it makes it so that you can't easily grab or add hooks from other packages. For instance, the font config hook, I might notice it in another package and want to grab it. But if I already had a configure hook, it would be difficult to add. Certainly, I could do it, but I think it adds a significant barrier.

Also, it makes it so that extensions couldn't easily add hooks.
On Jun 19 2020, at 5:00 pm, Kyle Fazzari <email address hidden> wrote:
> > Two thoughts there. One would be that you could have a configuration
> hook that you'd want to run on install for "zero configuration".
> Basically to init things.
>
> I feel that I must mention that the configure hook already runs at
> install time as well as configure time, but that doesn't really
> invalidate your point. Particularly because I really wish it didn't do
> that.
>
> > The other would be that you have an interface hook that would be on
> multiple interfaces.
>
> Typically those hooks are actually doing things specific to the
> interface though. That feels pretty hand-wavy.
>
> I want to re-state that I'm on board with the generally idea presented
> here. However, I wonder if the added complexity of the triggers is worth
> it given that, if we assume the original proposal for a moment, you
> could easily have the same executable triggered on multiple events with
> the following:
>
> hooks:
> configure:
> command: my-executable
> pre-refresh:
> command: my-executable
>
> This also supports the more complex use-case you mention, where if
> someone needed to run a complex set of steps upon a given event, they
> could just write an executable for it and specify it there.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1877448
>
> Title:
> Hooks stanza doesn't match parts and apps
>
> Status in Snapcraft:
> New
>
> Bug description:
> Working on adding some font hooks for the Inkscape package it was my
> first time using hooks, and I found it kinda confusing. I think this
> largely stemmed from the fact that the hooks section works differently
> than the parts and apps sections.
>
> The hooks section is abstractly like this: hooks.$trigger.$parameter
> Where apps and parts are mostly like this: apps.$name.$parameter
> I think that the $name attribute is useful for organization and allows
> for more flexible hooks. For instance I'd suggest what is now:
>
> hooks:
> configure:
> plugs: [ desktop ]
>
> Would look more like this:
> hooks:
> update-font-cache:
> trigger: [ configure ]
> command: update-font-cache.sh
> plugs: [ desktop ]
>
> This would allow for better organization and documentation of hooks.
> Of course this would require some translation as snapd expects the
> original format. But that could be achieved with Snapcraft building a
> shell script for each trigger calling the appropriate hooks.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/snapcraft/+bug/1877448/+subscriptions
>

[Sergio Schvezov (sergiusens)]

Ted Gould <email address hidden> writes:

> I think the problem with using the trigger as the name is that it makes
> it so that you can't easily grab or add hooks from other packages. For
> instance, the font config hook, I might notice it in another package and
> want to grab it. But if I already had a configure hook, it would be
> difficult to add. Certainly, I could do it, but I think it adds a
> significant barrier.

The use case we had was to be able to add this font logic as part of the
extension and still allow you, with little effort, to define your own hook logic.

[Kyle Fazzari (kyrofa)]

> I think the problem with using the trigger as the name is that it makes it so that you can't easily grab or add hooks from other packages.

That's a fair point.

> Also, it makes it so that extensions couldn't easily add hooks.

Ah, also an interesting point I hadn't considered. The proposed triggers do indeed make that nice.