Improve program file handling in the run module

problemtools/checks/includes.py

@@ -25,6 +25,7 @@ def check_includes(includes: Includes, language_config: Languages, format_versio
     _check_ambiguous_mainfile(includes, language_config, diag)
     _check_default_sets_mainfile(includes, language_config, diag)
     _check_default_path_collision(includes, diag)
+    _check_default_file_directory_collision(includes, diag)
 
 
 def _check_default_and_unknown_languages(
@@ -40,7 +41,7 @@ def _check_default_and_unknown_languages(
 
 def _default_include_paths(includes: Includes) -> list[Path]:
     default_includes = includes.languages.get(DEFAULT_LANGUAGE)
-    return [f.path for f in default_includes.files] if default_includes else []
+    return [f.path for f in default_includes.files.files] if default_includes else []
 
 
 def _check_default_path_collision(includes: Includes, diag: Diagnostics) -> None:
@@ -49,12 +50,35 @@ def _check_default_path_collision(includes: Includes, diag: Diagnostics) -> None
         for lang_id, lang_includes in includes.languages.items():
             if lang_id == DEFAULT_LANGUAGE:
                 continue
-            colliding = sorted(str(f.path) for f in lang_includes.files if f.path in default_paths)
+            colliding = sorted(str(f.path) for f in lang_includes.files.files if f.path in default_paths)
             if colliding:
                 names = ', '.join(colliding)
                 diag.error(f'Include files for language "{lang_id}" collide with "{DEFAULT_LANGUAGE}" include files: {names}')
 
 
+def _check_default_file_directory_collision(includes: Includes, diag: Diagnostics) -> None:
+    """Flag paths that are a file in the "default" include files and a directory in another
+    language's include files, or vice versa. Merging those is impossible."""
+
+    def files_and_dirs(paths: list[Path]) -> tuple[set[Path], set[Path]]:
+        return set(paths), {parent for path in paths for parent in path.parents} - {Path('.')}
+
+    default_files, default_dirs = files_and_dirs(_default_include_paths(includes))
+    if not default_files:
+        return
+    for lang_id, lang_includes in includes.languages.items():
+        if lang_id == DEFAULT_LANGUAGE:
+            continue
+        lang_files, lang_dirs = files_and_dirs([f.path for f in lang_includes.files.files])
+        colliding = sorted(str(path) for path in (default_files & lang_dirs) | (lang_files & default_dirs))
+        if colliding:
+            names = ', '.join(colliding)
+            diag.error(
+                f'Include files for language "{lang_id}" and "{DEFAULT_LANGUAGE}" have paths that are a file in one '
+                f'and a directory in the other: {names}'
+            )
+
+
 def _check_ambiguous_mainfile(includes: Includes, language_config: Languages, diag: Diagnostics) -> None:
     """Flag languages whose own include files have more than one plausible mainfile."""
     for lang_id, lang_includes in includes.languages.items():
@@ -64,7 +88,7 @@ def _check_ambiguous_mainfile(includes: Includes, language_config: Languages, di
         if language is None:
             continue
 
-        candidates = language.mainfile_candidates([f.path for f in lang_includes.files])
+        candidates = language.mainfile_candidates([f.path for f in lang_includes.files.files])
         if len(candidates) > 1:
             names = ', '.join(str(candidate) for candidate in candidates)
             diag.error(f'Include files for language "{lang_id}" have multiple possible mainfiles: {names}')

problemtools/checks/problem_package.py

@@ -5,6 +5,7 @@
 import difflib
 import os
 import re
+import stat
 from pathlib import Path
 
 from ..diagnostics import Diagnostics
@@ -16,6 +17,7 @@
 def check_problem_package(probdir: Path, format_version: FormatVersion, diag: Diagnostics) -> None:
     """Run all checks on the structure of a problem package."""
     _check_symlinks(probdir, diag)
+    _check_file_types(probdir, diag)
     _check_file_and_directory_names(probdir, diag)
     _check_root_directory_names(probdir, format_version, diag)
 
@@ -40,6 +42,22 @@ def _check_symlinks(probdir: Path, diag: Diagnostics) -> None:
                     diag.error(f'Symlink {relfile} links to {reltarget} which is an absolute path. Symlinks must be relative.')
 
 
+def _check_file_types(probdir: Path, diag: Diagnostics) -> None:
+    """Check that the package only contains regular files, directories and symlinks
+    (symlinks are checked by _check_symlinks), and warn about empty directories."""
+    for root, dirs, files in os.walk(probdir):
+        if root == str(probdir) and '.git' in dirs:
+            dirs.remove('.git')
+        # Path of the directory we're in, starting with problem shortname. Only used for nicer error messages.
+        reldir = os.path.relpath(root, probdir.parent)
+        if not dirs and not files:
+            diag.warning(f'Empty directory {reldir}')
+        for file in files:
+            mode = os.lstat(os.path.join(root, file)).st_mode
+            if not stat.S_ISREG(mode) and not stat.S_ISLNK(mode):
+                diag.error(f"'{file}' in {reldir} is not a regular file, directory or symlink")
+
+
 def _check_file_and_directory_names(probdir: Path, diag: Diagnostics) -> None:
     def _special_case_allowed_files(file: str, reldir: str) -> bool:
         return file == '.gitignore' or (file == '.timelimit' and reldir == probdir.name)

problemtools/model/__init__.py

@@ -5,8 +5,9 @@
 
 from .attachments import Attachments, load_attachments
 from .graders import DEFAULT_GRADER, Graders, load_graders
-from .includes import DEFAULT_LANGUAGE, IncludeFile, Includes, LanguageIncludes, load_includes
+from .includes import DEFAULT_LANGUAGE, Includes, LanguageIncludes, load_includes
 from .problem import Problem, load_problem
+from .program_files import ProgramFile, ProgramFiles, load_program_files
 from .statements import Statements, load_statements
 from .submissions import LegacyPolicy, Submission, Submissions, load_submissions
 from .testdata import DEFAULT_CONFIG, SCORING_ONLY_KEYS, TestCase, TestDataGroup, load_testdata
@@ -20,13 +21,14 @@
     'SCORING_ONLY_KEYS',
     'Attachments',
     'Graders',
-    'IncludeFile',
     'Includes',
     'InputValidators',
     'LanguageIncludes',
     'LegacyPolicy',
     'OutputValidators',
     'Problem',
+    'ProgramFile',
+    'ProgramFiles',
     'Statements',
     'Submission',
     'Submissions',
@@ -39,6 +41,7 @@
     'load_input_validators',
     'load_output_validators',
     'load_problem',
+    'load_program_files',
     'load_statements',
     'load_submissions',
     'load_testdata',

problemtools/model/includes.py

@@ -3,27 +3,22 @@
 
 from ..languages import Language, Languages
 from .paths import RelativePath, relpath, resolve
+from .program_files import ProgramFiles, load_program_files
 
 #: Pseudo-language whose include files are added for every language.
 DEFAULT_LANGUAGE = 'default'
 
 
 @dataclass(frozen=True)
-class IncludeFile:
-    """A single include file.
+class LanguageIncludes:
+    """Include files for a language.
 
-    `path` is relative to the include directory for its language, e.g. for
+    File paths are relative to the include directory for the language, e.g. for
     include/cpp/Vector/Vector.h, path is Vector/Vector.h.
     """
 
-    path: RelativePath
-    data: bytes
-
-
-@dataclass(frozen=True)
-class LanguageIncludes:
-    mainfile: str | None = None
-    files: list[IncludeFile] = field(default_factory=list)
+    mainfile: RelativePath | None = None
+    files: ProgramFiles = field(default_factory=ProgramFiles)
 
 
 @dataclass(frozen=True)
@@ -38,11 +33,12 @@ class Includes:
     def get_includes_for_language(self, language: str) -> LanguageIncludes:
         """All includes relevant for `language`: the files registered for
         DEFAULT_LANGUAGE (which apply to every language) plus those registered
-        for `language` itself, with the mainfile taken from `language`.
+        for `language` itself, with the mainfile taken from `language`. Where
+        paths coincide, the file registered for `language` wins.
         """
         default_includes = self.languages.get(DEFAULT_LANGUAGE, LanguageIncludes())
         lang_includes = self.languages.get(language, LanguageIncludes())
-        return LanguageIncludes(mainfile=lang_includes.mainfile, files=default_includes.files + lang_includes.files)
+        return LanguageIncludes(mainfile=lang_includes.mainfile, files=default_includes.files.merged(lang_includes.files))
 
 
 def load_includes(probdir: Path, language_config: Languages) -> Includes:
@@ -59,14 +55,13 @@ def load_includes(probdir: Path, language_config: Languages) -> Includes:
 
 
 def _load_language_includes(lang_dir: Path, language: Language | None) -> LanguageIncludes:
-    paths = sorted(p for p in lang_dir.rglob('*') if p.is_file())
-    files = [IncludeFile(path=relpath(path.relative_to(lang_dir)), data=path.read_bytes()) for path in paths]
+    files = load_program_files(lang_dir)
 
-    mainfile = None
+    mainfile: RelativePath | None = None
     if language is not None:
-        source_files = language.get_source_files(paths)
+        source_files = language.get_source_files([f.path for f in files.files])
         candidates = language.mainfile_candidates(source_files)
         if candidates:
-            mainfile = str(Path(candidates[0]).relative_to(lang_dir))
+            mainfile = relpath(candidates[0])
 
     return LanguageIncludes(mainfile=mainfile, files=files)

problemtools/model/program_files.py

@@ -0,0 +1,70 @@
+"""In-memory snapshot of the files making up a program (e.g. a submission or validator)."""
+
+import os
+from dataclasses import dataclass, field
+from pathlib import Path
+
+from .paths import RelativePath, relpath
+
+
+@dataclass(frozen=True)
+class ProgramFile:
+    """A single file of a program.
+
+    `path` is relative to the program's root, e.g. for
+    submissions/accepted/hello/src/main.cpp, path is src/main.cpp.
+    """
+
+    path: RelativePath
+    data: bytes
+    executable: bool
+
+
+@dataclass(frozen=True)
+class ProgramFiles:
+    """All files of a program, sorted by path."""
+
+    files: list[ProgramFile] = field(default_factory=list)
+
+    def merged(self, other: 'ProgramFiles') -> 'ProgramFiles':
+        """Files from both self and other; where paths coincide, the file from other wins."""
+        by_path = {file.path: file for file in self.files} | {file.path: file for file in other.files}
+        return ProgramFiles(files=sorted(by_path.values(), key=lambda file: file.path))
+
+    def materialize(self, dest: Path) -> None:
+        """Write the files into dest, creating subdirectories as needed.
+
+        Existing files are overwritten. Files are written with mode 0755 if
+        executable, 0644 otherwise.
+        """
+        for file in self.files:
+            target = dest / file.path
+            target.parent.mkdir(parents=True, exist_ok=True)
+            target.write_bytes(file.data)
+            target.chmod(0o755 if file.executable else 0o644)
+
+    def size(self) -> int:
+        """Total size of the files, in bytes."""
+        return sum(len(file.data) for file in self.files)
+
+
+def load_program_files(path: Path) -> ProgramFiles:
+    """Load the files of the program at path.
+
+    path is either a single file (giving a program with that one file), or a
+    directory, which is read recursively, following symlinks. A file counts as
+    executable if any of its execute bits are set.
+    """
+    if path.is_file():
+        return ProgramFiles(files=[_load_file(path, relpath(Path(path.name)))])
+
+    files = []
+    for root, _, filenames in os.walk(path, followlinks=True):
+        for filename in filenames:
+            file_path = Path(root) / filename
+            files.append(_load_file(file_path, relpath(file_path.relative_to(path))))
+    return ProgramFiles(files=sorted(files, key=lambda file: file.path))
+
+
+def _load_file(path: Path, relative: RelativePath) -> ProgramFile:
+    return ProgramFile(path=relative, data=path.read_bytes(), executable=bool(path.stat().st_mode & 0o111))

problemtools/run/__init__.py

@@ -91,11 +91,11 @@ def get_program(
         a Program instance, or None if no program was found at
         the given path.
     """
-    if includes is None:
-        # Imported lazily (rather than at module scope) since `model` depends on `run`
-        # (e.g. for `run.find_programs`), so importing it here avoids a circular import.
-        from ..model import Includes
+    # Imported lazily (rather than at module scope) since `model` depends on `run`
+    # (e.g. for `run.find_programs`), so importing it here avoids a circular import.
+    from ..model import Includes, load_program_files
 
+    if includes is None:
         includes = Includes()
 
     if path.is_file():
@@ -108,12 +108,12 @@ def get_program(
     else:
         build = path / 'build'
         if build.is_file() and os.access(build, os.X_OK):
-            return BuildRun(path)
+            return BuildRun(path.name, load_program_files(path))
         files = rutil.list_files_recursive(path)
 
     lang = language_config.detect_language(files)
     if lang is not None:
-        return SourceCode(path, lang, includes=includes.get_includes_for_language(lang.lang_id))
+        return SourceCode(path.name, load_program_files(path), lang, includes=includes.get_includes_for_language(lang.lang_id))
     return None
 
 

problemtools/run/buildrun.py

@@ -6,29 +6,32 @@
 import subprocess
 import tempfile
 from pathlib import Path
+from typing import TYPE_CHECKING
 
-from . import rutil
 from .errors import ProgramError
 from .program import CompileResult, Program
 
+if TYPE_CHECKING:
+    from ..model import ProgramFiles
+
 
 class BuildRun(Program):
     """Class for build/run-script program."""
 
-    def __init__(self, path: Path) -> None:
+    files: 'ProgramFiles'  # The program's files, including the build script
+
+    def __init__(self, name: str, files: 'ProgramFiles') -> None:
         """Instantiate BuildRun object.
 
         Args:
-            path: directory containing the build script.
+            name: name of the program.
+            files: the program's files, including the build script (see load_program_files).
         """
-        if not path.is_dir():
-            raise ProgramError(f'{path} is not a directory')
-
-        super().__init__(name=path.name)
-        self._source_path = path
+        super().__init__(name=name)
+        self.files = files
 
     def do_compile(self, work_dir: Path) -> CompileResult:
-        """Set up the compile work-space (copying the build script and friends into
+        """Set up the compile work-space (writing the build script and friends into
         work_dir) and run the build script."""
         name = self.name
         run_path = work_dir / name
@@ -38,13 +41,16 @@ def do_compile(self, work_dir: Path) -> CompileResult:
             run_path.mkdir(parents=True)
         self._path = run_path
 
-        rutil.add_files(self._source_path, self.path)
+        try:
+            self.files.materialize(self.path)
+        except OSError as e:
+            return CompileResult(False, f'Failed to write program files: {e}', self.path)
 
         build = self.path / 'build'
         if not build.is_file():
-            raise ProgramError(f'{self._source_path} does not have a build script')
+            raise ProgramError(f'{self.name} does not have a build script')
         if not os.access(build, os.X_OK):
-            raise ProgramError(f'{self._source_path}/build is not executable')
+            raise ProgramError(f'{self.name}/build is not executable')
 
         try:
             subprocess.check_output(['./build'], stderr=subprocess.STDOUT, cwd=self.path)