Comment 17 for bug 1824255

Revision history for this message
Chris Patterson (cjp256) wrote :

OK I think I finally get it now :) Thanks for educating me everyone.

I tested snapcraft 3.8, and it does not appear to regress on this front. So I searched further back to see what I think is the cause may be and stumbled on this commit which didn't appear in a snapcraft-legacy release until "2.44" (Kyle pointed out Xenial in unaffected, at 2.43.1):
https://github.com/snapcore/snapcraft/pull/2257/commits/7cba5de7ce52c93440c3711c8386fee63d65f455

In that commit, the scope of the common.env that is set to snap_env is reduced from all of create_snap_packaging() to write_snap_yaml(). Prior to that, the behavior looks like it probably generated the correct environment for snapcraft-generated trampolines (meta/hooks->snap/hooks). If that's the failure, then I don't think it's ever actually worked in 3.x.

I talked to Sergio earlier and he pressed for the use of command-chain, which seems like a good idea. In this case, we'll just be inserting snapcraft-runner into the command-chain. The question is for what scope:

(1) hooks using snapcraft-generated trampolines (closest to old behavior?)
(2) all hooks (new functionality?)

@kyrofa suggested that he would like to see it done for all hooks, and had a good idea above about gating compatibility changes on core20. I think if we choose to apply command-chain to all hooks (#2), the safest choice will be to do it for core20 onwards.

In the meantime, we can possibly automatically re-enable this for #1 unless that boat has sailed? Publishers of strict core/core18 snaps that bypass the trampoline (using pre-installed <project>/snap/hooks/...) can safely opt-in to this behavior by adding their own command-chain ["snap/command-chain/snapcraft-runner"], I can PR a change to ensure it's available if found in user-supplied command-chain.

In summary, the proposal is:

(1) include environment in trampolines generated by snapcraft:

  (a) master: insert snapcraft-runner into command-chain only for these hooks & ensure snapcraft-runner is generated if user-configured.

  (b) legacy: use Kyle's PR (2893)

(2) add new PR for core20 to include snapcraft-runer in command-chain unconditionally for all hooks

Thoughts?