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

lint: lint snap files inside an instance #4115

Merged
merged 3 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 199 additions & 11 deletions snapcraft/commands/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,29 @@

import argparse
import os
import shlex
import subprocess
import tempfile
import textwrap
from contextlib import contextmanager
from pathlib import Path
from shlex import join
from subprocess import CalledProcessError
from typing import Optional
from typing import Iterator, Optional

from craft_cli import BaseCommand, emit
from craft_cli.errors import ArgumentParsingError
from craft_providers.multipass import MultipassProvider
from craft_providers.util import snap_cmd
from overrides import overrides

from snapcraft import providers
from snapcraft.errors import SnapcraftError
from snapcraft.utils import get_managed_environment_home_path, is_managed_mode
from snapcraft import linters, projects, providers
from snapcraft.errors import LegacyFallback, SnapcraftError
from snapcraft.meta import snap_yaml
from snapcraft.parts.lifecycle import apply_yaml, extract_parse_info, process_yaml
from snapcraft.utils import (
get_host_architecture,
get_managed_environment_home_path,
is_managed_mode,
)


class LintCommand(BaseCommand):
Expand Down Expand Up @@ -137,10 +147,17 @@ def _prepare_instance(
:param assert_file: Optional path to assertion file to push into the instance.
:param http_proxy: http proxy to add to environment
:param https_proxy: https proxy to add to environment

:raises SnapcraftError: If `snapcraft lint` fails inside the instance.
"""
emit.progress("Checking build provider availability.")

provider = providers.get_provider()
if isinstance(provider, MultipassProvider):
# blocked by https://github.com/canonical/craft-providers/issues/169
raise SnapcraftError(
"'snapcraft lint' is not supported with Multipass as the build provider"
)
providers.ensure_provider_is_available(provider)

# create base configuration
Expand Down Expand Up @@ -177,18 +194,189 @@ def _prepare_instance(
# run linter inside the instance
command = ["snapcraft", "lint", str(snap_file_instance)]
try:
emit.debug(f"running {shlex.join(command)!r} in instance")
with emit.pause():
instance.execute_run(command, check=True)
except CalledProcessError as err:
except subprocess.CalledProcessError as error:
raise SnapcraftError(
f"failed to execute {join(command)!r} in instance",
) from err
f"failed to execute {shlex.join(command)!r} in instance",
) from error
finally:
providers.capture_logs_from_instance(instance)

# pylint: disable-next=unused-argument
def _run_linter(self, snap_file: Path, assert_file: Optional[Path]) -> None:
"""Run snapcraft linters on a snap file.

:param snap_file: Path to snap file to lint.
:param assert_file: Optional path to assertion file for the snap file.
"""
emit.progress("'snapcraft lint' not implemented.", permanent=True)
# unsquash, load snap.yaml, and optionally load snapcraft.yaml
with self._unsquash_snap(snap_file) as unsquashed_snap:
snap_metadata = snap_yaml.read(unsquashed_snap)
project = self._load_project(unsquashed_snap / "snap" / "snapcraft.yaml")

snap_install_path = self._install_snap(snap_file, assert_file, snap_metadata)

lint_filters = self._load_lint_filters(project)

# run the linters
issues = linters.run_linters(location=snap_install_path, lint=lint_filters)
linters.report(issues, intermediate=True)

@contextmanager
def _unsquash_snap(self, snap_file: Path) -> Iterator[Path]:
"""Unsquash a snap file to a temporary directory.

:param snap_file: Snap package to extract.

:yields: Path to the snap's unsquashed directory.

:raises SnapcraftError: If the snap fails to unsquash.
"""
snap_file = snap_file.resolve()

with tempfile.TemporaryDirectory(prefix=str(snap_file.parent)) as temp_dir:
emit.progress(f"Unsquashing snap file {snap_file.name!r}.")

# unsquashfs [options] filesystem [directories or files to extract] options:
# -force: if file already exists then overwrite
# -dest <pathname>: unsquash to <pathname>
extract_command = [
"unsquashfs",
"-force",
"-dest",
temp_dir,
str(snap_file),
]

try:
subprocess.run(extract_command, capture_output=True, check=True)
except subprocess.CalledProcessError as error:
raise SnapcraftError(
f"could not unsquash snap file {snap_file.name!r}"
) from error

yield Path(temp_dir)

def _load_project(self, snapcraft_yaml_file: Path) -> Optional[projects.Project]:
"""Load a snapcraft Project from a snapcraft.yaml, if present.

The snapcraft.yaml exist for snaps built with the `--enable-manifest` parameter.

:param snapcraft_yaml_file: path to snapcraft.yaml file to load

:returns: A Project containing the snapcraft.yaml's data or None if the yaml
file does not exist.
"""
if not snapcraft_yaml_file.exists():
emit.debug(f"Could not find {snapcraft_yaml_file.name!r}.")
return None

try:
# process_yaml will not parse core, core18, and core20 snaps
yaml_data = process_yaml(snapcraft_yaml_file)
except LegacyFallback as error:
raise SnapcraftError(
"can not lint snap using a base older than core22"
) from error

# process yaml before unmarshalling the data
arch = get_host_architecture()
yaml_data_for_arch = apply_yaml(yaml_data, arch, arch)
# discard parse-info - it is not needed
extract_parse_info(yaml_data_for_arch)
project = projects.Project.unmarshal(yaml_data_for_arch)
return project

def _install_snap(
self,
snap_file: Path,
assert_file: Optional[Path],
snap_metadata: snap_yaml.SnapMetadata,
) -> Path:
"""Install a snap file and optional assertion file.

If the architecture of the snap file does not match the host architecture, then
`snap install` will exit with a descriptive error.

:param snap_file: Snap file to install.
:param assert_file: Optional assertion file to install.
:param snap_metadata: SnapMetadata from the snap file.

:returns: Path to where snap was installed.

:raises SnapcraftError: If the snap cannot be installed.
"""
is_dangerous = not bool(assert_file)

if assert_file:
ack_command = snap_cmd.formulate_ack_command(assert_file)
emit.progress(
f"Installing assertion file with {shlex.join(ack_command)!r}."
)

try:
subprocess.run(ack_command, capture_output=True, check=True)
except subprocess.CalledProcessError:
# if assertion fails, then install the snap dangerously
is_dangerous = True
emit.progress(
f"Could not add assertions from file {assert_file.name!r}",
permanent=True,
)

install_command = snap_cmd.formulate_local_install_command(
classic=bool(snap_metadata.confinement == "classic"),
dangerous=is_dangerous,
snap_path=snap_file,
)
if snap_metadata.grade == "devel":
install_command.append("--devmode")

emit.progress(f"Installing snap with {shlex.join(install_command)!r}.")

try:
subprocess.run(install_command, capture_output=True, check=True)
except subprocess.CalledProcessError as error:
raise SnapcraftError(
f"could not install snap file {snap_file.name!r}"
) from error

return Path("/snap") / snap_metadata.name / "current"

def _load_lint_filters(self, project: Optional[projects.Project]) -> projects.Lint:
"""Load lint filters from a Project and disable the classic linter.

:param project: Project from the snap file, if present.

:returns: Lint config with classic linter disabled.
mr-cal marked this conversation as resolved.
Show resolved Hide resolved
"""
lint_config = projects.Lint(ignore=["classic"])

if project:
if project.lint:
emit.verbose("Collected lint config from 'snapcraft.yaml'.")
lint_config = project.lint

# remove any file-specific classic filters
for item in lint_config.ignore:
if isinstance(item, dict) and "classic" in item.keys():
lint_config.ignore.remove(item)

# disable entire classic linter with the "classic" string
if "classic" not in lint_config.ignore:
lint_config.ignore.append("classic")

else:
emit.verbose("No lint filters defined in 'snapcraft.yaml'.")
else:
emit.verbose(
"Not loading lint filters from 'snapcraft.yaml' because the file "
"does not exist inside the snap file."
)
emit.verbose(
"To include 'snapcraft.yaml' in a snap file, use the parameter "
"'--enable-manifest' when building the snap."
)

return lint_config
3 changes: 1 addition & 2 deletions snapcraft/linters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@
"""Extension processor and related utilities."""

from .base import LinterIssue
from .linters import LinterStatus, lint_command, report, run_linters
from .linters import LinterStatus, report, run_linters

__all__ = [
"LinterIssue",
"LinterStatus",
"lint_command",
"report",
"run_linters",
]
12 changes: 1 addition & 11 deletions snapcraft/linters/linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import os
from functools import partial
from pathlib import Path
from typing import TYPE_CHECKING, Dict, List, Optional, Type
from typing import Dict, List, Optional, Type

from craft_cli import emit

Expand All @@ -33,10 +33,6 @@
from .classic_linter import ClassicLinter
from .library_linter import LibraryLinter

if TYPE_CHECKING:
import argparse


LinterType = Type[Linter]


Expand Down Expand Up @@ -115,12 +111,6 @@ def _update_status(status: LinterStatus, result: LinterResult) -> LinterStatus:
return status


def lint_command(parsed_args: "argparse.Namespace") -> None:
"""``snapcraft lint`` command handler."""
# XXX: obtain lint configuration
run_linters(parsed_args.snap_file, lint=None)


def run_linters(location: Path, *, lint: Optional[projects.Lint]) -> List[LinterIssue]:
"""Run all the defined linters.

Expand Down
34 changes: 34 additions & 0 deletions tests/spread/core22/linters/lint-file-with-assert/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
summary: Run snapcraft lint on a snap file with a valid assert file.

restore: |
rm -f ./*.snap ./*.assert

execute: |
# download a snap and assertion with a core22 base
snap download core22

# give it a predictable name to simplify testing
mv core22_*.snap core22.snap
mv core22_*.assert core22.assert

# test the linter using a build provider
unset SNAPCRAFT_BUILD_ENVIRONMENT

snapcraft lint core22.snap 2> output.txt

# confirm there was not an assertion error
if grep -q "Could not add assertions from file" output.txt; then
exit 1
fi
# confirm linter executed
grep "Running linters..." output.txt

# make a bad assert file
cp core22.snap core22.assert

# snapcraft should handle the bad assert file and still run the linter
snapcraft lint core22.snap 2> output.txt

# confirm there was an assertion error
grep "Could not add assertions from file" output.txt
grep "Running linters..." output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Running linters...
Running linter: library
Lint warnings:
- library: linter-test: missing dependency 'libcaca.so.0'. (https://snapcraft.io/docs/linters-library)
- library: libpng16.so.16: unused library 'usr/lib/x86_64-linux-gnu/libpng16.so.16.37.0'. (https://snapcraft.io/docs/linters-library)
20 changes: 20 additions & 0 deletions tests/spread/core22/linters/lint-file/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: lint-file
base: core22
version: '0.1'
summary: Lint a packaged snapcraft file.
description: spread test

grade: devel
confinement: strict

parts:
my-part:
plugin: nil
source: src
build-packages:
- gcc
- libcaca-dev
stage-packages:
- libpng16-16
override-build:
gcc -o $CRAFT_PART_INSTALL/linter-test test.c -lcaca
7 changes: 7 additions & 0 deletions tests/spread/core22/linters/lint-file/src/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include "caca.h"

int main()
{
caca_create_canvas(80, 24);
return 0;
}
11 changes: 8 additions & 3 deletions tests/spread/core22/linters/lint-file/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@ restore: |
rm -f ./*.snap ./*.assert

execute: |
# build the test snap destructively to save time
snapcraft

# test the linter using a build provider
unset SNAPCRAFT_BUILD_ENVIRONMENT
snapcraft lint lint-file_0.1_*.snap 2> output.txt

snap download hello
# get the lint warnings at end of the log file
sed -n '/Running linters.../,+4 p' < output.txt > linter-output.txt

# `snapcraft lint` is a no-op, but ensure it exits without error
snapcraft lint hello_*.snap
diff -u linter-output.txt expected-linter-output.txt
Loading