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

feat(Snapcraft): move parts lifecycle into a reference #163

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Sophie-Pages
Copy link

First step into moving the Parts lifecycle into Snapcraft's reference: create the reference.
It is missing the additional work and redirect links on the other Snapcraft pages.

@medubelko medubelko self-requested a review January 20, 2025 18:57
@Sophie-Pages Sophie-Pages force-pushed the feat/snapcraft-parts-lifecycle branch 3 times, most recently from f1b0922 to 9fc34e5 Compare January 26, 2025 18:09
Streamline documentation and add links to the reference.
Copy link

@dilyn-corner dilyn-corner left a comment

Choose a reason for hiding this comment

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

I really like this -- good work! Left some comments, mostly high-level thoughts (those ending with '?'). Others are, I think, more necessary changes.


Snaps are created using a build recipe defined in a file called `snapcraft.yaml`.

When the snapcraft tool is executed on the command line, it will look for the file in the current project work directory, either in the top-level folder or a `snap` subdirectory. If the file is found, snapcraft will then parse its contents and progress the build toward completion.
Copy link

@dilyn-corner dilyn-corner Jan 27, 2025

Choose a reason for hiding this comment

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

@medubelko does snapcraft (plan to continue to) support build-aux/snap? Is it a recommended path?

@Sophie-Pages
Copy link
Author

Thank you @dilyn-corner and @medubelko for your precious comments! I will take all that into account and create new commits very soon!

@Sophie-Pages
Copy link
Author

Sophie-Pages commented Feb 4, 2025

Hi @dilyn-corner and @medubelko,

I reviewed your comments except for the support build-aux/snap comment.

After talking with Michael we decided that I could try to improve the lifecycle explanation even if it's out of the scope of the initial issue.
I clarified, reorganised, and added missing information related to the lifecycle.

Two topics remain on my side:

  • I added information about permissions in the prime step. I think the users would benefit from having a link to more info on this topic but I don't think the lifecycle explanation is the place to do that.
    Can I link the readthedocs page and maybe in another issue a snapcraft.io page on permissions could be created?
  • I would like to add a similar processing diagram. Because it will need to be redone from sratch, I wanted your approval before working on it.

Is there anything else that you think could be missing from the lifecycle explanation?

Thank you

Copy link

@dilyn-corner dilyn-corner left a comment

Choose a reason for hiding this comment

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

I added information about permissions in the prime step. I think the users would benefit from having a link to more info on this topic but I don't think the lifecycle explanation is the place to do that.

Agreed; I also haven't ever really seen documentation surrounding this, which gives me the impression that it's more of a general "craft" thing than a snap thing (is it for rocks? charms?).

Can I link the readthedocs page and maybe in another issue a snapcraft.io page on permissions could be created?

Ultimately I think it may fall to @mr-cal's judgement; I've never seen a snap use this syntax.

I would like to add a similar processing diagram. Because it will need to be redone from sratch, I wanted your approval before working on it.

I think this would be a very powerful explanatory tool, but I also don't think it should block merging these excellent changes.

Is there anything else that you think could be missing from the lifecycle explanation?

It may be useful to add a link to organize's definition or use similarly to how it's done for the stage keyword; I think at the very least a link to here (and that page having anchors would be super useful...).

* This is the first time all the different parts that make up the snap are actually placed in the same directory. If multiple parts provide the same file with differing contents, you will get a conflict. You can avoid these conflicts by using the [`stage` keyword](/t/snapcraft-parts-metadata/8336#heading--stage) to enable or block files coming from the part. You can also use this keyword to filter out files that are not required in the snap itself, for example build files specific to a single part.
1. **Prime**: copies the staged components into the priming area.
* This is very similar to the stage step, but files go into the priming area instead of the staging area. The `prime` step exists because the staging area might still contain files that are required for the build but not for the snap. For example, if you have a part that downloads and installs a compiler, then you stage this part so other parts can use the compiler during building. You can then use the `prime` filter keyword to make sure that it doesn't get copied to the priming area, so it's not taking up space in the snap.
* During this step, if specific `permissions` were specified, they will be applied.
Copy link

@dilyn-corner dilyn-corner Feb 4, 2025

Choose a reason for hiding this comment

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

What exactly is meant by 'permissions' here? I'm not familiar with this aspect of the prime step.
(This is really just reiterating your comment about these changes; I'm musing).

Copy link
Author

@Sophie-Pages Sophie-Pages Feb 5, 2025

Choose a reason for hiding this comment

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

I suppose it is indeed more of a "craft" thing but because there is an existing Snapcraft documentation I thought it would be useful to mention it for the users.
It works just fine for snaps (I tested it) and allows to change the permissions of specific files during the prime step.

Choose a reason for hiding this comment

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

Fascinating! I suspect it is fine to leave in. TIL!


Each plugin defines the default actions that happen during a step. This behavior can be changed in two ways:
- By using `override-<step-name>` in `snapcraft.yaml`. See [Overriding steps](/t/scriptlets/4892) for more details.
- By using a local plugin. This can inherit the parent plugin or scaffolding from the original. See [Local plugins](/t/writing-local-plugins/5125) for more details. Please note that using a local plugin is deprecated for base core22 and onwards. Because it hasn't been supported for recent bases, it will need a core20 to work.

Choose a reason for hiding this comment

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

Please note that using a local plugin is deprecated for base core22 and onwards. Because it hasn't been supported for recent bases, it will need a core20 to work.

This phrasing feels a little cludgy; local plugins aren't just deprecated, they're wholly unsupported (albeit, there's technically some tricks you can pull, but we don't mention that...). "it will need a core20" should probably read "the snap must use base: core20 or earlier".

Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
- By using a local plugin. This can inherit the parent plugin or scaffolding from the original. See [Local plugins](/t/writing-local-plugins/5125) for more details. Please note that using a local plugin is deprecated for base core22 and onwards. Because it hasn't been supported for recent bases, it will need a core20 to work.
- By using a local plugin. This can inherit the parent plugin or scaffolding from the original. See [Local plugins](/t/writing-local-plugins/5125) for more details. Please note that using a local plugin has been deprecated in `base: core20` and fully removed in `base: core22`.

I chose to frame it this way so the users are not led to think they should use core20 while 20.04 will reach end of free support in a few months. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

For the organize link, do you want me to pull the reference in CODA and add an anchor? Maybe each key from this page would benefit of having an anchor but that would be a different issue I suppose? I can as well use this link https://snapcraft.io/docs/snapcraft-yaml-schema#p-21225-organize-79, even if it is not ideal, that would be a middle ground before the issue on anchors is done? Either way, I can do what you think is best.

Choose a reason for hiding this comment

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

The new language is great!

I agree that each key should have its own anchor (why do to the sidebar hyperlinks not include links to the specific parts directives, for instance?), and I agree that doing this would be more apt as a separate issue. But I think at that point we are fast approaching a design consideration that I don't think I'm qualified for; @degville @medubelko?

@Sophie-Pages Sophie-Pages force-pushed the feat/snapcraft-parts-lifecycle branch from 142419e to ca62923 Compare February 5, 2025 23:06
@Sophie-Pages
Copy link
Author

Hi @dilyn-corner and @medubelko ,

@Sophie-Pages
Copy link
Author

Sophie-Pages commented Feb 19, 2025

Hi @dilyn-corner and @medubelko,
I added the diagram source file for maintainability.

I believe the diagram could benefit from a written text to enhance accessibility. This could be done in another issue to ensure it doesn't block the PR.

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 this pull request may close these issues.

4 participants