packaging: use local patchelf

.travis.yml

@@ -113,8 +113,8 @@ jobs:
         - sudo lxd init --auto
       script:
         - sudo snapcraft snap --use-lxd
-        - cp *.snap "snapcraft-pr$TRAVIS_PULL_REQUEST.snap"
-        - cp "snapcraft-pr$TRAVIS_PULL_REQUEST.snap" "$TRAVIS_BUILD_DIR/snaps-cache/snapcraft-pr$TRAVIS_PULL_REQUEST.snap"
+        - sudo cp *.snap "snapcraft-pr$TRAVIS_PULL_REQUEST.snap"
+        - sudo cp "snapcraft-pr$TRAVIS_PULL_REQUEST.snap" "$TRAVIS_BUILD_DIR/snaps-cache/snapcraft-pr$TRAVIS_PULL_REQUEST.snap"
       after_success:
         - timeout 180 /snap/bin/transfer snapcraft-pr$TRAVIS_PULL_REQUEST.snap
       after_failure:

snap/snapcraft.yaml

@@ -35,6 +35,18 @@ parts:
     stage:
       - snapcraft-completion
 
+  patchelf:
+    plugin: autotools
+    source: https://github.com/snapcore/patchelf
+    source-type: git
+    source-tag: '0.10'
+    build-packages:
+      - g++
+      - make
+    override-build: |
+      snapcraftctl build
+      make check
+
   snapcraft-libs:
     plugin: nil
     stage-packages:
@@ -48,7 +60,6 @@ parts:
         - libsodium18
         - libxml2
         - libxslt1.1
-        - patchelf
         - squashfs-tools
         - xdelta3
     override-build: |

snapcraft/internal/elf.py

@@ -25,6 +25,7 @@
 
 import elftools.elf.elffile
 from pkg_resources import parse_version
+from elftools.common.exceptions import ELFError
 
 from snapcraft import file_utils
 from snapcraft.internal import common, errors, repo
