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.

Sign up for GitHub

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

Jump to bottom

cmake plugin: use build snaps to search paths #2399

Merged
merged 6 commits into from Dec 4, 2018
Merged

cmake plugin: use build snaps to search paths #2399

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe1d664
cmake plugin: use native primitives
sergiusens Nov 8, 2018
0e3de33
cmake plugin: use build snaps to search paths
sergiusens Nov 8, 2018
af1f345
Merge branch 'master' into cmake-find-root-build-snaps
sergiusens Nov 11, 2018
e6fca82
Merge remote-tracking branch 'origin/master' into cmake-find-root-bui…
sergiusens Dec 3, 2018
b5ead7a
different logic
sergiusens Dec 4, 2018
e674645
black
sergiusens Dec 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 57 additions & 6 deletions snapcraft/plugins/cmake.py
View file
Open in desktop
Expand Up @@ -19,8 +19,9 @@
These are projects that have a CMakeLists.txt that drives the build.
The plugin requires a CMakeLists.txt in the root of the source tree.

This plugin also supports options from the `make` plugin. Run
`snapcraft help make` for more details.
If the part has a list of build-snaps listed, the part will be set up in
such a way that the paths to those snaps are used as paths for find_package
and find_library by use of `CMAKE_FIND_ROOT_PATH``.

This plugin uses the common plugin keywords as well as those for "sources".
For more information check the 'plugins' topic for the former and the
Expand All @@ -33,12 +34,17 @@
configure flags to pass to the build using the common cmake semantics.
"""

import logging
import os

import snapcraft.plugins.make
import snapcraft
from snapcraft.internal import errors


class CMakePlugin(snapcraft.plugins.make.MakePlugin):
logger = logging.getLogger(name=__name__)


class CMakePlugin(snapcraft.BasePlugin):
@classmethod
def schema(cls):
schema = super().schema()
Expand All @@ -49,6 +55,14 @@ def schema(cls):
"items": {"type": "string"},
"default": [],
}
# For backwards compatibility
schema["properties"]["make-parameters"] = {
"type": "array",
"minitems": 1,
"uniqueItems": True,
"items": {"type": "string"},
"default": [],
}
schema["required"] = ["source"]

return schema
Expand All @@ -64,6 +78,12 @@ def __init__(self, name, options, project):
self.build_packages.append("cmake")
self.out_of_source_build = True

if project.info.base not in ("core16", "core18"):
raise errors.PluginBaseError(part_name=self.name, base=project.info.base)

if options.make_parameters:
logger.warning("make-paramaters is deprecated, ignoring.")

def build(self):
source_subdir = getattr(self.options, "source_subdir", None)
if source_subdir:
Expand All @@ -73,15 +93,46 @@ def build(self):

env = self._build_environment()

build_snap_paths = [
os.path.join(os.path.sep, "snap", snap_name.split("/")[0], "current")
for snap_name in self.options.build_snaps
]

configflags = []
root_path_appended = False
for configflag in self.options.configflags:
Copy link
Contributor

Choose a reason for hiding this comment

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

@sergiusens At the time we couldn't think of a way to write the conditional in a more straight forward way. I've since put a stop-gap in place in our tooling, where I ended up writing it like this (untested python translation):

configflags = self.options.configflags[:]
for flag in self.options.configflags:
    parts = flag.split('=', 2)
    if parts[0] != "-DCMAKE_FIND_ROOT_PATH":
        continue

    configflags.remove(flag)
    build_snap_paths.append(parts[1])

if build_snap_paths:
	configflags.append("-DCMAKE_FIND_ROOT_PATH={}".format(";".join(build_snap_paths)))

i.e. instead of handling the append as a condition I've turned it it into a cleanup problem. the loop now pulls the original flag out of the list to "clean it up" and push its value into the path list so it gets joined along with all the other values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I chose a different approach, thanks for the nudge 😄

if configflag.startswith("-DCMAKE_FIND_ROOT_PATH="):
configflags.append(
"{};{}".format(configflag, ";".join(build_snap_paths))
)
root_path_appended = True
else:
configflags.append(configflag)
if not root_path_appended and build_snap_paths:
configflags.append(
"-DCMAKE_FIND_ROOT_PATH={}".format(";".join(build_snap_paths))
)

self.run(["cmake", sourcedir, "-DCMAKE_INSTALL_PREFIX="] + configflags, env=env)

# TODO: there is a better way to specify the job count on newer versions of cmake
# https://github.com/Kitware/CMake/commit/1ab3881ec9e809ac5f6cad5cd84048310b8683e2
self.run(
["cmake", sourcedir, "-DCMAKE_INSTALL_PREFIX="] + self.options.configflags,
[
"cmake",
"--build",
".",
"--",
"-j{}".format(self.project.parallel_build_count),
],
env=env,
)

self.make(env=env)
self.run(["cmake", "--build", ".", "--target", "install"], env=env)

def _build_environment(self):
env = os.environ.copy()
env["DESTDIR"] = self.installdir
env["CMAKE_PREFIX_PATH"] = "$CMAKE_PREFIX_PATH:{}".format(
self.project.stage_dir
)
Expand Down
117 changes: 101 additions & 16 deletions tests/unit/plugins/test_cmake.py
View file
Open in desktop
Expand Up @@ -23,23 +23,20 @@
import snapcraft
from snapcraft.internal import errors
from snapcraft.plugins import cmake
from tests import unit
from tests import fixture_setup, unit


class CMakeTestCase(unit.TestCase):
class CMakeBaseTest(unit.TestCase):
def setUp(self):
super().setUp()

class Options:
configflags = []
source_subdir = None

# inherited from MakePlugin
makefile = None
make_parameters = []
make_install_var = "DESTDIR"
disable_parallel = False
artifacts = []
build_snaps = []

self.options = Options()

Expand All @@ -58,16 +55,13 @@ class Options:
self.run_mock = patcher.start()
self.addCleanup(patcher.stop)

patcher = mock.patch("sys.stdout")
patcher.start()
self.addCleanup(patcher.stop)
self.useFixture(fixture_setup.CleanEnvironment())


class CMakeTest(CMakeBaseTest):
def test_get_build_properties(self):
expected_build_properties = ["configflags"]
resulting_build_properties = cmake.CMakePlugin.get_build_properties()
expected_build_properties.extend(
snapcraft.plugins.make.MakePlugin.get_build_properties()
)
self.assertThat(
resulting_build_properties, HasLength(len(expected_build_properties))
)
Expand All @@ -87,9 +81,13 @@ def test_build_referencing_sourcedir_if_no_subdir(self):
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(["make", "-j2"], cwd=plugin.builddir, env=mock.ANY),
mock.call(
["make", "install", "DESTDIR={}".format(plugin.installdir)],
["cmake", "--build", ".", "--", "-j2"],
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(
["cmake", "--build", ".", "--target", "install"],
cwd=plugin.builddir,
env=mock.ANY,
),
Expand All @@ -111,9 +109,13 @@ def test_build_referencing_sourcedir_with_subdir(self):
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(["make", "-j2"], cwd=plugin.builddir, env=mock.ANY),
mock.call(
["make", "install", "DESTDIR={}".format(plugin.installdir)],
["cmake", "--build", ".", "--", "-j2"],
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(
["cmake", "--build", ".", "--target", "install"],
cwd=plugin.builddir,
env=mock.ANY,
),
Expand All @@ -127,6 +129,7 @@ def test_build_environment(self):

expected = {}

expected["DESTDIR"] = plugin.installdir
expected["CMAKE_PREFIX_PATH"] = "$CMAKE_PREFIX_PATH:{}".format(self.stage_dir)
expected["CMAKE_INCLUDE_PATH"] = "$CMAKE_INCLUDE_PATH:" + ":".join(
["{0}/include", "{0}/usr/include", "{0}/include/{1}", "{0}/usr/include/{1}"]
Expand Down Expand Up @@ -174,3 +177,85 @@ def test_unsupported_base(self):

self.assertThat(raised.part_name, Equals("test-part"))
self.assertThat(raised.base, Equals("unsupported-base"))


class CMakeBuildTest(CMakeBaseTest):

scenarios = (
("no snaps", dict(build_snaps=[], expected_root_paths=[])),
(
"one build snap",
dict(
build_snaps=["kde-plasma-sdk"],
expected_root_paths=["/snap/kde-plasma-sdk/current"],
),
),
(
"one build snap, preexisting root find path",
dict(
build_snaps=["kde-plasma-sdk"],
expected_root_paths=["/snap/kde-plasma-sdk/current"],
find_root_flag="-DCMAKE_FIND_ROOT_PATH=/yocto",
),
),
(
"one build snap with channel",
dict(
build_snaps=["kde-plasma-sdk/latest/edge"],
expected_root_paths=["/snap/kde-plasma-sdk/current"],
),
),
(
"two build snap with channel",
dict(
build_snaps=["gnome-sdk", "kde-plasma-sdk/latest/edge"],
expected_root_paths=[
"/snap/gnome-sdk/current",
"/snap/kde-plasma-sdk/current",
],
),
),
)

def setUp(self):
super().setUp()
self.options.build_snaps = self.build_snaps
if hasattr(self, "find_root_flag"):
self.options.configflags = [self.find_root_flag]

if self.expected_root_paths and not hasattr(self, "find_root_flag"):
self.expected_configflags = [
"-DCMAKE_FIND_ROOT_PATH={}".format(";".join(self.expected_root_paths))
]
elif hasattr(self, "find_root_flag"):
self.expected_configflags = [
"{};{}".format(self.find_root_flag, ";".join(self.expected_root_paths))
]
else:
self.expected_configflags = []

def test_build(self):
plugin = cmake.CMakePlugin("test-part", self.options, self.project)
os.makedirs(plugin.builddir)
plugin.build()

self.run_mock.assert_has_calls(
[
mock.call(
["cmake", plugin.sourcedir, "-DCMAKE_INSTALL_PREFIX="]
+ self.expected_configflags,
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(
["cmake", "--build", ".", "--", "-j2"],
cwd=plugin.builddir,
env=mock.ANY,
),
mock.call(
["cmake", "--build", ".", "--target", "install"],
cwd=plugin.builddir,
env=mock.ANY,
),
]
)