Skip to content

Commit

Permalink
add SingleFileExecutable to make it easier to consume Snapshots of ex…
Browse files Browse the repository at this point in the history
…ecutables (pantsbuild#8860)

### Problem

We use the same boilerplate in `cloc.py` and `download_pex_bin.py` to wrap a `Snapshot` consumed by a `UrlToFetch`, e.g.: https://github.com/pantsbuild/pants/blob/43ffe73ada2fcbc2571172193c30b906d2de944f/src/python/pants/backend/python/rules/download_pex_bin.py#L64-L65

### Solution

- Create `SingleFileExecutable` dataclass in `engine/fs.py`, which validates that the input has only one file, and has a convenience method `.exe_filename` to return a relative path to the wrapped executable which can be used in a hermetic process execution.
- Use `SingleFileExecutable` for both `cloc` and `pex`.

### Result

Along with pantsbuild#8859, this helps to simplify the experience of fetching an executable via `UrlToFetch`!
  • Loading branch information
cosmicexplorer authored Dec 25, 2019
1 parent e500eb8 commit 92ebffa
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 8 deletions.
15 changes: 11 additions & 4 deletions src/python/pants/backend/python/rules/download_pex_bin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,23 @@
from pants.backend.python.subsystems.python_native_code import PexBuildEnvironment
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
from pants.engine.fs import Digest, Snapshot, UrlToFetch
from pants.engine.fs import Digest, SingleFileExecutable, Snapshot, UrlToFetch
from pants.engine.isolated_process import ExecuteProcessRequest
from pants.engine.rules import rule
from pants.engine.selectors import Get


@dataclass(frozen=True)
class DownloadedPexBin(HermeticPex):
executable: str
directory_digest: Digest
exe: SingleFileExecutable

@property
def executable(self) -> str:
return self.exe.exe_filename

@property
def directory_digest(self) -> Digest:
return self.exe.directory_digest

def create_execute_request( # type: ignore[override]
self,
Expand Down Expand Up @@ -63,7 +70,7 @@ async def download_pex_bin() -> DownloadedPexBin:
url = 'https://github.com/pantsbuild/pex/releases/download/v1.6.12/pex'
digest = Digest('ce64cb72cd23d2123dd48126af54ccf2b718d9ecb98c2ed3045ed1802e89e7e1', 1842359)
snapshot = await Get[Snapshot](UrlToFetch(url, digest))
return DownloadedPexBin(executable=snapshot.files[0], directory_digest=snapshot.directory_digest)
return DownloadedPexBin(SingleFileExecutable(snapshot))


def rules():
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ python_library(
':selectors',
'src/python/pants/base:project_tree',
'src/python/pants/option',
'src/python/pants/util:dirutil',
'src/python/pants/util:meta',
],
tags = {'partially_type_checked'},
Expand Down
34 changes: 33 additions & 1 deletion src/python/pants/engine/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
from pants.engine.rules import RootRule
from pants.option.custom_types import GlobExpansionConjunction as GlobExpansionConjunction
from pants.option.global_options import GlobMatchErrorBehavior
from pants.util.dirutil import maybe_read_file, safe_delete, safe_file_dump
from pants.util.dirutil import (
ensure_relative_file_name,
maybe_read_file,
safe_delete,
safe_file_dump,
)
from pants.util.meta import frozen_after_init


Expand Down Expand Up @@ -239,6 +244,33 @@ def materialize_directories(
)


@frozen_after_init
@dataclass(unsafe_hash=True)
class SingleFileExecutable:
"""Wraps a `Snapshot` and ensures that it only contains a single file."""
_exe_filename: Path
directory_digest: Digest

@property
def exe_filename(self) -> str:
return ensure_relative_file_name(self._exe_filename)

class ValidationError(ValueError): pass

@classmethod
def _raise_validation_error(cls, snapshot: Snapshot, should_message: str) -> None:
raise cls.ValidationError(f'snapshot {snapshot} used for {cls} should {should_message}')

def __init__(self, snapshot: Snapshot) -> None:
if len(snapshot.files) != 1:
self._raise_validation_error(snapshot, 'have exactly 1 file!')
if snapshot.directory_digest == EMPTY_DIRECTORY_DIGEST:
self._raise_validation_error(snapshot, 'have a non-empty digest!')

self._exe_filename = Path(snapshot.files[0])
self.directory_digest = snapshot.directory_digest


def create_fs_rules():
"""Creates rules that consume the intrinsic filesystem types."""
return [
Expand Down
42 changes: 42 additions & 0 deletions src/python/pants/fs/fs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@

from pants.engine.console import Console
from pants.engine.fs import (
EMPTY_DIRECTORY_DIGEST,
Digest,
DirectoryToMaterialize,
FileContent,
InputFilesContent,
MaterializeDirectoriesResult,
MaterializeDirectoryResult,
SingleFileExecutable,
Snapshot,
Workspace,
)
from pants.engine.goal import Goal, GoalSubsystem
Expand Down Expand Up @@ -106,3 +109,42 @@ def test_is_child_of(self):

assert not is_child_of(Path("/other/random/directory/root/dist/dir"), mock_build_root)
assert not is_child_of(Path("../not_root/dist/dir"), mock_build_root)


class SingleFileExecutableTest(TestBase):

def test_raises_with_multiple_files(self):
input_files_content = InputFilesContent((
FileContent(path='a.txt', content=b'test file contents'),
FileContent(path='b.txt', content=b'more test file contents'),
))

snapshot = self.request_single_product(Snapshot, input_files_content)

with self.assertRaisesWithMessage(
SingleFileExecutable.ValidationError,
f'snapshot {snapshot} used for {SingleFileExecutable} should have exactly 1 file!'):
SingleFileExecutable(snapshot)

def test_raises_empty_digest(self):
snapshot = Snapshot(EMPTY_DIRECTORY_DIGEST, files=('a.txt',), dirs=())

with self.assertRaisesWithMessage(
SingleFileExecutable.ValidationError,
f'snapshot {snapshot} used for {SingleFileExecutable} should have a non-empty digest!'):
SingleFileExecutable(snapshot)

def test_accepts_single_file_snapshot(self):
input_files_content = InputFilesContent((
FileContent(path='subdir/a.txt', content=b'test file contents'),
))
snapshot = self.request_single_product(Snapshot, input_files_content)

assert SingleFileExecutable(snapshot).exe_filename == './subdir/a.txt'

input_files_content = InputFilesContent((
FileContent(path='some_silly_file_name', content=b'test file contents'),
))
snapshot = self.request_single_product(Snapshot, input_files_content)

assert SingleFileExecutable(snapshot).exe_filename == './some_silly_file_name'
14 changes: 11 additions & 3 deletions src/python/pants/rules/core/cloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
FileContent,
FilesContent,
InputFilesContent,
SingleFileExecutable,
Snapshot,
UrlToFetch,
)
Expand All @@ -25,8 +26,15 @@
@dataclass(frozen=True)
class DownloadedClocScript:
"""Cloc script as downloaded from the pantsbuild binaries repo."""
script_path: str
digest: Digest
exe: SingleFileExecutable

@property
def script_path(self) -> str:
return self.exe.exe_filename

@property
def digest(self) -> Digest:
return self.exe.directory_digest


#TODO(#7790) - We can't call this feature-complete with the v1 version of cloc
Expand All @@ -37,7 +45,7 @@ async def download_cloc_script() -> DownloadedClocScript:
sha_256 = "2b23012b1c3c53bd6b9dd43cd6aa75715eed4feb2cb6db56ac3fbbd2dffeac9d"
digest = Digest(sha_256, 546279)
snapshot = await Get[Snapshot](UrlToFetch(url, digest))
return DownloadedClocScript(script_path=snapshot.files[0], digest=snapshot.directory_digest)
return DownloadedClocScript(SingleFileExecutable(snapshot))


class CountLinesOfCodeOptions(GoalSubsystem):
Expand Down
12 changes: 12 additions & 0 deletions src/python/pants/util/dirutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import uuid
from collections import defaultdict
from contextlib import contextmanager
from pathlib import Path
from typing import (
Any,
Callable,
Expand Down Expand Up @@ -74,6 +75,17 @@ def fast_relpath_optional(path: str, start: str) -> Optional[str]:
return None


def ensure_relative_file_name(path: Path) -> str:
"""Return a string representing the `path`, with a leading './'.
This ensures that the returned string can be used as the executable file when executing a
subprocess, without putting the executable file on the PATH.
"""
if path.is_absolute():
raise ValueError(f'path {path} is expected to be relative!')
return f'./{path}'


def safe_mkdir(directory: str, clean: bool = False) -> None:
"""Ensure a directory is present.
Expand Down
16 changes: 16 additions & 0 deletions tests/python/pants_test/util/test_dirutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import unittest.mock
from contextlib import contextmanager
from dataclasses import dataclass
from pathlib import Path
from typing import Iterator, Tuple, Union

from pants.util import dirutil
Expand All @@ -18,6 +19,7 @@
_mkdtemp_unregister_cleaner,
absolute_symlink,
check_no_overlapping_paths,
ensure_relative_file_name,
fast_relpath,
get_basedir,
longest_dir_prefix,
Expand Down Expand Up @@ -92,6 +94,20 @@ def test_fast_relpath_invalid(self) -> None:
with self.assertRaises(ValueError):
fast_relpath('/a/baseball', '/a/b')

def test_ensure_relative_file_name(self) -> None:
path = Path('./subdir/a.txt')
assert str(path) == 'subdir/a.txt'
assert ensure_relative_file_name(path) == './subdir/a.txt'

path = Path('./a.txt')
assert ensure_relative_file_name(path) == './a.txt'

path = Path('some_exe')
assert ensure_relative_file_name(path) == './some_exe'

path = Path('./leading_slash')
assert ensure_relative_file_name(path) == './leading_slash'

@strict_patch('atexit.register')
@strict_patch('os.getpid')
@strict_patch('pants.util.dirutil.safe_rmtree')
Expand Down

0 comments on commit 92ebffa

Please sign in to comment.