@@ -376,6 +377,16 @@ def load_dependencies(
                 library_paths.add(l.path)
         return library_paths
 
+    def is_dynamic_executable(self) -> bool:
+        # This way of answering the question is perhaps a bit OTT. But it works.
+        try:
+            ef = elftools.elf.elffile.ELFFile(open(self.path, "rb"))
+        except ELFError as elf_error:
+            logger.debug("ELFFile({}) failed: {}".format(self.path, elf_error))
+            return False
+        else:
+            return "PT_DYNAMIC" in [s.header.p_type for s in ef.iter_segments()]
+
 
 class Patcher:
     """Patcher holds the necessary logic to patch elf files."""
@@ -438,41 +449,6 @@ def patch(self, *, elf_file: ElfFile) -> None:
         self._run_patchelf(patchelf_args=patchelf_args, elf_file_path=elf_file.path)
 
     def _run_patchelf(self, *, patchelf_args: List[str], elf_file_path: str) -> None:
-        try:
-            return self._do_run_patchelf(
-                patchelf_args=patchelf_args, elf_file_path=elf_file_path
-            )
-        except errors.PatcherError as patch_error:
-            # This is needed for patchelf to properly work with
-            # go binaries (LP: #1736861).
-            # We do this here instead of the go plugin for two reasons, the
-            # first being that we do not want to blindly remove the section,
-            # only doing it when necessary, and the second, this logic
-            # should eventually be removed once patchelf catches up.
-            try:
-                logger.warning(
-                    "Failed to update {!r}. Retrying after stripping "
-                    "the .note.go.buildid from the elf file.".format(elf_file_path)
-                )
-                subprocess.check_call(
-                    [
-                        self._strip_cmd,
-                        "--remove-section",
-                        ".note.go.buildid",
-                        elf_file_path,
-                    ]
-                )
-            except subprocess.CalledProcessError:
-                logger.warning(
-                    "Could not properly strip .note.go.buildid "
-                    "from {!r}.".format(elf_file_path)
-                )
-                raise patch_error
-            return self._do_run_patchelf(
-                patchelf_args=patchelf_args, elf_file_path=elf_file_path
-            )
-
-    def _do_run_patchelf(self, *, patchelf_args: List[str], elf_file_path: str) -> None:
         # Run patchelf on a copy of the primed file and replace it
         # after it is successful. This allows us to break the potential
         # hard link created when migrating the file across the steps of
@@ -487,24 +463,9 @@ def _do_run_patchelf(self, *, patchelf_args: List[str], elf_file_path: str) -> N
             # bundled with snapcraft which means its lack of existence is a
             # "packager" error.
             except subprocess.CalledProcessError as call_error:
-                patchelf_version = (
-                    subprocess.check_output([self._patchelf_cmd, "--version"])
-                    .decode()
-                    .strip()
+                raise errors.PatcherGenericError(
+                    elf_file=elf_file_path, process_exception=call_error
                 )
-                # 0.10 is the version where patching certain binaries will
-                # work (currently known affected packages are mostly built
-                # with go).
-                if parse_version(patchelf_version) < parse_version("0.10"):
-                    raise errors.PatcherNewerPatchelfError(
-                        elf_file=elf_file_path,
-                        process_exception=call_error,
-                        patchelf_version=patchelf_version,
-                    )
-                else:
-                    raise errors.PatcherGenericError(
-                        elf_file=elf_file_path, process_exception=call_error
-                    )
 
             # We unlink to break the potential hard link
             os.unlink(elf_file_path)

snapcraft/plugins/go.py

@@ -54,7 +54,7 @@
 
 import snapcraft
 from snapcraft import common
-from snapcraft.internal import errors
+from snapcraft.internal import elf, errors
 
 
 logger = logging.getLogger(__name__)
@@ -103,6 +103,7 @@ def __init__(self, name, options, project):
         super().__init__(name, options, project)
 
         self._setup_base_tools(options.go_channel, project.info.base)
+        self._is_classic = project.info.confinement == "classic"
 
         self._gopath = os.path.join(self.partdir, "go")
         self._gopath_src = os.path.join(self._gopath, "src")
@@ -177,6 +178,16 @@ def build(self):
         for package in packages:
             binary = os.path.join(self._gopath_bin, self._binary_name(package))
             self._run(["go", "build", "-o", binary] + tags + [package])
+            # Relink with system linker if executable is dynamic in order to be
+            # able to set rpath later on. This workaround can be removed after
+            # https://github.com/NixOS/patchelf/issues/146 is fixed.
+            if self._is_classic:
+                if elf.ElfFile(path=binary).is_dynamic_executable():
+                    self._run(
+                        ["go", "build", "-ldflags", "-linkmode=external", "-o", binary]
+                        + tags
+                        + [package]
+                    )
 
         install_bin_path = os.path.join(self.installdir, "bin")
         os.makedirs(install_bin_path, exist_ok=True)

tests/integration/__init__.py

@@ -55,7 +55,7 @@ def setUp(self):
         package_type = os.getenv("SNAPCRAFT_PACKAGE_TYPE")
         if package_type == "snap":
             self.snapcraft_command = "/snap/bin/snapcraft"
-            self.patchelf_command = "/snap/snapcraft/current/usr/bin/patchelf"
+            self.patchelf_command = "/snap/snapcraft/current/bin/patchelf"
             self.execstack_command = "/snap/snapcraft/current/usr/sbin/execstack"
         elif package_type == "deb":
             self.snapcraft_command = "/usr/bin/snapcraft"

tests/spread/plugins/go/rpath-rewrite-cgo/task.yaml

@@ -0,0 +1,36 @@
+summary: Build and run Go snaps with dynamic executables
+
+environment:
+  SNAP_DIR: ../snaps/go-cgo
+
+prepare: |
+  #shellcheck source=tests/spread/tools/snapcraft-yaml.sh
+  . "$TOOLS_DIR/snapcraft-yaml.sh"
+  set_base "$SNAP_DIR/snap/snapcraft.yaml"
+
+  # Set the confinement type of this snap to classic
+  sed -i 's/\(confinement.*\)strict/\1classic/;/^confinement:/a base: core18' "$SNAP_DIR/snap/snapcraft.yaml"
+
+restore: |
+  #shellcheck source=tests/spread/tools/snapcraft-yaml.sh
+  . "$TOOLS_DIR/snapcraft-yaml.sh"
+  cd "$SNAP_DIR"
+  snapcraft clean
+  rm -f ./*.snap
+  restore_yaml "snap/snapcraft.yaml"
+
+execute: |
+  cd "$SNAP_DIR"
+
+  snapcraft prime
+
+  if ! snap list go; then
+    echo "The go snap from the Snap Store should have been installed."
+    exit 1
+  fi
+
+  # Ensure binaries are properly patched
+  /snap/snapcraft/current/bin/patchelf --print-rpath prime/bin/go-cgo | MATCH "/snap/core[0-9]*/current"
+
+  # Run generated binary
+  prime/bin/go-cgo | MATCH "more or less"

tests/unit/plugins/test_go.py

@@ -238,6 +238,14 @@ def test_get_build_properties(self):
             self.assertIn(property, resulting_build_properties)
 
 
+class MockElfFile:
+    def __init__(self, *, path: str) -> None:
+        self.path = path
+
+    def is_dynamic_executable(self) -> bool:
+        return True
+
+
 class GoPluginTest(GoPluginBaseTest):
     def setUp(self):
 
@@ -609,6 +617,61 @@ class Options:
             env=mock.ANY,
         )
 
+    @mock.patch("snapcraft.internal.elf.ElfFile")
+    def test_build_classic_dynamic_relink(self, mock_elffile):
+        class Options:
+            source = ""
+            go_channel = "latest/stable"
+            go_packages = ["github.com/gotools/vet"]
+            go_importpath = ""
+            go_buildtags = ""
+
+        mock_elffile.return_value = MockElfFile(path="foo")
+        self.project.info.confinement = "classic"
+        plugin = go.GoPlugin("test-part", Options(), self.project)
+
+        os.makedirs(plugin.sourcedir)
+
+        plugin.pull()
+
+        os.makedirs(plugin._gopath_bin)
+        os.makedirs(plugin.builddir)
+        # fake some binaries
+        binary = os.path.join(plugin._gopath_bin, "vet")
+        open(binary, "w").close()
+
+        self.run_mock.reset_mock()
+        plugin.build()
+
+        self.run_mock.assert_has_calls(
+            [
+                mock.call(
+                    ["go", "build", "-o", binary, plugin.options.go_packages[0]],
+                    cwd=plugin._gopath_src,
+                    env=mock.ANY,
+                ),
+                mock.call(
+                    [
+                        "go",
+                        "build",
+                        "-ldflags",
+                        "-linkmode=external",
+                        "-o",
+                        binary,
+                        plugin.options.go_packages[0],
+                    ],
+                    cwd=plugin._gopath_src,
+                    env=mock.ANY,
+                ),
+            ]
+        )
+
+        self.assertTrue(os.path.exists(plugin._gopath))
+        self.assertTrue(os.path.exists(plugin._gopath_src))
+        self.assertTrue(os.path.exists(plugin._gopath_bin))
+        vet_binary = os.path.join(plugin.installdir, "bin", "vet")
+        self.assertTrue(os.path.exists(vet_binary))
+
 
 class GoPluginSchemaValidationTest(unit.TestCase):
     def test_sources_validation_neither(self):

tests/unit/test_elf.py

@@ -16,7 +16,6 @@
 import fixtures
 import logging
 import os
-import subprocess
 import tempfile
 import sys
 
@@ -334,39 +333,6 @@ def test_patch_fails_raises_patcherror_exception(self):
             errors.PatcherGenericError, elf_patcher.patch, elf_file=elf_file
         )
 
