-
Notifications
You must be signed in to change notification settings - Fork 445
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
refactor: use init command from craft-application #5129
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
include schema/* | ||
include extensions/* | ||
recursive-include snapcraft/templates * |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,16 +150,6 @@ def _configure_services(self, provider_name: str | None) -> None: | |
|
||
super()._configure_services(provider_name) | ||
|
||
@property | ||
def command_groups(self): | ||
"""Replace craft-application's LifecycleCommand group.""" | ||
_command_groups = super().command_groups | ||
for index, command_group in enumerate(_command_groups): | ||
if command_group.name == "Lifecycle": | ||
_command_groups[index] = cli.CORE24_LIFECYCLE_COMMAND_GROUP | ||
|
||
return _command_groups | ||
Comment on lines
-153
to
-161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have had to do more hacks for |
||
|
||
@override | ||
def _resolve_project_path(self, project_dir: pathlib.Path | None) -> pathlib.Path: | ||
"""Overridden to handle the two possible locations for snapcraft.yaml.""" | ||
|
@@ -467,7 +457,7 @@ def create_app() -> Snapcraft: | |
services=snapcraft_services, | ||
) | ||
|
||
for group in cli.COMMAND_GROUPS: | ||
for group in [cli.CORE24_LIFECYCLE_COMMAND_GROUP, *cli.COMMAND_GROUPS]: | ||
app.add_command_group(group.name, group.commands) | ||
|
||
return app | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,74 +16,44 @@ | |
|
||
"""Snapcraft init command.""" | ||
|
||
from pathlib import Path | ||
from textwrap import dedent | ||
import argparse | ||
|
||
from craft_application.commands import AppCommand | ||
from craft_cli import emit | ||
from overrides import overrides | ||
import craft_application.commands | ||
import craft_cli | ||
from typing_extensions import override | ||
|
||
from snapcraft import errors | ||
from snapcraft.parts.yaml_utils import get_snap_project | ||
|
||
_TEMPLATE_YAML = dedent( | ||
"""\ | ||
name: my-snap-name # you probably want to 'snapcraft register <name>' | ||
base: core24 # the base snap is the execution environment for this snap | ||
version: '0.1' # just for humans, typically '1.2+git' or '1.3.2' | ||
summary: Single-line elevator pitch for your amazing snap # 79 char long summary | ||
description: | | ||
This is my-snap's description. You have a paragraph or two to tell the | ||
most important story about your snap. Keep it under 100 words though, | ||
we live in tweetspace and your description wants to look good in the snap | ||
store. | ||
|
||
grade: devel # must be 'stable' to release into candidate/stable channels | ||
confinement: devmode # use 'strict' once you have the right plugs and slots | ||
class InitCommand(craft_application.commands.InitCommand): | ||
"""Snapcraft init command.""" | ||
|
||
parts: | ||
my-part: | ||
# See 'snapcraft plugins' | ||
plugin: nil | ||
""" | ||
) | ||
@override | ||
def run(self, parsed_args: argparse.Namespace) -> None: | ||
"""Pack a directory or run the lifecycle and pack all artifacts.""" | ||
project_dir = self._get_project_dir(parsed_args) | ||
|
||
if parsed_args.name: | ||
craft_cli.emit.progress( | ||
"Ignoring '--name' parameter because it is not supported yet.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this should be an error, a warning, or if I should change the default template to use this as the snap name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you changed the default template to support the name, would you be able to drop this class wholesale? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. The other extra things that snapcraft adds are:
I think the first is worth keeping. The second could be added as a comment to the snapcraft.yaml file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright. Then I think it should be an error and we should plan to address it soon There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd vote for starting to support the name from the start, and address the rest in the separate item. Out of curiosity, shouldn't you override the |
||
permanent=True, | ||
) | ||
|
||
class InitCommand(AppCommand): | ||
"""Initialize a snapcraft project.""" | ||
|
||
name = "init" | ||
help_msg = "Initialize a snapcraft project." | ||
overview = "Initialize a snapcraft project in the current directory." | ||
|
||
@overrides | ||
def run(self, parsed_args): | ||
"""Initialize a snapcraft project in the current directory. | ||
|
||
:raises SnapcraftError: If a snapcraft.yaml already exists. | ||
""" | ||
emit.progress("Checking for an existing 'snapcraft.yaml'.") | ||
|
||
# if a project is found, then raise an error | ||
try: | ||
project = get_snap_project() | ||
craft_cli.emit.progress("Checking for an existing 'snapcraft.yaml'.") | ||
project = get_snap_project(project_dir) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this one sounds like maybe it should go on Snapcraft's (new) InitService |
||
raise errors.SnapcraftError( | ||
"could not initialize a new snapcraft project because " | ||
"could not initialise a new snapcraft project because " | ||
f"{str(project.project_file)!r} already exists" | ||
) | ||
# the `ProjectMissing` error means a new project can be initialized | ||
# the `ProjectMissing` error means a new project can be initialized | ||
except errors.ProjectMissing: | ||
emit.progress("Could not find an existing 'snapcraft.yaml'.") | ||
|
||
snapcraft_yaml_path = Path("snap/snapcraft.yaml") | ||
|
||
emit.progress(f"Creating {str(snapcraft_yaml_path)!r}.") | ||
craft_cli.emit.progress("Could not find an existing 'snapcraft.yaml'.") | ||
|
||
snapcraft_yaml_path.parent.mkdir(exist_ok=True) | ||
snapcraft_yaml_path.write_text(_TEMPLATE_YAML, encoding="utf-8") | ||
super().run(parsed_args) | ||
|
||
emit.message(f"Created {str(snapcraft_yaml_path)!r}.") | ||
emit.message( | ||
craft_cli.emit.message( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it also feels like maybe this message should be on the initservice; or maybe a way for the service to give a template-based string to be presented to the user after init is done (this would be useful in rockcraft, which has per-profile docs) |
||
"Go to https://docs.snapcraft.io/the-snapcraft-format/8337 for more " | ||
"information about the snapcraft.yaml format." | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
name: my-snap-name # you probably want to 'snapcraft register <name>' | ||
base: core24 # the base snap is the execution environment for this snap | ||
version: '0.1' # just for humans, typically '1.2+git' or '1.3.2' | ||
summary: Single-line elevator pitch for your amazing snap # 79 char long summary | ||
description: | | ||
This is my-snap's description. You have a paragraph or two to tell the | ||
most important story about your snap. Keep it under 100 words though, | ||
we live in tweetspace and your description wants to look good in the snap | ||
store. | ||
|
||
grade: devel # must be 'stable' to release into candidate/stable channels | ||
confinement: devmode # use 'strict' once you have the right plugs and slots | ||
|
||
parts: | ||
my-part: | ||
# See 'snapcraft plugins' | ||
plugin: nil |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,14 +1,40 @@ | ||||||||
summary: Run snapcraft init | ||||||||
|
||||||||
environment: | ||||||||
PROFILE/default_profile: null | ||||||||
PROFILE/simple_profile: simple | ||||||||
PROFILE: null | ||||||||
PROJECT_DIR/default_dir: null | ||||||||
PROJECT_DIR/project_dir: test-project-dir | ||||||||
PROJECT_DIR: null | ||||||||
Comment on lines
+4
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recently realized that in that case it will be one test case per variant, so it won't create a combination of It will create 4 variant's: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. spread variants are so annoying. I also only semi-recently internalized that this: environment:
VAR1/bla: var1-bla
VAR1/ble: var1-ble
VAR2/bla: var2-bla
VAR2/ble: var2-ble can be thought of as: environment:
bla:
VAR1: var1-bla
VAR2: var2-bla
ble:
VAR1: var1-be
VAR2: var2-ble |
||||||||
|
||||||||
restore: | | ||||||||
if [[ -n "$PROJECT_DIR" ]]; then | ||||||||
cd "$PROJECT_DIR" | ||||||||
fi | ||||||||
|
||||||||
Comment on lines
+14
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
rm -f ./*.snap | ||||||||
rm -rf ./snap | ||||||||
|
||||||||
execute: | | ||||||||
# unset SNAPCRAFT_BUILD_ENVIRONMENT=host | ||||||||
unset SNAPCRAFT_BUILD_ENVIRONMENT | ||||||||
|
||||||||
# initialize a new snapcraft project | ||||||||
snapcraft init | ||||||||
args=("init") | ||||||||
|
||||||||
if [[ -n "$PROFILE" ]]; then | ||||||||
args+=("--profile" "$PROFILE") | ||||||||
fi | ||||||||
|
||||||||
if [[ -n "$PROJECT_DIR" ]]; then | ||||||||
args+=("$PROJECT_DIR") | ||||||||
fi | ||||||||
|
||||||||
snapcraft "${args[@]}" | ||||||||
|
||||||||
if [[ -n "$PROJECT_DIR" ]]; then | ||||||||
cd "$PROJECT_DIR" | ||||||||
fi | ||||||||
|
||||||||
# the base should be core24 | ||||||||
grep "^base: core24" snap/snapcraft.yaml | ||||||||
|
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.
reminder to update this