-
Notifications
You must be signed in to change notification settings - Fork 53
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
base: main
Are you sure you want to change the base?
feat(Snapcraft): move parts lifecycle into a reference #163
Conversation
2cfb4e8
to
d8ae70b
Compare
f1b0922
to
9fc34e5
Compare
Streamline documentation and add links to the reference.
There was a problem hiding this 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. |
There was a problem hiding this comment.
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?
Thank you @dilyn-corner and @medubelko for your precious comments! I will take all that into account and create new commits very soon! |
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. Two topics remain on my side:
Is there anything else that you think could be missing from the lifecycle explanation? Thank you |
There was a problem hiding this 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. |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
142419e
to
ca62923
Compare
Hi @dilyn-corner and @medubelko ,
|
Hi @dilyn-corner and @medubelko, 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. |
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.