-    def test_patch_fails_with_old_version(self):
-        self.fake_elf = fixture_setup.FakeElf(
-            root_path=self.path, patchelf_version="0.8"
-        )
-        self.useFixture(self.fake_elf)
-
-        elf_file = self.fake_elf["fake_elf-bad-patchelf"]
-        # The base_path does not matter here as there are not files to
-        # be crawled for.
-        elf_patcher = elf.Patcher(dynamic_linker="/lib/fake-ld", root_path="/fake")
-
-        with mock.patch(
-            "subprocess.check_call", wraps=subprocess.check_call
-        ) as mock_check_call:
-            self.assertRaises(
-                errors.PatcherNewerPatchelfError, elf_patcher.patch, elf_file=elf_file
-            )
-
-            # Test that .note.go.buildid is stripped off
-            mock_check_call.assert_has_calls(
-                [
-                    mock.call(
-                        ["patchelf", "--set-interpreter", "/lib/fake-ld", mock.ANY]
-                    ),
-                    mock.call(
-                        ["strip", "--remove-section", ".note.go.buildid", mock.ANY]
-                    ),
-                    mock.call(
-                        ["patchelf", "--set-interpreter", "/lib/fake-ld", mock.ANY]
-                    ),
-                ]
-            )
-
 
 class TestSonameCache(unit.TestCase):
     def setUp(self):