diff --git a/README.rst b/README.rst
index 04b526b..a2465c1 100644
--- a/README.rst
+++ b/README.rst
@@ -85,12 +85,17 @@ Create a repository, stage a remote catalog, then commit it::
hm download --all --dry-run
hm download --all
-``hm`` is the CLI command; ``hallmark`` remains an alias. Initialization creates the empty
-local repository. Remote ``add`` discovers files without downloading dataset
-contents or committing the catalog. CyVerse, ordinary browsable HTTPS
-directories, SSH and SFTP use the same workflow. Omit the filter to catalog
-all discoverable files beneath the supplied URL.
-
+``hm`` is the CLI command; ``hallmark`` remains an alias. Commands work from any
+folder inside a repository; ``hm`` finds ``.hm`` in parent folders. If ``.hm``
+is damaged or missing files, commands explain the problem and change nothing.
+Initialization creates the empty local repository. Remote ``add`` discovers
+files without downloading dataset contents or committing the catalog. CyVerse,
+ordinary browsable HTTPS directories, SSH and SFTP use the same workflow. Omit
+the filter to catalog all discoverable files beneath the supplied URL.
+
+``hm download PATH...`` selects catalogued files or folders instead of
+``--all``; ``--filter`` and ``--fmt`` only narrow a selection, and paths that
+are not in the catalog stop the download before the server is contacted.
Every nonempty CLI transfer asks for confirmation. For separate Python
downloads, callers can inspect
``repo.plan_download()`` and then execute ``repo.download(plan, approved=True)``.
@@ -99,10 +104,12 @@ Optional authentication profiles remain local and can be selected with
The obsolete ``build`` and remote initialization interfaces have been removed.
Use ``init``, ``add`` and ``commit`` for new remote catalogs.
-Use ``hm clone CATALOG PATH`` for an existing Git-hosted ``.hm`` or a
-published HTTP/SFTP catalog snapshot. Catalogs can live on GitHub or another
+Use ``hm clone CATALOG [PATH]`` for an existing Git-hosted ``.hm`` or a
+published HTTP/SFTP catalog snapshot. ``PATH`` defaults to a new folder named
+after the source and must otherwise be new or empty, outside any Hallmark
+repository. Catalogs can live on GitHub or another
server while their data remotes point elsewhere. Git clones preserve the full
-catalog and its history. Clone displays a data download plan and asks for
+catalog, its history and every branch, after checking each branch's catalog. Clone displays a data download plan and asks for
confirmation by default; declining keeps the catalog. Use ``hm clone CATALOG
PATH --no-download`` for catalog only. Filters narrow the download and cannot
accompany ``--no-download``. Python ``Repo.clone`` downloads without prompting
@@ -123,7 +130,9 @@ Remote ``add`` supports one dataset root and one ``data.tsv`` table per
repository. Keep remote catalogs separate from locally versioned files.
Local ``add`` selects files by filename format; ``commit`` stores their
contents in ``.hm/objects``. Clone copies catalog history and downloads from
-its configured data remote; transfer of a local object store is not implemented.
+its configured data remote. Repositories with a branch that tracks local files
+cannot be cloned yet: clone reports this and creates nothing, even with
+``--no-download``. Empty repositories and remote catalogs can be cloned.
Development and support
-----------------------
diff --git a/demo/demo_cli.ipynb b/demo/demo_cli.ipynb
index 4c2107b..2d866e6 100644
--- a/demo/demo_cli.ipynb
+++ b/demo/demo_cli.ipynb
@@ -438,7 +438,7 @@
"source": [
"### 2.4 Download some files\n",
"\n",
- "`--filter` picks files by name. `*` matches any characters.\n",
+ "`--all` selects every file in the catalog, and `--filter` keeps only the names that match. `*` matches any characters.\n",
"Hallmark shows the plan and asks `[y/N]`. Here we answer `y` with `echo y`.\n",
"Each file is checked against its checksum."
]
@@ -450,7 +450,7 @@
"metadata": {},
"outputs": [],
"source": [
- "echo y | hm download --filter \"SgrA_*\" 2>/dev/null\n",
+ "echo y | hm download --all --filter \"SgrA_*\" 2>/dev/null\n",
"ls SgrA_*.txt"
]
},
@@ -541,7 +541,7 @@
"source": [
"echo n | hm clone eht-data/.hm later-copy\n",
"cd later-copy\n",
- "echo y | hm download --filter \"M87_*\" 2>/dev/null\n",
+ "echo y | hm download --all --filter \"M87_*\" 2>/dev/null\n",
"cd .."
]
},
@@ -588,7 +588,8 @@
"| `hm checkout NAME` | Switch to a branch, or create it |\n",
"| `hm set-config --fmt \"PATTERN\"` | Change the file name pattern |\n",
"| `hm set-config --encoding \"FIELD=REGEX\"` | Add an encoding rule |\n",
- "| `hm download --all` / `--filter` / `PATHS` | Download files in the catalog (asks first) |\n",
+ "| `hm download --all` / `PATHS` | Download all files, or files and folders in the catalog (asks first) |\n",
+ "| `hm download ... --filter \"GLOB\"` | Download only the selected files that match (asks first) |\n",
"| `hm download --dry-run` | Show what would be downloaded |\n",
"| `hm clone SOURCE DIR` | Copy a repository and download its data (asks first) |\n",
"| `hm clone SOURCE DIR --no-download` | Copy only the catalog and history |"
diff --git a/doc/api.rst b/doc/api.rst
index 689295e..e5ee7bc 100644
--- a/doc/api.rst
+++ b/doc/api.rst
@@ -98,9 +98,11 @@ Stage and commit a remote catalog without downloading dataset files::
hm add 'ssh://lab-data/srv/data/run{run:d}.h5'
hm commit -m 'Add remote data'
-Use ``hm clone SOURCE PATH`` for an existing Hallmark Git repository
-or published HTTP/SFTP snapshot. Git clones retain the full catalog and its
-history; snapshots start new local history. Use ``--source-type git`` or
+Use ``hm clone SOURCE [PATH]`` for an existing Hallmark Git repository
+or published HTTP/SFTP snapshot. ``PATH`` defaults to a new folder named after
+the source; it must be new or empty and outside any Hallmark repository,
+including the source. Git clones retain the full catalog, its history and
+every branch; snapshots start new local history. Use ``--source-type git`` or
``--source-type catalog`` to override automatic detection.
Downloading remote data
@@ -109,11 +111,14 @@ Downloading remote data
Preview the selected files using the local catalog, then confirm a download::
hm download --all --dry-run
- hm download --filter 'runs/**'
-
-Explicit paths and ``--tsv data.tsv`` also select files. Every nonempty transfer
-in the CLI requires interactive confirmation, including the default clone
-transfer. ``clone --no-download`` skips data and the prompt. Clone filters and
+ hm download runs
+
+Choose catalogued paths or folders, relative to the current folder, or
+``--all`` or ``--tsv data.tsv``; ``--filter`` and ``--fmt`` only narrow that
+selection, as in ``hm download --all --filter 'runs/**'``. Paths that are not
+in the catalog stop the download before the server is contacted. Every
+nonempty transfer in the CLI requires interactive confirmation, including the
+default clone transfer. ``clone --no-download`` skips data and the prompt. Clone filters and
formats cannot be combined with this flag and leave the complete catalog unchanged.
Declining clone's prompt keeps the catalog and exits successfully.
A filter or filename format never authorizes a transfer.
diff --git a/doc/backends.rst b/doc/backends.rst
index c321cd9..c097f50 100644
--- a/doc/backends.rst
+++ b/doc/backends.rst
@@ -114,8 +114,8 @@ as missing objects.
Discovery reads listings and published metadata only. It must not download
payloads to infer checksums. The shared downloader owns approval, destination
-path validation, checksum verification, atomic replacement and worker
-concurrency. Backends map logical paths to server paths or API requests and
+path validation, checksum verification, skipping or refusing existing files,
+moving finished files into place and worker concurrency. Backends map logical paths to server paths or API requests and
write only to the temporary destination they are given.
An instance can serve concurrent ``fetch`` calls. Protect shared mutable
diff --git a/doc/private_data.rst b/doc/private_data.rst
index ecfdf6d..83ddab4 100644
--- a/doc/private_data.rst
+++ b/doc/private_data.rst
@@ -66,8 +66,12 @@ Suppose the export contains ``runs/run_001.h5``, ``runs/run_002.h5`` and
This creates ``./lab/.hm`` and catalogs only the matching run files. The URL
is the exact discovery root. A filename format selects paths and extracts
-parameters; it does not approve a download. A glob filter can select paths
-without defining parameters:
+parameters; it does not approve a download. Each branch has one filename
+format: adding the bare URL again rescans with the saved format, and a new
+format is refused, before the server is contacted, if any catalogued file does
+not fit it. There is no option to force it; a broader format that fits every
+catalogued file replaces the old one. A glob filter can select paths without
+defining parameters:
.. code-block:: bash
@@ -83,6 +87,11 @@ directories needed to find matching files. Discovery shows an indeterminate
progress bar with counts of completed directories and discovered files.
The percentage and completion time remain unknown until a total is available.
+A scan stages its catalog only after it completes. If the connection or access
+fails, or no file matches the URL, filter and filename format, ``add`` explains
+the problem, exits with an error and keeps the previously staged catalog
+unchanged; it never stages a partial result.
+
Both URL schemes use structured SFTP directory enumeration and file transfers.
SFTP-only accounts work for discovery as well as downloading. No
``--allow-remote-commands`` or ``--remote-hash`` option is needed. Discovery
@@ -104,6 +113,17 @@ nonsecret backend settings. These settings are saved with the data remote;
credentials belong in local authentication configuration. See :doc:`backends`
for the plugin interface.
+Keep passwords and keys outside ``.hm``. ``add URL`` and
+``set-config --remote-url`` reject URLs containing a password, an HTTP(S)
+username or token, a query string or a fragment, before contacting the server
+and without repeating the URL. Store SSH users and keys in ``~/.ssh/config`` or
+a local profile in ``~/.config/hallmark/auth.yml`` (see section 4), and
+HTTP(S) logins in ``~/.netrc``. An SSH username such as
+``ssh://researcher@lab-data/srv/exports/lab/`` is not a secret and is allowed.
+``clone`` likewise refuses a source URL with a password or an HTTP(S) username
+or token, before creating anything, because Git would save it in
+``.hm/.git/config``; use a Git credential helper or ``~/.netrc`` instead.
+
3. Preview, approve and download
--------------------------------
@@ -117,7 +137,8 @@ From the directory containing ``lab``, enter that repository:
hm download runs/run_001.h5
The dry run reads only the local catalog. It reports file count, known bytes,
-the number of unknown file sizes, destination and source. The CLI reports
+the number of unknown file sizes, destination and source, then lists the
+selected files and any paths that are not in the catalog. The CLI reports
duration as unknown. Python callers can supply a transfer rate to estimate
duration when all file sizes are known. The dry run does not test credentials,
host trust or remote-file existence.
@@ -133,21 +154,42 @@ Choose a scope from inside ``lab``:
.. code-block:: bash
- hm download --filter 'runs/run_00[12].h5' --dry-run
+ hm download runs --dry-run
+ hm download --all --filter 'runs/run_00[12].h5' --dry-run
hm download --all --output ./payload
-Explicit paths use their recorded catalog checksums when available, just like
-``--tsv`` and ``--all``. A path absent from the catalog can be requested, but
-its size and checksum are unknown. Files without a usable publisher checksum
-can be downloaded, but their contents cannot be checked against the catalog.
+Choose files with catalogued paths or folders, ``--tsv`` or ``--all``. Paths
+are relative to the current folder, which can be any folder inside the
+repository. A folder selects every catalogued file below it, including
+subfolders; ``.`` selects everything below the current folder. ``--filter``
+and ``--fmt`` only narrow that selection: on their own they show usage and
+download nothing. A path that matches no catalogued file, such as a typo or a
+folder without catalogued files, stops the download before the server is
+contacted; ``--dry-run`` lists such paths. Selected files use their recorded
+catalog checksums. Files without a usable publisher checksum can be
+downloaded, but their contents cannot be checked against the catalog.
``--tsv`` can be repeated and combined with explicit paths; overlapping entries
are deduplicated. ``--all`` cannot be combined with either selection mode.
Relative output paths are relative to the current directory. Without
``--output``, downloads go to the repository worktree; bare ``.hm`` repositories
-require an explicit output directory. Repeating a download transfers the
-selection again and replaces destinations only after successful transfer and
-any recorded checksum verification.
+require an explicit output directory.
+
+Existing files are never replaced. Before contacting the server, a download
+checks each selected file that already exists at its destination, first by
+its recorded size and then by its catalog checksum. A file with its catalog
+checksum is reported as already downloaded and skipped, so repeating a
+download, for example after a failed transfer, fetches only what is missing.
+When the catalog has no checksum for a file, as for SSH exports without a
+checksum manifest, a file with the recorded size is reported as present (size
+matches, not verified) and also skipped. A file with a different size or
+contents, a file the catalog records neither a checksum nor a size for, or a
+folder in its place is a conflict: the download stops and transfers nothing.
+Delete conflicting files first to replace them; there is no overwrite option.
+``--dry-run`` lists files to download, skipped and unverified files, conflicts
+and unknown paths. Checksums from manifests that do not name their algorithm,
+such as ``checksums.txt``, are used when the digest length identifies MD5,
+SHA-1, SHA-256 or SHA-512.
After committing the catalog, use ``download`` to review and approve a transfer.
Declining keeps the catalog available; an empty selection needs no approval:
@@ -168,8 +210,26 @@ HTTP/SFTP directory containing its metadata. For example, copy this catalog:
hm clone ./.hm ../lab-copy
-Git endpoints preserve the complete catalog and history. Published HTTP/SFTP
-snapshots start new local history. A snapshot URL may name the metadata
+Without a destination, ``hm clone`` creates a folder in the current folder
+named after the source: ``lab`` for ``./lab/.hm`` or
+``https://github.com/example/lab.git``. A destination must be a new or empty
+folder outside every Hallmark repository, including the source, because
+repositories cannot be nested; these checks happen before the source is read.
+A failed clone removes only what it created: the destination folder if it was
+new, or the ``.hm`` it added to an existing empty folder. It never follows a
+symbolic link while cleaning up.
+Only empty repositories and remote catalogs can be cloned: if any branch of the
+source tracks local files, whose contents live in its ``.hm/objects``, clone
+reports it and creates nothing, even with ``--no-download``. A local source is
+checked before anything is copied; a Git host or snapshot is checked after its
+metadata is read, and the partial copy is removed.
+
+Git endpoints preserve the complete catalog and history, with a local branch
+for every branch of the source; the copy starts on the branch the source has
+selected. Before finishing, clone checks every branch's committed catalog:
+readable ``config.yml``, ``meta.yml`` and tables, safe relative paths and
+well-formed checksums. An invalid catalog stops the clone and removes the
+copy. Published HTTP/SFTP snapshots start new local history. A snapshot URL may name the metadata
directory itself or its parent containing ``.hm``. Use ``--source-type git``
to require Git or ``--source-type catalog`` to require a snapshot. Automatic
selection treats local paths, SCP-style addresses, Git/file/SSH URLs and URLs
@@ -190,6 +250,10 @@ confirmation; Enter or ``n`` keeps the catalog and exits successfully:
Use ``--no-download`` for catalog only, without a prompt. It cannot be combined
with ``--filter`` or ``--fmt`` and is required for bare destinations. An empty
selection prints ``No files selected for download.`` without prompting.
+If the download cannot run or a file fails, clone reports the copy as
+incomplete. It keeps the catalog and the files that downloaded, and names the
+``hm download --all`` command, with the same filters, that retries; files
+already downloaded are skipped.
Git authentication and data authentication are independent; catalog-only cloning
does not require credentials for its data.
@@ -283,9 +347,11 @@ Without ``approved=True``, a nonempty transfer raises ``DownloadError`` before
opening a connection. A plan freezes the selected files, source URL, profile
reference, backend, backend options and destination. Later catalog or remote
configuration changes do not redirect it. ``plan_download(filter="**/*.h5")``
-selects matching files from the catalog;
-``plan_download(output_path="subset", tsv_names=["data.tsv"])`` selects a TSV
-and destination. Planning makes no network requests. Known size totals and
+selects matching files from the catalog; ``plan_download(file_paths=["runs"])``
+selects a folder; ``plan_download(output_path="subset", tsv_names=["data.tsv"])``
+selects a TSV and destination. Planning makes no network requests. Requested
+paths that match no catalogued file are listed in ``plan.unknown_paths``;
+downloading such a plan raises ``DownloadError`` before opening a connection. Known size totals and
unknown-size counts are available as ``known_bytes`` and
``unknown_size_count``. ``estimated_seconds`` stays ``None`` unless all sizes
are known and ``estimated_bytes_per_second`` was supplied when planning.
@@ -346,7 +412,7 @@ access retains Requests' normal ``.netrc`` and environment behavior; Hallmark's
named auth profiles apply to SSH/SFTP.
From either initialized catalog, inspect and approve a selected subset using
-``hm download --filter PATTERN --dry-run`` and then the same command
+``hm download --all --filter PATTERN --dry-run`` and then the same command
without ``--dry-run``. Python follows the same steps:
.. code-block:: python
@@ -425,10 +491,12 @@ catalog hosting independent of data hosting.
Download behavior and troubleshooting
-------------------------------------
-Files are downloaded to temporary paths and published atomically after transfer
-and any recorded checksum verification. A failed transfer preserves an existing
-destination and removes its temporary file. Successful files remain available
-when another file fails; a multi-file download is not a single transaction.
+Unfinished downloads are kept in temporary ``.part`` files beside their
+destinations and moved into place only after the transfer and any recorded
+checksum verification succeed. A failed transfer removes its temporary file.
+A destination that appears while the download runs is not replaced; that file
+fails instead. Successful files remain available when another file fails; a
+multi-file download is not a single transaction.
The CLI exits nonzero on transfer failures. Cancellation stops the active
operation and closes the SSH connections and processes it started.
@@ -451,6 +519,9 @@ operation and closes the SSH connections and processes it started.
* - Checksum mismatch
- Check whether the export changed since catalog creation. Reconcile the
catalog and source against a trusted version before retrying.
+ * - Existing files conflict with the catalog
+ - Compare the listed files with the catalog. Delete or move them, then
+ download again; Hallmark never overwrites existing files.
* - Transfer timeout or session limit
- Adjust the local per-file time budget or lower concurrency to match
the server's capacity.
diff --git a/doc/usecase.rst b/doc/usecase.rst
index c6fd20e..466b62b 100644
--- a/doc/usecase.rst
+++ b/doc/usecase.rst
@@ -155,13 +155,16 @@ The ``--remote`` option selects one data server; |hallmark|_ does not
automatically try another server if the download fails.
Downloads are written to temporary files and checked against any
-checksums supplied with the selection before atomically replacing
-their destinations.
+checksums supplied with the selection before they are moved into place.
The ``--tsv`` and ``--all`` options include catalog checksums. Explicit
-paths also use their recorded checksums when available.
-If a transfer or checksum check fails, the existing destination is
-preserved and the temporary file is removed.
+paths and folders must be in the catalog and use their recorded checksums.
+If a transfer or checksum check fails, the temporary file is removed.
Files that have already downloaded successfully remain available.
+Existing files are never replaced: files that already match their
+catalog checksum, or their recorded size when the catalog has no checksum,
+are skipped, and a file with different or uncheckable contents stops the
+download before anything is transferred. Delete conflicting files first to
+replace them.
SSH access and file paths
~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -294,7 +297,8 @@ an apparently complete catalog.
The generated data remote ``origin`` records the source URL and optional
profile name.
Existing Git-hosted catalogs keep their complete catalog, history and
-recorded data remotes when cloned. Clone offers data downloads by default and
+recorded data remotes when cloned. Repositories that track local files cannot
+be cloned yet. Clone offers data downloads by default and
asks for confirmation. ``--no-download`` copies only the catalog. Clone filters
narrow the download and cannot accompany ``--no-download``; the catalog and
history remain unchanged.
diff --git a/mod/hallmark/__init__.py b/mod/hallmark/__init__.py
index 9ddf4d9..b20c557 100644
--- a/mod/hallmark/__init__.py
+++ b/mod/hallmark/__init__.py
@@ -17,13 +17,14 @@
from .repo import Repo
from .paraframe import ParaFrame
-from .remote.plan import DownloadItem, DownloadPlan
+from .remote.plan import DownloadConflict, DownloadItem, DownloadPlan
from .remote.backends import (
DataBackend, HttpBackend, SshBackend, CyVerseBackend, RemoteEntry,
register_backend,
)
__all__ = [
- "Repo", "ParaFrame", "DownloadItem", "DownloadPlan", "DataBackend",
- "HttpBackend", "SshBackend", "CyVerseBackend", "RemoteEntry", "register_backend",
+ "Repo", "ParaFrame", "DownloadConflict", "DownloadItem", "DownloadPlan",
+ "DataBackend", "HttpBackend", "SshBackend", "CyVerseBackend", "RemoteEntry",
+ "register_backend",
]
diff --git a/mod/hallmark/cli.py b/mod/hallmark/cli.py
index ca1996b..574dddb 100644
--- a/mod/hallmark/cli.py
+++ b/mod/hallmark/cli.py
@@ -14,6 +14,7 @@
"""Hallmark CLI entrypoint and command wiring."""
+import os
from contextlib import contextmanager
from pathlib import Path
@@ -24,8 +25,10 @@
from . import Repo
from .remote.download import DownloadError
+from .remote.clone import default_clone_destination
from .remote.discovery import path_matches
-from .error import CheckoutError, CloneError
+from .error import CheckoutError, CloneError, DothmError
+from .utils import use_working_directory
# use a context manager to translate application errors into clean Click errors
@@ -117,7 +120,9 @@ def _run_download(repo, plan, *, max_workers):
"""
click.echo(plan.summary())
if not plan.file_count:
- click.echo("No files selected for download.")
+ click.echo("All selected files are already downloaded."
+ if plan.skipped or plan.unverified
+ else "No files selected for download.")
return
click.confirm("Download these files?", default=False, abort=True)
results = repo.download(plan, approved=True, max_workers=max_workers,
@@ -138,9 +143,58 @@ def hallmark(ctx):
if ctx.invoked_subcommand in [None, "init", "clone"]:
# return early without attempting to open a repository
return
- # attempt to open the hallmark repository in the current directory
+ # open the repository containing the current folder, searching its parents
with _translate_cli_errors(GitError, prefix="Failed to open hallmark repository"):
- ctx.obj = Repo(".")
+ with _translate_cli_errors(DothmError):
+ ctx.obj = Repo.find(Path.cwd())
+
+
+def _repo_relative_paths(repo, paths):
+ """
+ Convert paths given relative to the current folder into repository paths.
+
+ Bare repositories have no worktree, so their paths are already relative to
+ the catalog root and are returned unchanged.
+
+ Args:
+ repo: The hallmark repository object.
+ paths (sequence[str]): Paths as typed by the user.
+
+ Returns:
+ tuple[str, ...]: POSIX paths relative to the worktree; "." names the
+ whole worktree.
+
+ Raises:
+ ClickException: If a path is outside the worktree or inside ``.hm``.
+ """
+ if repo.worktree is None:
+ return tuple(paths)
+ worktree = Path(repo.worktree)
+ converted = []
+ for value in paths:
+ # Normalize lexically: catalog paths are literal and need not exist yet.
+ absolute = Path(os.path.normpath(Path.cwd() / value))
+ try:
+ relative = absolute.relative_to(worktree)
+ except ValueError:
+ raise ClickException(
+ f'"{value}" is outside the repository worktree "{worktree}"') from None
+ if relative.parts and relative.parts[0].lower() == ".hm":
+ raise ClickException(f'"{value}" is inside .hm, not a data path')
+ converted.append(relative.as_posix())
+ return tuple(converted)
+
+
+def _names_worktree_root(repo, value):
+ """Return whether an add input is "." or another path to the worktree root."""
+ if repo.worktree is None:
+ return False
+ if value == ".":
+ return True
+ try:
+ return (Path.cwd() / value).resolve() == Path(repo.worktree).resolve()
+ except (OSError, RuntimeError, ValueError):
+ return False
def _load_backend_options(path):
@@ -254,11 +308,14 @@ def add(repo, encoding, inputs, auth, backend, backend_options, filters, remote_
"""Add files to the hallmark index.
A remote URL or URL pattern stages a catalog without downloading files
- or committing. Use --fmt and --filter to select remote paths.
+ or committing. Use --fmt and --filter to select remote paths. A scan that
+ fails or matches no files keeps the previously staged catalog.
`hm add [--regex] FORMAT` uses the branch format string workflow.
`hm add "."` rebuilds the manifest from current files that match
- the branch `fmt` in `config.yml`.
+ the branch `fmt` in `config.yml`, across the whole repository from any
+ folder inside it. Any other path to the worktree root, such as `..` from
+ a subfolder, does the same.
Explicit path inputs such as shell-expanded `*` are not supported yet
with the parameter-based manifest format.
"""
@@ -276,6 +333,11 @@ def add(repo, encoding, inputs, auth, backend, backend_options, filters, remote_
remote = "://" in inputs[0]
if remote or any(value is not None for value in remote_options.values()):
pf = repo.add(inputs[0], encoding, progress=True, **remote_options)
+ elif _names_worktree_root(repo, inputs[0]):
+ # Like ".", a path naming the worktree root stages the whole
+ # repository from any folder inside it.
+ with use_working_directory(repo.worktree):
+ pf = repo.add(".", encoding)
else:
pf = repo.add(inputs[0], encoding)
# oterhwise, use the add_paths method for multiple inputs
@@ -399,6 +461,35 @@ def checkout(repo, target_branch):
click.echo(f'Switched to branch "{target_branch}".')
+def _echo_paths(title, paths, limit=20):
+ """Print a titled list of paths, abbreviating long lists."""
+ if not paths:
+ return
+ click.echo(title)
+ for path in paths[:limit]:
+ click.echo(f" {path}")
+ if len(paths) > limit:
+ click.echo(f" ... {len(paths) - limit} more")
+
+
+def _report_plan_problems(plan):
+ """
+ List unknown paths and conflicting files that prevent a download.
+
+ Returns:
+ list[str]: Short descriptions of the problems; empty if there are none.
+ """
+ _echo_paths("Existing files that conflict with the catalog:", [
+ f"{conflict.item.relative_path.as_posix()} ({conflict.reason})"
+ for conflict in plan.conflicts])
+ if plan.conflicts:
+ click.echo("Delete conflicting files first to replace them.")
+ _echo_paths("Not in the catalog:", plan.unknown_paths)
+ return [f"{count} {text}" for count, text in (
+ (len(plan.conflicts), "existing file(s) conflict with the catalog"),
+ (len(plan.unknown_paths), "path(s) not in the catalog")) if count]
+
+
@hallmark.command(short_help="Download files from the configured data remote.")
@click.argument("files", nargs=-1)
@click.option("--tsv", "tsv_names", multiple=True,
@@ -406,9 +497,9 @@ def checkout(repo, target_branch):
@click.option("--all", "download_all", is_flag=True,
help="Select all cataloged files.")
@click.option("--filter", "filters", multiple=True,
- help="Select paths matching a glob. ** matches recursively. "
- "May be repeated.")
-@click.option("--fmt", help="Select paths matching a filename format.")
+ help="Keep only selected paths matching a glob. ** matches "
+ "recursively. May be repeated.")
+@click.option("--fmt", help="Keep only selected paths matching a filename format.")
@click.option("--remote", "remote_name",
help="Name of the configured data remote to use.")
@click.option("--output", type=click.Path(file_okay=False),
@@ -423,32 +514,50 @@ def download(repo, files, tsv_names, download_all, filters, fmt, remote_name,
"""
Download selected files from a configured data remote.
+ Choose catalogued FILES or folders, or --all; a folder selects every
+ catalogued file below it. FILES are relative to the current folder, which
+ can be any folder inside the repository. --filter and --fmt only narrow
+ that selection. Files already downloaded with their catalog checksum, or
+ with the catalog size when it records no checksum, are skipped. Paths
+ that are not in the catalog, and existing files that differ from it or
+ cannot be checked, stop the download before the server is contacted;
+ delete conflicting files first to replace them.
+
Preview the selection with --dry-run. Every nonempty download displays
its plan and asks for confirmation before transferring files.
"""
if download_all and (files or tsv_names):
- raise ClickException("--all cannot be combined with file paths or --tsv")
- if not files and not tsv_names and not download_all and not filters and not fmt:
- raise ClickException("Provide file paths, --tsv, --all, --filter, or --fmt")
+ raise click.UsageError("Give paths or --all, not both")
+ if not files and not tsv_names and not download_all:
+ raise click.UsageError(
+ "Choose files with paths or --all; --filter and --fmt only narrow "
+ "that selection")
if repo.worktree is None and output is None:
raise ClickException("--output is required when downloading from a bare repo")
+ files = _repo_relative_paths(repo, files)
with _translate_cli_errors(DownloadError, ValueError):
plan = repo.plan_download(
output, file_paths=files, tsv_names=tsv_names, all_files=download_all,
filter=filters or None, fmt=fmt, remote_name=remote_name)
if dry_run:
click.echo(plan.summary())
- for item in plan.items[:20]:
- click.echo(f" {item.relative_path.as_posix()}")
- if plan.file_count > 20:
- click.echo(f" ... {plan.file_count - 20} more file(s)")
+ _echo_paths("Files to download:", [
+ item.relative_path.as_posix() for item in plan.items])
+ _echo_paths("Already downloaded (skipped):", [
+ item.relative_path.as_posix() for item in plan.skipped])
+ _echo_paths("Present (size matches, not verified):", [
+ item.relative_path.as_posix() for item in plan.unverified])
+ _report_plan_problems(plan)
return
+ problems = _report_plan_problems(plan)
+ if problems:
+ raise ClickException("; ".join(problems) + "; nothing was downloaded")
_run_download(repo, plan, max_workers=max_workers)
@hallmark.command(short_help="Clone an existing Hallmark catalog.")
@click.argument("url")
-@click.argument("path")
+@click.argument("path", required=False)
@click.option("--auth", help="Optional local SSH authentication profile.")
@click.option("--filter", "filters", multiple=True,
help="Select paths matching a glob. ** matches recursively. "
@@ -467,6 +576,8 @@ def clone(url, path, auth, filters, fmt, source_type, no_download, max_workers):
"""
Clone an existing Git catalog or published catalog snapshot at PATH.
+ PATH defaults to a new folder named after the source. It must be new or
+ an empty folder, outside any Hallmark repository and the source.
By default, copy the catalog then ask before downloading dataset files.
Declining keeps the catalog and exits successfully. --no-download skips data
and the prompt; bare destinations require it. --filter and --fmt narrow the
@@ -475,6 +586,11 @@ def clone(url, path, auth, filters, fmt, source_type, no_download, max_workers):
"""
if (filters or fmt is not None) and no_download:
raise ClickException("--filter and --fmt cannot be used with --no-download")
+ if path is None:
+ try:
+ path = str(default_clone_destination(url))
+ except CloneError as exc:
+ raise click.UsageError(str(exc)) from exc
if not no_download and Repo.resolve_repo_paths(path)[1] is None:
raise ClickException("Bare clones require --no-download")
diff --git a/mod/hallmark/remote/add.py b/mod/hallmark/remote/add.py
index 4df2a5a..19417a3 100644
--- a/mod/hallmark/remote/add.py
+++ b/mod/hallmark/remote/add.py
@@ -6,9 +6,13 @@
from .discovery import discover_remote_files, path_matches
from ..utils import as_list_of_dicts
-from ..repo.config import normalize_remotes, validate_tsv_filename
+from ..repo.config import (
+ check_pattern_keeps_catalogue, filename_fields, normalize_remotes,
+ single_data_format, validate_tsv_filename)
from ..transport import OperationContext, RemoteSpec
-from ..transport.base import copy_backend_options
+from ..transport.base import (
+ DownloadError, RemoteConfigurationError, copy_backend_options,
+ reject_url_secrets)
def is_remote_catalog(state):
@@ -19,6 +23,7 @@ def split_remote_pattern(value, fmt=None):
parts = urlsplit(value)
if parts.scheme not in {"https", "http", "ssh", "sftp"}:
raise ValueError("Remote add requires an HTTP(S), SSH or SFTP URL")
+ reject_url_secrets(value)
path = parts.path
if "{" in path:
if fmt is not None:
@@ -31,6 +36,37 @@ def split_remote_pattern(value, fmt=None):
return urlunsplit(parts._replace(path=path)), fmt
+CATALOG_COLUMNS = ("path", "checksum_algorithm", "checksum", "size_bytes", "mtime")
+
+
+def _pattern_fields(fmt, path):
+ """Return the filename fields that a matching pattern extracts from a path."""
+ matched = parse.parse(fmt, path, case_sensitive=True)
+ return {key: value for key, value in matched.named.items()
+ if key not in {*CATALOG_COLUMNS, "sha1"}}
+
+
+def _replace_pattern_fields(frame, fmt, old_fmt):
+ """Swap the old pattern's field columns for those of a new pattern."""
+ fields = pd.DataFrame([_pattern_fields(fmt, path) for path in frame["path"]],
+ index=frame.index)
+ old_fields = set(filename_fields(old_fmt)) if old_fmt else set()
+ replaced = ((old_fields | set(fields.columns))
+ & (set(frame.columns) - set(CATALOG_COLUMNS)))
+ return frame.drop(columns=sorted(replaced)).join(fields)
+
+
+def _describe_selection(fmt, filter):
+ """Describe a remote selection for messages about the files it matched."""
+ parts = []
+ if fmt:
+ parts.append(f"pattern {fmt!r}")
+ if filter:
+ patterns = [filter] if isinstance(filter, str) else list(filter)
+ parts.append("filter " + ", ".join(repr(pattern) for pattern in patterns))
+ return " and ".join(parts) or "any file"
+
+
def add_remote(repo, value, *, fmt=None, filter=None, auth=None, backend=None,
backend_options=None, progress=False):
url, fmt = split_remote_pattern(value, fmt)
@@ -61,25 +97,39 @@ def add_remote(repo, value, *, fmt=None, filter=None, auth=None, backend=None,
or remotes[0].get("url") != source.url):
raise ValueError(
"Remote add currently supports one dataset root per repository")
- with OperationContext(source) as context:
- entries = discover_remote_files(
- context, filter=filter, fmt=fmt, progress=progress
- )
- columns = ["path", "checksum_algorithm", "checksum", "size_bytes", "mtime"]
+ # A branch has one pattern: a bare URL rescans with the saved pattern, and
+ # a new pattern must fit every catalogued file before the server is read.
+ saved_fmt = single_data_format(repo.state.config)
+ if fmt is None:
+ fmt = saved_fmt
+ check_pattern_keeps_catalogue(repo.state, fmt)
+ try:
+ with OperationContext(source) as context:
+ entries = discover_remote_files(
+ context, filter=filter, fmt=fmt, progress=progress
+ )
+ except RemoteConfigurationError:
+ raise
+ except DownloadError as exc:
+ # Nothing is saved until the scan completes, so the catalogue is intact.
+ raise DownloadError(
+ f"Remote scan failed (previous catalogue kept): {exc}") from exc
+ if not entries:
+ raise ValueError(
+ f"No remote files matched {_describe_selection(fmt, filter)} under "
+ f"{url}; previous catalogue kept")
rows = []
for entry in entries:
- row = dict(zip(columns, (entry.path, entry.checksum_algorithm,
- entry.checksum, entry.size, entry.mtime)))
+ row = dict(zip(CATALOG_COLUMNS, (entry.path, entry.checksum_algorithm,
+ entry.checksum, entry.size, entry.mtime)))
if fmt:
- matched = parse.parse(fmt, entry.path, case_sensitive=True)
- row.update({key: value for key, value in matched.named.items()
- if key not in {*columns, "sha1"}})
+ row.update(_pattern_fields(fmt, entry.path))
rows.append(row)
- frame = pd.DataFrame(rows) if rows else pd.DataFrame(columns=columns)
- if frame.empty:
- return frame
+ frame = pd.DataFrame(rows)
state = deepcopy(repo.state)
if not existing.empty:
+ if fmt != saved_fmt:
+ existing = _replace_pattern_fields(existing, fmt, saved_fmt)
frame = pd.concat([existing, frame], ignore_index=True, sort=False)
frame = frame.drop_duplicates(subset=["path"], keep="last")
state.data = frame.fillna("").sort_values("path").reset_index(drop=True)
diff --git a/mod/hallmark/remote/clone.py b/mod/hallmark/remote/clone.py
index 085dcd7..35d741b 100644
--- a/mod/hallmark/remote/clone.py
+++ b/mod/hallmark/remote/clone.py
@@ -6,14 +6,17 @@
from io import StringIO
from pathlib import Path
from shutil import rmtree
-from urllib.parse import urlsplit
+from urllib.parse import unquote, urlsplit
import pandas as pd
import yaml
+from git import Repo as GitRepo
+from git.exc import GitCommandError, GitError
from ..repo.dothm import Dothm
from ..error import CloneError, DestinationExistsError
-from ..utils import as_list_of_dicts
+from ..utils import as_list_of_dicts, load_yaml
+from .download import _checksum_from_row, _validate_expected_checksum
from ..repo.config import (
filename_fields,
normalize_remotes,
@@ -21,8 +24,11 @@
row_to_path,
)
from ..transport import OperationContext, RemoteSpec
-from ..transport.base import RemoteObjectMissing, validate_remote_path
+from ..transport.base import (
+ DownloadError, RemoteObjectMissing, reject_url_credentials,
+ validate_remote_path)
from ..repo.worktree import Worktree
+from ..repo.branches import validate_branch_name
def _catalog_filenames(config):
@@ -63,14 +69,30 @@ def _resolve_catalog_path(row, formats):
return paths[0]
-def _validate_snapshot(files, config):
- """Validate catalog tables and paths before writing the snapshot."""
+def _validate_catalog(files, config):
+ """
+ Validate a catalog's tables before it is written or kept.
+
+ Every row must resolve to one safe relative path, and its checksum, if
+ any, must use a supported algorithm and a well-formed digest.
+
+ Args:
+ files (dict[str, str]): Text of ``data.tsv`` and the other TSVs
+ named by ``config``.
+ config (dict): Parsed ``config.yml``.
+
+ Raises:
+ CloneError: If a table is missing, unreadable or invalid.
+ ValueError: If a path or table name is unsafe.
+ """
for name in _catalog_filenames(config):
+ if name not in files:
+ raise CloneError(f"Catalog table is missing: {name}")
try:
frame = pd.read_csv(StringIO(files[name]), sep="\t", dtype=str,
keep_default_na=False)
except (pd.errors.EmptyDataError, pd.errors.ParserError) as exc:
- raise CloneError(f"Invalid published catalog table: {name}") from exc
+ raise CloneError(f"Invalid catalog table: {name}") from exc
formats = _catalog_formats(config, name)
has_fields = False
for fmt in formats:
@@ -79,9 +101,61 @@ def _validate_snapshot(files, config):
has_fields = True
break
if not ({"path", "sha1"} & set(frame.columns) or has_fields):
- raise CloneError(f"Unrecognized published catalog columns: {name}")
+ raise CloneError(f"Unrecognized catalog columns: {name}")
for _, row in frame.iterrows():
- _resolve_catalog_path(row, formats)
+ path = _resolve_catalog_path(row, formats)
+ try:
+ _validate_expected_checksum(_checksum_from_row(row))
+ except DownloadError as exc:
+ raise CloneError(f"{name}: {path}: {exc}") from exc
+
+
+def _validate_branch(git, branch):
+ """
+ Validate the committed catalog of one branch of a cloned repository.
+
+ Raises:
+ CloneError: If a state file is missing or the catalog is invalid.
+ """
+ def read(name):
+ try:
+ return git.show("--end-of-options", f"{branch}:{name}")
+ except GitCommandError:
+ raise CloneError(
+ f"Branch '{branch}' is missing {name}") from None
+
+ try:
+ config = load_yaml(read("config.yml"))
+ texts = {"meta.yml": read("meta.yml"), "data.tsv": read("data.tsv")}
+ load_yaml(texts["meta.yml"])
+ except (ValueError, yaml.YAMLError) as exc:
+ detail = " ".join(str(exc).split())
+ raise CloneError(
+ f"Branch '{branch}' has an invalid config.yml or meta.yml: "
+ f"{detail}") from exc
+ try:
+ for name in _catalog_filenames(config):
+ texts.setdefault(name, read(name))
+ _validate_catalog(texts, config)
+ except (CloneError, ValueError) as exc:
+ raise CloneError(
+ f"Branch '{branch}' has an invalid catalog: {exc}") from exc
+
+
+def _copy_branch_pointers(dothm):
+ """
+ Create a local branch for every branch copied from the source.
+
+ Returns:
+ list[str]: Names of all local branches.
+ """
+ local = {head.name for head in dothm.heads}
+ for remote in dothm.remotes:
+ for ref in remote.refs:
+ if ref.remote_head != "HEAD" and ref.remote_head not in local:
+ dothm.git.branch("--track", "--", ref.remote_head, ref.name)
+ local.add(ref.remote_head)
+ return sorted(local)
def _read_catalog_snapshot(context):
@@ -118,7 +192,7 @@ def _read_catalog_snapshot(context):
for name in _catalog_filenames(config):
if name not in files:
files[name] = context.read_text(name)
- _validate_snapshot(files, config)
+ _validate_catalog(files, config)
return files
@@ -134,6 +208,204 @@ def _commit_catalog_metadata(repo, files, message):
+def _scp_style(url):
+ """Return whether a Git source uses the SCP-like ``host:path`` form."""
+ return ("://" not in url and not Path(url).expanduser().exists()
+ and bool(re.match(r"^(?:[^/@:]+@)?[^/:]+:.+", url)))
+
+
+def default_clone_destination(source) -> Path:
+ """
+ Name the folder that a clone creates when no destination is given.
+
+ The name is the last segment of the source path or URL without a ``.git``
+ suffix. A ``.hm`` folder is named after its worktree, and ``NAME.hm``
+ after ``NAME``, so the default is never a bare repository.
+
+ Args:
+ source (str): Git URL, local path, or published catalog URL.
+
+ Returns:
+ Path: Relative folder name in the current folder.
+
+ Raises:
+ CloneError: If the source has no usable name.
+ """
+ text = str(source).strip()
+ if "://" in text:
+ path = unquote(urlsplit(text).path)
+ elif _scp_style(text):
+ path = text.split(":", 1)[1]
+ else:
+ path = text
+ segments = [part for part in path.split("/") if part not in ("", ".")]
+ while segments:
+ name = segments.pop()
+ if name == ".hm":
+ continue
+ for suffix in (".git", ".hm"):
+ if name.endswith(suffix):
+ name = name[:-len(suffix)]
+ if name and name != "..":
+ return Path(name)
+ break
+ raise CloneError("Cannot name a folder after this source; give a DIRECTORY")
+
+
+def _local_source_root(url):
+ """Return the folder of a local source repository, or None for remote URLs."""
+ if url.startswith("file://"):
+ local = Path(unquote(urlsplit(url).path))
+ elif "://" in url:
+ return None
+ else:
+ local = Path(url).expanduser()
+ if not local.exists():
+ return None
+ local = local.resolve()
+ return local.parent if local.name == ".hm" else local
+
+
+def _check_destination(cls, url, destination, display_path):
+ """
+ Refuse a destination that is not empty, is nested in a repository, or
+ overlaps a local source. Nothing is created or contacted.
+
+ Returns:
+ bool: True when the destination is an existing empty folder.
+ """
+ if destination.is_symlink() or (destination.exists() and (
+ not destination.is_dir() or any(destination.iterdir()))):
+ raise DestinationExistsError(
+ f"fatal: destination path '{display_path}' already exists and is "
+ "not empty.")
+ target = destination.resolve()
+ source = _local_source_root(url)
+ if source is not None and (target == source or source in target.parents
+ or target in source.parents):
+ raise CloneError(
+ f"Destination '{display_path}' overlaps the source repository "
+ f"'{source}'; choose a separate folder")
+ enclosing = cls.find_root(target.parent)
+ if enclosing is not None:
+ raise CloneError(
+ f"Destination '{display_path}' is inside the Hallmark repository at "
+ f"'{enclosing}'; nested repositories are not supported, so choose "
+ "a folder outside it")
+ return destination.is_dir()
+
+
+def _remove_partial_clone(cls, destination, *, created):
+ """
+ Remove what a failed clone created, never following a symbolic link.
+
+ The folders the clone created for its destination are removed. In a
+ worktree folder that existed only its new ``.hm`` is removed; a bare
+ folder that existed, and held nothing before, is emptied again.
+
+ Args:
+ cls: Repository class, used to tell bare destinations apart.
+ destination (Path): Clone destination.
+ created (Path, optional): Highest folder created for the destination,
+ or None if the destination existed.
+ """
+ if created is not None:
+ if not created.is_symlink():
+ rmtree(created, ignore_errors=True)
+ return
+ if destination.is_symlink():
+ return
+ if cls.resolve_repo_paths(destination)[1] is not None:
+ dothm = destination / ".hm"
+ if dothm.is_symlink():
+ dothm.unlink()
+ else:
+ rmtree(dothm, ignore_errors=True)
+ return
+ destination.mkdir(exist_ok=True)
+ for child in destination.iterdir():
+ if child.is_dir() and not child.is_symlink():
+ rmtree(child, ignore_errors=True)
+ else:
+ child.unlink(missing_ok=True)
+
+
+def _tracks_local_files(table_text):
+ """
+ Return whether a ``data.tsv`` catalog versions local files.
+
+ Local catalogs record a ``sha1`` per file and keep contents in
+ ``.hm/objects``; remote catalogs list ``path`` rows on a data server.
+ Unreadable tables are left to catalog validation.
+ """
+ try:
+ frame = pd.read_csv(StringIO(table_text), sep="\t", dtype=str,
+ keep_default_na=False)
+ except (pd.errors.EmptyDataError, pd.errors.ParserError):
+ return False
+ return (not frame.empty and "sha1" in frame.columns
+ and "path" not in frame.columns)
+
+
+def _refuse_local_file_branches(git, branches):
+ """
+ Refuse a clone when a branch's committed catalog versions local files.
+
+ Args:
+ git: GitPython command wrapper of the repository holding the branches.
+ branches (list[tuple[str, str]]): Branch names and their revisions.
+
+ Raises:
+ CloneError: If any branch tracks local files.
+ """
+ local = []
+ for name, revision in branches:
+ try:
+ table = git.show("--end-of-options", f"{revision}:data.tsv")
+ except GitCommandError:
+ continue
+ if _tracks_local_files(table):
+ local.append(name)
+ if local:
+ names = ", ".join(f"'{name}'" for name in local)
+ raise CloneError(
+ f"Branch {names} tracks local files; cloning local-file repositories "
+ "is not supported yet. Nothing was created.")
+
+
+def _refuse_invalid_branch_names(git, names):
+ """
+ Refuse source branches whose names Git would misread, such as options.
+
+ Raises:
+ CloneError: If a branch name is invalid.
+ """
+ for name in names:
+ try:
+ validate_branch_name(git, name)
+ except ValueError:
+ raise CloneError(
+ f"Source branch {name!r} has an invalid name; cloning it is not "
+ "supported. Nothing was created.") from None
+
+
+def _check_local_source(url):
+ """Inspect a local Git source read-only, before anything is written."""
+ root = _local_source_root(url)
+ if root is None:
+ return
+ git_dir = root / ".hm" if (root / ".hm").is_dir() else root
+ try:
+ source = GitRepo(git_dir)
+ except GitError:
+ # Not a readable repository: cloning reports the problem itself.
+ return
+ with source:
+ names = [head.name for head in source.heads]
+ _refuse_invalid_branch_names(source.git, names)
+ _refuse_local_file_branches(source.git, [(name, name) for name in names])
+
+
def _is_git_source(url, source_type):
"""Determine whether the selected source should be cloned with Git."""
if source_type == "git":
@@ -155,11 +427,20 @@ def _clone_git(cls, url, destination, display_path, auth):
local = Path(url).expanduser()
git_url = str(local / ".hm") if (local / ".hm").is_dir() else url
try:
- Dothm.clone(git_url, dothm_path, display_path=display_path)
+ dothm = Dothm.clone(git_url, dothm_path, display_path=display_path)
except CloneError as exc:
raise CloneError(
f"{exc}\nFor a raw dataset, use hm init PATH, "
"then hm add URL.") from exc
+ # Sources on a Git host can only be inspected once copied; the caller
+ # removes the copy if a check fails.
+ _refuse_invalid_branch_names(dothm.git, [
+ ref.remote_head for remote in dothm.remotes
+ for ref in remote.refs if ref.remote_head != "HEAD"])
+ branches = _copy_branch_pointers(dothm)
+ _refuse_local_file_branches(dothm.git, [(name, name) for name in branches])
+ for name in branches:
+ _validate_branch(dothm.git, name)
if worktree_path:
Worktree.init(worktree_path)
return cls(destination)
@@ -169,9 +450,13 @@ def clone_catalog(cls, url, path, *, auth=None, source_type="auto"):
"""
Clone an existing Git catalog or published HTTP/SFTP catalog snapshot.
- Existing destinations are rejected before source access. An incomplete
- destination created by this call is removed on failure. Dataset discovery
- belongs to ``Repo.add(URL)``.
+ Source URLs with credentials are rejected first. Repositories with a
+ branch that tracks local files cannot be cloned yet. The destination must
+ be new or an empty folder, outside any Hallmark repository and the local
+ source; this is checked before source access. On failure, only what this
+ call created is removed: the destination it created, or the ``.hm`` it
+ added to an existing worktree folder. An existing bare folder is emptied
+ again. Dataset discovery belongs to ``Repo.add(URL)``.
Args:
cls: Repository class used to initialize or open the result.
@@ -184,26 +469,38 @@ def clone_catalog(cls, url, path, *, auth=None, source_type="auto"):
Repo: Repository containing complete catalog metadata without payloads.
Raises:
- DestinationExistsError: If the destination already exists.
- CloneError: If a requested catalog is missing or invalid.
+ DestinationExistsError: If the destination exists and is not an empty
+ folder.
+ CloneError: If the destination is inside a repository or overlaps a
+ local source, a branch tracks local files, or a requested catalog
+ is missing or invalid.
ValueError: If source options are invalid.
DownloadError: If remote metadata cannot be read or validated.
"""
if source_type not in {"auto", "git", "catalog"}:
raise ValueError("source_type must be auto, git, or catalog")
+ # Git records the source URL in .hm/.git/config, so it must hold no secret.
+ reject_url_credentials(str(url))
url = str(url)
destination = Path(path).expanduser().absolute()
- if destination.exists() or destination.is_symlink():
- raise DestinationExistsError(
- f"fatal: destination path '{path}' already exists and is not empty.")
+ existed = _check_destination(cls, url, destination, path)
is_git = _is_git_source(url, source_type)
if is_git and auth is not None:
raise ValueError("Git cloning uses Git/SSH authentication, not auth profiles")
- destination.parent.mkdir(parents=True, exist_ok=True)
- try:
- destination.mkdir()
- except FileExistsError as exc:
- raise DestinationExistsError(f"Destination already exists: {path}") from exc
+ if is_git:
+ _check_local_source(url)
+ created = None
+ if not existed:
+ # Remember the highest missing folder so a failure removes them all.
+ created = destination
+ while not (created.parent.exists() or created.parent.is_symlink()):
+ created = created.parent
+ destination.parent.mkdir(parents=True, exist_ok=True)
+ try:
+ destination.mkdir()
+ except FileExistsError as exc:
+ raise DestinationExistsError(
+ f"Destination already exists: {path}") from exc
try:
if is_git:
return _clone_git(cls, url, destination, path, auth)
@@ -220,11 +517,18 @@ def clone_catalog(cls, url, path, *, auth=None, source_type="auto"):
raise CloneError("No published Hallmark catalog at this URL; "
"use hm init PATH, then hm add URL "
"for a raw dataset")
+ if _tracks_local_files(snapshot["data.tsv"]):
+ raise CloneError(
+ "The published catalog tracks local files; cloning local-file "
+ "repositories is not supported yet. Nothing was created.")
+ if cls.resolve_repo_paths(destination)[1] is None:
+ # Initialization creates a bare ".hm" folder itself.
+ destination.rmdir()
repo = cls.init(destination)
for name, contents in snapshot.items():
(repo.dothm.path / name).write_text(contents, encoding="utf-8")
_commit_catalog_metadata(repo, snapshot, "Import published catalog snapshot")
return repo
except BaseException:
- rmtree(destination, ignore_errors=True)
+ _remove_partial_clone(cls, destination, created=created)
raise
diff --git a/mod/hallmark/remote/download.py b/mod/hallmark/remote/download.py
index 888424b..632b571 100644
--- a/mod/hallmark/remote/download.py
+++ b/mod/hallmark/remote/download.py
@@ -2,6 +2,7 @@
from __future__ import annotations
+import os
from concurrent.futures import FIRST_COMPLETED, ThreadPoolExecutor, wait
from decimal import Decimal, InvalidOperation
from pathlib import Path
@@ -13,9 +14,10 @@
from ..transport import OperationContext, RemoteSpec
from ..transport.base import DownloadError, validate_remote_path
-from .plan import DownloadItem, DownloadPlan
+from .plan import DownloadConflict, DownloadItem, DownloadPlan
from ..utils import (
CHECKSUM_ALGORITHMS_BY_STRENGTH,
+ CHECKSUM_LENGTHS,
SUPPORTED_CHECKSUM_ALGORITHMS,
as_list_of_dicts,
replace_file_on_success,
@@ -34,6 +36,9 @@
ChecksumSpec = Union[str, tuple[str, str]]
# The size of chunks to read from a file when downloading or computing checksums.
DOWNLOAD_CHUNK_SIZE = 8192
+# Supported checksum algorithms by the length of their hexadecimal digests.
+ALGORITHMS_BY_DIGEST_LENGTH = {
+ length: algorithm for algorithm, length in CHECKSUM_LENGTHS.items()}
def _repository_config(repo) -> dict:
"""
@@ -152,7 +157,13 @@ def _parse_checksum(value, algorithm=None) -> Optional[ChecksumSpec]:
algorithm_text = _clean_checksum(algorithm)
# If the algorithm is None, return None to indicate no valid algorithm is available
if algorithm_text is None:
- return None
+ if str(algorithm).strip().lower() != "unknown":
+ return None
+ # Manifests such as checksums.txt do not name their algorithm; the
+ # length of a well-formed digest identifies the supported one it is.
+ algorithm_text = ALGORITHMS_BY_DIGEST_LENGTH.get(len(checksum))
+ if algorithm_text is None or not is_valid_checksum(algorithm_text, checksum):
+ return None
# Normalize the algorithm to lowercase and check if it is supported
algorithm_text = algorithm_text.lower()
if algorithm_text not in SUPPORTED_CHECKSUM_ALGORITHMS:
@@ -504,7 +515,7 @@ def _download_and_verify_file(
context, relative_path, destination, expected_checksum, chunk_size
):
"""
- Download and verify a file before replacing its destination.
+ Download and verify a file, then move it to its absent destination.
Args:
context (OperationContext): Transport and destination for this download.
@@ -518,7 +529,8 @@ def _download_and_verify_file(
int: Number of downloaded bytes.
Raises:
- DownloadError: If the transfer, checksum, or destination is invalid.
+ DownloadError: If the transfer, checksum, or destination is invalid,
+ or the destination exists.
"""
validated_checksum = _validate_expected_checksum(expected_checksum)
try:
@@ -535,12 +547,43 @@ def _download_and_verify_file(
# Another process can still change the path after this check.
resolve_path_in_root(context.output_root, relative_path,
label="download destination")
+ # Planning skips or refuses existing files, so a file here appeared
+ # later; never replace it. The same race as above remains.
+ if destination.exists() or destination.is_symlink():
+ raise DownloadError(
+ f"Refusing to replace {destination.name}: it appeared after "
+ "the download was planned; delete it first to replace it")
context.check_cancelled()
return size
except (OSError, ValueError) as exc:
raise DownloadError(f"Failed to write {destination.name}: {exc}") from None
+def _normalize_requests(file_paths: Sequence[str]) -> tuple[str, ...]:
+ """
+ Used by _select_download_items and plan_download.
+ Normalize requested catalog paths and folders, keeping their order.
+
+ A trailing "/" is optional for folders, and "." requests the whole catalog.
+
+ Returns:
+ tuple[str, ...]: Unique POSIX paths relative to the catalog root.
+ """
+ requests = []
+ for value in file_paths:
+ text = str(value)
+ request = "." if text.rstrip("/") == "." else validate_remote_path(
+ text).as_posix()
+ if request not in requests:
+ requests.append(request)
+ return tuple(requests)
+
+
+def _is_requested(path: str, request: str) -> bool:
+ """Return whether a catalog path is a requested file or lies below a folder."""
+ return request == "." or path == request or path.startswith(request + "/")
+
+
def _select_download_items(
repo,
file_paths: Sequence[str] = (),
@@ -553,7 +596,9 @@ def _select_download_items(
Args:
repo: The hallmark repository object.
- file_paths: A sequence of specific file paths to download.
+ file_paths: Catalogued files or folders to download. A folder selects
+ every catalogued file below it; paths outside the catalog select
+ nothing.
tsv_names: A sequence of TSV names to download files from.
all_files: If True, include all files from the repository's configuration.
@@ -568,7 +613,10 @@ def _select_download_items(
# dictionary to store selected files with relative paths and optional checksums.
selected: dict[str, tuple[Path, Optional[ChecksumSpec]]] = {}
metadata = {}
- explicit_paths = {validate_remote_path(path).as_posix() for path in file_paths}
+ explicit_paths = _normalize_requests(file_paths)
+
+ def requested(path: str) -> bool:
+ return any(_is_requested(path, request) for request in explicit_paths)
def add_file(
value: Union[str, Path],
@@ -609,12 +657,9 @@ def add_frame(frame: pd.DataFrame, fmt_entries: list[dict],
row = dict(zip(columns, values))
# add the resolved remote path and its checksum to the selected files
relative_path = validate_remote_path(_resolve_remote_path(row, fmt_entries))
- if not explicit_only or relative_path.as_posix() in explicit_paths:
+ if not explicit_only or requested(relative_path.as_posix()):
add_file(relative_path, _checksum_from_row(row), row)
- # Add explicitly requested file paths to the selected files.
- for file_path in file_paths:
- add_file(file_path)
# Organize data configuration entries by their TSV names for easier access.
entries_by_tsv: dict[str, list[dict]] = {}
for entry in data_config:
@@ -691,7 +736,7 @@ def add_frame(frame: pd.DataFrame, fmt_entries: list[dict],
# if a file path is specified in the entry, add it to the selected files
if file_path and (
all_files
- or validate_remote_path(file_path).as_posix() in explicit_paths
+ or requested(validate_remote_path(file_path).as_posix())
):
add_file(file_path, _checksum_from_config(entry), entry)
@@ -711,6 +756,73 @@ def add_frame(frame: pd.DataFrame, fmt_entries: list[dict],
for path, checksum in selected.values()]
+def _existing_file_status(item: DownloadItem, path: Path):
+ """
+ Used by _sort_existing_destinations.
+ Compare an existing destination with its catalog size and checksum.
+
+ The size is compared first, so a file of the wrong size is never read.
+
+ Returns:
+ tuple[str, str | None]: ``("verified", None)`` when the checksum
+ matches; ``("unverified", None)`` when the catalog has no usable
+ checksum but records the same size; otherwise ``("conflict", reason)``.
+ """
+ if path.is_symlink() or not path.is_file():
+ return "conflict", "not a regular file"
+ try:
+ size = path.stat().st_size
+ except OSError:
+ return "conflict", "cannot be read"
+ if item.size_bytes is not None and size != item.size_bytes:
+ return "conflict", (f"{size:,} bytes, but the catalog records "
+ f"{item.size_bytes:,}")
+ try:
+ expected = _validate_expected_checksum(item.checksum)
+ except DownloadError:
+ expected = None
+ if expected is None:
+ if item.size_bytes is None:
+ return "conflict", "no catalog checksum or size to verify it"
+ return "unverified", None
+ algorithm, digest = expected
+ try:
+ actual = calculate_file_checksum(path, algorithm=algorithm)
+ except OSError:
+ return "conflict", "cannot be read"
+ if actual.lower() != digest:
+ return "conflict", "different contents"
+ return "verified", None
+
+
+def _sort_existing_destinations(items, destinations):
+ """
+ Used by plan_download.
+ Separate files to transfer from those whose destinations already exist.
+
+ Existing files are checked in parallel with their catalog size and
+ checksum algorithm, using a small pool because hashing reads whole files.
+
+ Returns:
+ tuple: Items to transfer, items already downloaded and verified,
+ items present with the catalog size but no checksum, and conflicts.
+ """
+ existing = [(item, path) for item, path in zip(items, destinations)
+ if path.exists() or path.is_symlink()]
+ if not existing:
+ return list(items), [], [], []
+ with ThreadPoolExecutor(max_workers=min(8, os.cpu_count() or 1)) as executor:
+ statuses = list(executor.map(
+ lambda pair: _existing_file_status(*pair), existing))
+ present = {item.relative_path for item, _ in existing}
+ by_status = {"verified": [], "unverified": [], "conflict": []}
+ for (item, _), (status, reason) in zip(existing, statuses):
+ by_status[status].append(
+ DownloadConflict(item, reason) if status == "conflict" else item)
+ return ([item for item in items if item.relative_path not in present],
+ by_status["verified"], by_status["unverified"], by_status["conflict"])
+
+
def _parse_file_size(value) -> Optional[int]:
"""Read a recorded size without treating missing or malformed sizes as zero."""
if value is None or isinstance(value, bool) or pd.isna(value):
@@ -741,12 +853,21 @@ def plan_download(
No network requests are made. With no explicit paths or TSVs, select
the complete catalog before applying filters. Missing sizes remain unknown.
+ Requested paths that match no catalogued file are recorded in
+ ``unknown_paths``. Existing destinations are compared with the catalog:
+ files with their catalog checksum are ``skipped``; files without a usable
+ catalog checksum but with the recorded size are ``unverified`` and also
+ not transferred; files that differ, cannot be checked at all or are not
+ regular files are ``conflicts``. A plan with unknown paths or conflicts
+ cannot be executed.
Args:
repo: The hallmark repository object.
output_path (Path | str, optional): Destination directory. Defaults
to the worktree; required for a bare repository.
- file_paths (sequence[str], optional): Remote-relative file paths.
+ file_paths (sequence[str], optional): Catalogued files or folders,
+ relative to the catalog root. A folder selects every catalogued
+ file below it, and "." selects the whole catalog.
tsv_names (sequence[str], optional): Catalog TSVs to select.
all_files (bool): Select all configured files. Cannot be combined
with explicit paths or TSVs. Defaults to False.
@@ -782,6 +903,11 @@ def plan_download(
items = _select_download_items(
repo, file_paths=file_paths, tsv_names=tsv_names,
all_files=all_files or (not file_paths and not tsv_names))
+ # A path is unknown when no catalogued file is, or lies below, that path.
+ selected_paths = [item.relative_path.as_posix() for item in items]
+ unknown_paths = tuple(
+ request for request in _normalize_requests(file_paths)
+ if not any(_is_requested(path, request) for path in selected_paths))
if filter is not None or fmt is not None:
from .discovery import path_matches
items = [item for item in items if path_matches(
@@ -795,18 +921,23 @@ def plan_download(
source = RemoteSpec.from_url(
remote["url"], remote.get("auth"), backend=remote.get("backend"),
backend_options=remote.get("backend_options"))
+ destinations = []
for item in items:
try:
- resolve_path_in_root(output_root, item.relative_path,
- label="download destination")
+ destinations.append(resolve_path_in_root(
+ output_root, item.relative_path, label="download destination"))
except ValueError as exc:
raise DownloadError(str(exc)) from exc
+ items, skipped, unverified, conflicts = _sort_existing_destinations(
+ items, destinations)
return DownloadPlan(
tuple(items), remote.get("url"), output_root,
remote_auth=remote.get("auth"), remote_name=remote.get("name"),
estimated_bytes_per_second=estimated_bytes_per_second,
remote_backend=source.backend if source is not None else None,
- backend_options=remote.get("backend_options"))
+ backend_options=remote.get("backend_options"),
+ unknown_paths=unknown_paths, skipped=tuple(skipped),
+ unverified=tuple(unverified), conflicts=tuple(conflicts))
def execute_download_plan(
@@ -834,11 +965,22 @@ def execute_download_plan(
Raises:
TypeError: If ``plan`` is not a DownloadPlan.
- DownloadError: If approval is missing, setup fails, or the
- destination is invalid.
+ DownloadError: If the plan has unknown paths or conflicts, approval
+ is missing, setup fails, or the destination is invalid.
"""
if not isinstance(plan, DownloadPlan):
raise TypeError("plan must be a DownloadPlan")
+ if plan.unknown_paths:
+ raise DownloadError(
+ "Not in the catalog: " + ", ".join(plan.unknown_paths)
+ + "; nothing was downloaded")
+ if plan.conflicts:
+ raise DownloadError(
+ "Existing files conflict with the catalog: " + ", ".join(
+ f"{conflict.item.relative_path.as_posix()} ({conflict.reason})"
+ for conflict in plan.conflicts)
+ + "; nothing was downloaded. "
+ "Delete conflicting files first to replace them")
if plan.items and approved is not True:
raise DownloadError("Dataset downloads require explicit approval")
max_workers = _require_positive_integer(max_workers, label="max_workers")
diff --git a/mod/hallmark/remote/plan.py b/mod/hallmark/remote/plan.py
index fbf7854..c762e64 100644
--- a/mod/hallmark/remote/plan.py
+++ b/mod/hallmark/remote/plan.py
@@ -50,6 +50,21 @@ def __post_init__(self) -> None:
raise ValueError("mtime must be a string or None")
+@dataclass(frozen=True)
+class DownloadConflict:
+ """
+ A selected file whose destination exists and cannot be kept as it is.
+
+ Attributes:
+ item (DownloadItem): The catalogued file.
+ reason (str): Why the existing destination conflicts, for example
+ ``"different contents"``.
+ """
+
+ item: DownloadItem
+ reason: str
+
+
@dataclass(frozen=True)
class DownloadPlan:
"""
@@ -57,9 +72,12 @@ class DownloadPlan:
Creating a plan does not approve a transfer. Sizes come from the catalog;
a duration estimate requires every file size and a supplied transfer rate.
+ Existing destinations are never replaced: files already downloaded are
+ skipped, and any conflict stops the whole download.
Attributes:
- items (tuple[DownloadItem]): Selected files and their metadata.
+ items (tuple[DownloadItem]): Selected files to transfer, with their
+ metadata.
remote_url (str, optional): Data source URL; may be None for an empty plan.
output_path (Path): Absolute destination directory.
remote_auth (str, optional): Local SSH profile name.
@@ -68,6 +86,16 @@ class DownloadPlan:
by the caller for duration estimates.
remote_backend (str, optional): Registered backend selected for transfer.
backend_options (mapping): Immutable backend configuration snapshot.
+ unknown_paths (tuple[str]): Requested paths that match no catalogued
+ file. A plan with unknown paths cannot be executed.
+ skipped (tuple[DownloadItem]): Selected files already present with
+ their catalog checksum; they are not transferred.
+ unverified (tuple[DownloadItem]): Selected files already present with
+ their catalog size but without a catalog checksum to verify them;
+ they are not transferred.
+ conflicts (tuple[DownloadConflict]): Selected files whose existing
+ destinations differ from the catalog or cannot be verified. A plan
+ with conflicts cannot be executed.
"""
items: tuple[DownloadItem, ...]
@@ -78,10 +106,25 @@ class DownloadPlan:
estimated_bytes_per_second: Optional[float] = None
remote_backend: Optional[str] = None
backend_options: Mapping = field(default_factory=dict, repr=False, hash=False)
+ unknown_paths: tuple[str, ...] = ()
+ skipped: tuple[DownloadItem, ...] = ()
+ conflicts: tuple[DownloadConflict, ...] = ()
+ unverified: tuple[DownloadItem, ...] = ()
def __post_init__(self) -> None:
"""Copy the selection and validate the source fields and rate."""
- object.__setattr__(self, "items", tuple(self.items))
+ for name in ("items", "unknown_paths", "skipped", "conflicts",
+ "unverified"):
+ object.__setattr__(self, name, tuple(getattr(self, name)))
+ if any(not isinstance(path, str) for path in self.unknown_paths):
+ raise TypeError("unknown_paths must contain strings")
+ for name in ("skipped", "unverified"):
+ if any(not isinstance(item, DownloadItem)
+ for item in getattr(self, name)):
+ raise TypeError(f"{name} must contain DownloadItem values")
+ if any(not isinstance(conflict, DownloadConflict)
+ for conflict in self.conflicts):
+ raise TypeError("conflicts must contain DownloadConflict values")
object.__setattr__(
self, "output_path", Path(self.output_path).expanduser().absolute())
if any(not isinstance(item, DownloadItem) for item in self.items):
@@ -133,7 +176,9 @@ def summary(self) -> str:
Describe the planned transfer, including unavailable estimates.
Returns:
- str: File count, size and duration estimates, source, and destination.
+ str: File count, size and duration estimates, source, destination,
+ and the numbers of skipped and unverified files, conflicts and
+ unknown paths.
URL credentials, query parameters, and fragments are omitted.
"""
size = f"{self.known_bytes:,} bytes"
@@ -147,6 +192,13 @@ def summary(self) -> str:
source = urlunsplit(parsed._replace(
netloc=parsed.netloc.rsplit("@", 1)[-1], query="", fragment=""))
backend = f"\nBackend: {self.remote_backend}" if self.remote_backend else ""
+ notes = "".join(f"\n{count} {text}" for count, text in (
+ (len(self.skipped), "file(s) already downloaded; skipped"),
+ (len(self.unverified),
+ "file(s) present (size matches, not verified); skipped"),
+ (len(self.conflicts), "existing file(s) conflict with the catalog"),
+ (len(self.unknown_paths), "path(s) not in the catalog"),
+ ) if count)
return (f"{self.file_count} file(s); {size}; estimated duration: {duration}"
f"\nSource: {source}{backend}"
- f"\nDestination: {self.output_path}")
+ f"\nDestination: {self.output_path}{notes}")
diff --git a/mod/hallmark/repo/__init__.py b/mod/hallmark/repo/__init__.py
index b2fdacf..d38cefc 100644
--- a/mod/hallmark/repo/__init__.py
+++ b/mod/hallmark/repo/__init__.py
@@ -15,13 +15,14 @@
from __future__ import annotations
+import shlex
from concurrent.futures import ThreadPoolExecutor
from dataclasses import dataclass
from pathlib import Path
from typing import Dict, List, Optional, Tuple, Union
-from git.exc import GitCommandError
+from git.exc import InvalidGitRepositoryError, NoSuchPathError
-from .branches import checkout, add_worktree
+from .branches import checkout, add_worktree, validate_branch_name
from ..remote.add import add_remote, is_remote_catalog
from .dothm import Dothm
from .state import State
@@ -31,7 +32,7 @@
from .manifest import build_file_table, file_versions_by_path, iter_manifest_entries
from .history import (
load_head_state)
-from ..error import DestinationExistsError
+from ..error import DestinationExistsError, DothmError
from ..utils import (
FILE_IO_CHUNK_SIZE,
use_working_directory,
@@ -80,6 +81,56 @@ def resolve_repo_paths(path: Union[Path, str]) -> Tuple[Path, Optional[Path]]:
return path, None
return path / ".hm", path
+ @staticmethod
+ def find_root(start: Union[Path, str] = ".") -> Optional[Path]:
+ """
+ Locate the repository containing a folder without opening it.
+
+ Args:
+ start (Path | str): Folder to search from. Parent folders are
+ searched in turn.
+
+ Returns:
+ Path | None: The worktree, or the bare ``.hm`` repository, of the
+ nearest enclosing repository; None if there is none.
+ """
+ start = Path(start).expanduser().resolve()
+ for folder in (start, *start.parents):
+ # A bare repository is itself the ".hm" folder, as in Repo(path).
+ if folder.name == ".hm" or (
+ folder.suffix == ".hm" and (folder / ".git").exists()):
+ return folder
+ # Stop at a damaged ".hm" too, so opening it can explain the damage.
+ if (folder / ".hm").exists() or (folder / ".hm").is_symlink():
+ return folder
+ return None
+
+ @classmethod
+ def find(cls, start: Union[Path, str] = ".") -> "Repo":
+ """
+ Open the repository containing a folder, searching parent folders.
+
+ ``Repo(path)`` opens exactly ``path``; ``find`` lets commands run from
+ any folder inside a worktree.
+
+ Args:
+ start (Path | str): Folder to search from. Defaults to the current
+ folder.
+
+ Returns:
+ Repo: The nearest enclosing repository.
+
+ Raises:
+ DothmError: If neither the folder nor any parent contains ``.hm``.
+ """
+ root = cls.find_root(start)
+ if root is None:
+ raise DothmError(
+ f'Not a Hallmark repository (or any parent folder): '
+ f'"{Path(start).expanduser().resolve()}"; '
+ "run hm init or hm clone")
+ return cls(root)
+
def __init__(self, path: Union[Path, str]) -> None:
'''
Open an existing hallmark repository.
@@ -90,18 +141,29 @@ def __init__(self, path: Union[Path, str]) -> None:
none.
'''
dothm_path, worktree_path = self.resolve_repo_paths(path)
- self.dothm = Dothm(dothm_path)
+ # Opening only reads: a damaged repository is explained, not repaired.
+ try:
+ self.dothm = Dothm(dothm_path)
+ except InvalidGitRepositoryError as exc:
+ raise DothmError(
+ f'Repository at "{dothm_path}" is damaged: it is not a Git '
+ "worktree; nothing was changed") from exc
+ except NoSuchPathError as exc:
+ # A missing .hm keeps the GitPython error; a dangling link is damage.
+ if not Path(dothm_path).is_symlink():
+ raise
+ raise DothmError(
+ f'Repository at "{dothm_path}" is damaged: it links to a folder '
+ "that does not exist; nothing was changed") from exc
self.worktree = worktree_path and Worktree(worktree_path)
- self.state = self.dothm.load_state()
+ try:
+ self.state = self.dothm.load_state()
+ except DothmError as exc:
+ raise DothmError(f"{exc}; nothing was changed") from exc
self.download_result = None
- common = Path(self.dothm.common_dir).resolve().parent
- self.objects = Objects(common)
- dothm_objects = Path(dothm_path) / "objects"
- main_objects = common / "objects"
- if dothm_objects.resolve() != main_objects.resolve() \
- and not dothm_objects.exists():
- dothm_objects.symlink_to(main_objects)
+ # Linked worktrees share the object store of the main ".hm".
+ self.objects = Objects(Path(self.dothm.common_dir).resolve().parent)
def _resolve_worktree_path(self, value, *, label: str = "tracked path") -> Path:
"""
@@ -135,21 +197,7 @@ def _validate_branch_name(self, value) -> str:
Raises:
ValueError: If the branch name is invalid.
"""
- # Normalize the branch name to ensure it is a non-empty string
- branch_name = require_nonempty_string(value, label="branch name")
- # if the branch name starts with a hyphen, raise a ValueError
- if branch_name.startswith("-"):
- raise ValueError(f"invalid branch name: {branch_name!r}")
-
- # try to validate the branch name using Git's check_ref_format command
- try:
- self.dothm.git.check_ref_format("--branch", branch_name)
- # if Git raises a GitCommandError, re-raise it as a ValueError
- except GitCommandError as exc:
- raise ValueError(f"invalid branch name: {branch_name!r}") from exc
-
- # if all checks pass, return the normalized branch name
- return branch_name
+ return validate_branch_name(self.dothm.git, value)
def _calculate_file_checksums(self, pf: ParaFrame) -> None:
"""
@@ -206,27 +254,43 @@ def init(cls, path: Union[Path, str] = ".") -> "Repo":
def _download_cloned_files(self, *, approve, max_workers, progress,
filter=None, fmt=None):
- """Run an approved transfer after catalog creation has completed."""
+ """
+ Run an approved transfer after catalog creation has completed.
+
+ Raises:
+ DownloadError: If the download cannot run or a file fails. The
+ message reports the clone as incomplete and how to retry.
+ """
from ..remote.download import DownloadError
- plan = self.plan_download(filter=filter, fmt=fmt)
- if plan.file_count and approve is not None and not approve(plan):
- return
- self.download_result = self.download(
- plan, approved=True, max_workers=max_workers, progress=progress)
+ patterns = [filter] if isinstance(filter, str) else list(filter or [])
+ command = ["hm download --all",
+ *(f"--filter {shlex.quote(pattern)}" for pattern in patterns)]
+ if fmt is not None:
+ command.append(f"--fmt {shlex.quote(fmt)}")
+ retry = f'run {" ".join(command)} in "{self.worktree}" to retry'
+ incomplete = f'Clone incomplete: catalogue at "{self.dothm.path}"'
+ try:
+ plan = self.plan_download(filter=filter, fmt=fmt)
+ if plan.file_count and approve is not None and not approve(plan):
+ return
+ self.download_result = self.download(
+ plan, approved=True, max_workers=max_workers, progress=progress)
+ except DownloadError as exc:
+ raise DownloadError(
+ f"{incomplete}; the download failed: {exc}; {retry}") from exc
if self.download_result["failed"]:
details = "\n".join(self.download_result["errors"][:5])
raise DownloadError(
- f"Failed to download {self.download_result['failed']} "
- f"file(s):\n{details}\n"
- f'Catalog kept at "{self.dothm.path}". '
- "Successfully downloaded files were kept.")
+ f"{incomplete}; {self.download_result['failed']} file(s) "
+ f"failed:\n{details}\n"
+ f"Successfully downloaded files were kept; {retry}.")
@classmethod
def clone(
cls,
url: str,
- path: Union[Path, str],
+ path: Optional[Union[Path, str]] = None,
*,
auth: Optional[str] = None,
filter=None,
@@ -247,7 +311,10 @@ def clone(
Args:
url (str): Existing Git catalog or published snapshot location.
- path (Path | str): New worktree or bare ``.hm`` repository path.
+ path (Path | str, optional): New or empty worktree folder, or bare
+ ``.hm`` repository path, outside any repository and the local
+ source. Defaults to a folder in the current folder named after
+ the source.
auth (str, optional): Local profile for snapshot metadata access.
Git sources use Git's authentication configuration instead.
filter (str | list[str], optional): Download selection globs.
@@ -270,16 +337,21 @@ def clone(
when a download was attempted, including an empty selection.
Raises:
- DestinationExistsError: If the destination already exists.
- CloneError: If a requested catalog is missing or invalid.
+ DestinationExistsError: If the destination exists and is not an
+ empty folder.
+ CloneError: If the destination is nested in a repository or
+ overlaps the source, or a requested catalog is missing or
+ invalid.
ValueError: If source options are invalid or conflict.
DownloadError: If metadata access or downloading fails, or a download
is requested for a bare destination.
"""
- from ..remote.clone import clone_catalog
+ from ..remote.clone import clone_catalog, default_clone_destination
from ..remote.discovery import path_matches
from ..remote.download import DownloadError, _require_positive_integer
+ if path is None:
+ path = default_clone_destination(url)
_require_positive_integer(max_workers, label="max_workers")
if (filter is not None or fmt is not None) and not download:
raise ValueError("clone filter and fmt require download=True; "
diff --git a/mod/hallmark/repo/branches.py b/mod/hallmark/repo/branches.py
index b94c728..1d041ab 100644
--- a/mod/hallmark/repo/branches.py
+++ b/mod/hallmark/repo/branches.py
@@ -5,7 +5,7 @@
from git.exc import GitCommandError
from ..error import CheckoutError, DestinationExistsError, DothmError
-from ..utils import resolve_path_in_root
+from ..utils import require_nonempty_string, resolve_path_in_root
from ..remote.add import is_remote_catalog
from .config import branch_filename_format, row_to_path, single_data_format
from .manifest import iter_manifest_entries
@@ -15,6 +15,39 @@
from .changes import ensure_clean_tracked_files, tracked_paths
+def validate_branch_name(git, value) -> str:
+ """
+ Validate and normalize a Git branch name.
+
+ Names that Git would read as options, such as ``-q``, are refused.
+
+ Args:
+ git: GitPython command wrapper used to run ``check-ref-format``.
+ value (str): The branch name to validate.
+
+ Returns:
+ str: The normalized branch name.
+
+ Raises:
+ ValueError: If the branch name is invalid.
+ """
+ # Normalize the branch name to ensure it is a non-empty string
+ branch_name = require_nonempty_string(value, label="branch name")
+ # if the branch name starts with a hyphen, raise a ValueError
+ if branch_name.startswith("-"):
+ raise ValueError(f"invalid branch name: {branch_name!r}")
+
+ # try to validate the branch name using Git's check_ref_format command
+ try:
+ git.check_ref_format("--branch", branch_name)
+ # if Git raises a GitCommandError, re-raise it as a ValueError
+ except GitCommandError as exc:
+ raise ValueError(f"invalid branch name: {branch_name!r}") from exc
+
+ # if all checks pass, return the normalized branch name
+ return branch_name
+
+
def checkout(repo, target_branch):
# Validate and normalize the target branch name
target_branch = repo._validate_branch_name(target_branch)
@@ -366,6 +399,9 @@ def add_worktree(repo, target_branch):
# iterate over the target state data and restore files from the object store
try:
+ # Point the new ".hm" at the shared object store for tools that read
+ # objects through a repository path, such as a clone of this worktree.
+ (target_dothm / "objects").symlink_to(repo.objects.root)
for _, row in target_state.data.iterrows():
rel_path = row_to_path(row, target_fmt)
# resolve relative path to an absolute path in the target worktree
diff --git a/mod/hallmark/repo/config.py b/mod/hallmark/repo/config.py
index 31694bb..fcf134e 100644
--- a/mod/hallmark/repo/config.py
+++ b/mod/hallmark/repo/config.py
@@ -12,6 +12,8 @@
from string import Formatter
from typing import Dict, Optional
+import parse
+
from ..utils import (
convert_format_value, require_nonempty_string,
validate_path_name, validate_relative_path)
@@ -21,6 +23,7 @@
validate_backend_name,
validate_profile_name,
reject_control_characters,
+ reject_url_secrets,
copy_backend_options,
)
@@ -399,6 +402,7 @@ def set_config(
if isinstance(remote_url, str):
reject_control_characters(remote_url, "Remote URL")
remote_url = require_nonempty_string(remote_url, label="remote_url")
+ reject_url_secrets(remote_url)
if remote_auth not in (None, ""):
validate_profile_name(remote_auth)
@@ -484,6 +488,52 @@ def filename_fields(fmt: str) -> list[str]:
return fields
+def _catalogued_paths(state) -> list[str]:
+ """
+ Used by check_pattern_keeps_catalogue.
+ Return the relative path of every file in a state's catalog.
+ """
+ if "path" in state.data.columns:
+ return [str(path) for path in state.data["path"]]
+ fmt = single_data_format(state.config)
+ if fmt is None:
+ return []
+ return [row_to_path(row, fmt).as_posix()
+ for row in state.data.to_dict(orient="records")]
+
+
+def check_pattern_keeps_catalogue(state, new_fmt: Optional[str]) -> None:
+ """
+ Refuse a filename pattern that files in the catalog do not fit.
+
+ A branch has one pattern, so replacing it must not drop catalogued files.
+ Only local state is read.
+
+ Args:
+ state (State): Staged repository state.
+ new_fmt (str, optional): Proposed filename pattern; None keeps none.
+
+ Raises:
+ ValueError: If any catalogued path does not match ``new_fmt``. The
+ message lists a few of them; there is no force option.
+ """
+ if new_fmt is None or state.data.empty:
+ return
+ parser = parse.compile(new_fmt, case_sensitive=True)
+ misfits = [path for path in _catalogued_paths(state)
+ if parser.parse(path) is None]
+ if not misfits:
+ return
+ shown = ", ".join(misfits[:3])
+ if len(misfits) > 3:
+ shown += f" and {len(misfits) - 3} more"
+ raise ValueError(
+ f"Pattern {new_fmt!r} does not fit {len(misfits)} catalogued file(s) "
+ f"({shown}). A branch has one pattern and a new one cannot drop "
+ "catalogued files; use a pattern that fits them too (there is no "
+ "force option)")
+
+
def validate_tsv_filename(value) -> str:
"""
Validate a TSV database name and add the .tsv suffix when necessary.
diff --git a/mod/hallmark/repo/dothm.py b/mod/hallmark/repo/dothm.py
index 9243189..387c820 100644
--- a/mod/hallmark/repo/dothm.py
+++ b/mod/hallmark/repo/dothm.py
@@ -29,6 +29,9 @@
replace_file_on_success, load_yaml_file, validate_path_name)
from .state import State
+# Files that make up the state database of every ``.hm`` repository.
+STATE_FILES = ("config.yml", "meta.yml", "data.tsv")
+
class _HallmarkYamlDumper(yaml.Dumper):
"""
@@ -196,10 +199,36 @@ def link_worktree(self, path: Union[Path, str], branch: Optional[str] = None):
return Dothm(path)
def load_state(self) -> State:
+ """
+ Read the state database files.
+
+ Returns:
+ State: The repository configuration, metadata and catalog.
+
+ Raises:
+ DothmError: If a state file is missing or cannot be read. The
+ files are only read, never repaired.
+ """
+ missing = [name for name in STATE_FILES if not (self.path / name).is_file()]
+ if missing:
+ raise DothmError(
+ f'Repository at "{self.path}" is damaged: missing '
+ + ", ".join(missing))
+
+ def read(name, reader):
+ try:
+ return reader(Path(name).stem)
+ except (OSError, ValueError, yaml.YAMLError) as exc:
+ # ValueError covers pandas parser and Unicode decoding errors.
+ detail = " ".join(str(exc).split())
+ raise DothmError(
+ f'Repository at "{self.path}" is damaged: cannot read '
+ f"{name} ({detail})") from exc
+
return State(
- config = self.read_yaml("config"),
- meta = self.read_yaml("meta"),
- data = self.read_tsv("data"))
+ config = read("config.yml", self.read_yaml),
+ meta = read("meta.yml", self.read_yaml),
+ data = read("data.tsv", self.read_tsv))
def save_state(self, state: State) -> None:
self.write_yaml(state.config, "config")
diff --git a/mod/hallmark/transport/base.py b/mod/hallmark/transport/base.py
index 58df83f..b88549c 100644
--- a/mod/hallmark/transport/base.py
+++ b/mod/hallmark/transport/base.py
@@ -95,6 +95,45 @@ def reject_control_characters(value: str, label: str) -> None:
raise RemoteConfigurationError(f"{label} contains control characters")
+def reject_url_credentials(url: str) -> None:
+ """
+ Refuse a URL carrying a password, or an HTTP(S) username or token.
+
+ SSH usernames are allowed. Error messages never repeat the URL.
+
+ Raises:
+ RemoteConfigurationError: If the URL contains credentials.
+ """
+ try:
+ parsed = urlsplit(url)
+ except ValueError:
+ raise RemoteConfigurationError("Invalid remote URL") from None
+ if parsed.password is not None or (
+ parsed.username is not None
+ and parsed.scheme.lower() in {"http", "https"}):
+ raise RemoteConfigurationError(
+ "URLs must not contain credentials; configure them locally, for "
+ "example in ~/.netrc, ~/.ssh/config or a Git credential helper")
+
+
+def reject_url_secrets(url: str) -> None:
+ """
+ Refuse a data-remote URL that would store a secret in the catalog.
+
+ Passwords, HTTP(S) usernames (often access tokens), query strings and
+ fragments are rejected; SSH usernames are allowed. Error messages never
+ repeat the URL.
+
+ Raises:
+ RemoteConfigurationError: If the URL could contain a secret.
+ """
+ reject_url_credentials(url)
+ if "?" in url or "#" in url:
+ raise RemoteConfigurationError(
+ "Remote URLs must not contain a query or fragment; "
+ "percent-encode ? and # in file names")
+
+
def validate_remote_path(value) -> Path:
"""Validate a literal catalog path without decoding or trimming it."""
raw = str(value)
diff --git a/test/test_backend_config.py b/test/test_backend_config.py
index f1c8029..5708189 100644
--- a/test/test_backend_config.py
+++ b/test/test_backend_config.py
@@ -54,7 +54,7 @@ def test_add_passes_backend_settings_to_discovery(monkeypatch, tmp_path):
def discover_remote_files(context, **kwargs):
captured.append(context.remote)
- return []
+ return [RemoteEntry("tile.fits")]
# No backend installation or network is required to inspect this hand-off.
class Context:
diff --git a/test/test_catalog.py b/test/test_catalog.py
index 61bd5e8..b5ff875 100644
--- a/test/test_catalog.py
+++ b/test/test_catalog.py
@@ -59,12 +59,20 @@ def test_remote_add_does_not_require_filename_inference(tmp_path, metadata_serve
assert repo.plan_download().file_count == 1
-def test_empty_filtered_add_is_valid(tmp_path, metadata_server):
+def test_empty_filtered_add_is_an_error_and_keeps_catalog(tmp_path, metadata_server):
pages, _ = metadata_server
root = "https://example.test/data/"
pages[root] = 'Index of data
notes.txt'
repo = Repo.init(tmp_path / "clone")
- repo.add(root, remote_fmt="run{run:d}.h5")
+ before = {name: (repo.dothm.path / name).read_bytes()
+ for name in ("config.yml", "data.tsv", ".git/index")}
+ with pytest.raises(ValueError) as error:
+ repo.add(root, remote_fmt="run{run:d}.h5")
+ message = str(error.value)
+ assert "No remote files matched" in message
+ assert "run{run:d}.h5" in message and root in message
+ assert "previous catalogue kept" in message
+ assert before == {name: (repo.dothm.path / name).read_bytes() for name in before}
assert repo.state.data.empty
assert repo.plan_download().file_count == 0
@@ -101,14 +109,15 @@ def test_published_snapshot_preserves_data_remote(tmp_path, metadata_server, nes
def test_filtered_git_clone_preserves_history_without_objects(tmp_path, monkeypatch):
+ import pandas as pd
+
source = Repo.init(tmp_path / "source")
- (source.worktree / "run1.h5").write_text("one")
- (source.worktree / "run2.h5").write_text("two")
- source.add("run{run:d}.h5")
- source.set_config(remote_url="https://example.test/data/")
+ source.state.config = {"data": [{"db": "data.tsv"}],
+ "remote": {"url": "https://example.test/data/"}}
+ source.state.data = pd.DataFrame({"path": ["run1.h5", "run2.h5"]})
+ source.dothm.save_state(source.state)
source.commit("Original scientific data")
head = source.dothm.head.commit.hexsha
- assert list(source.objects.root.rglob("*"))
def fail(*args, **kwargs):
raise AssertionError("Filtered metadata clone must not transfer data")
@@ -390,3 +399,30 @@ def get(url, **kwargs):
assert all(url.startswith(catalog_url) for url in metadata_server[1])
assert (repo.worktree / "a.fits").read_bytes() == b"fits"
assert repo.download_result["succeeded"] == 1
+
+
+@pytest.mark.parametrize("name", ["empty", "empty.hm"])
+def test_snapshot_clone_into_existing_empty_folder(tmp_path, metadata_server, name):
+ pages, _ = metadata_server
+ root = "https://example.test/published/"
+ pages[root + "config.yml"] = "data:\n- db: data.tsv\n"
+ pages[root + "meta.yml"] = "{}\n"
+ pages[root + "data.tsv"] = "path\na.h5\n"
+ target = tmp_path / name
+ target.mkdir()
+ repo = Repo.clone(root, target, download=False)
+ assert repo.state.data["path"].tolist() == ["a.h5"]
+ assert repo.dothm.path == (target if name.endswith(".hm") else target / ".hm")
+
+
+def test_failed_snapshot_clone_keeps_existing_folder_empty(tmp_path, metadata_server):
+ pages, _ = metadata_server
+ root = "https://example.test/published/"
+ pages[root + "config.yml"] = "data:\n- db: data.tsv\n"
+ pages[root + "meta.yml"] = "{}\n"
+ pages[root + "data.tsv"] = "path\n../escape\n"
+ target = tmp_path / "empty"
+ target.mkdir()
+ with pytest.raises((CloneError, ValueError)):
+ Repo.clone(root, target, download=False)
+ assert target.is_dir() and list(target.iterdir()) == []
diff --git a/test/test_cli.py b/test/test_cli.py
index df11e45..c4e8116 100644
--- a/test/test_cli.py
+++ b/test/test_cli.py
@@ -42,7 +42,7 @@
def _install_repo(monkeypatch, worktree=Path("worktree")):
"""
- Install a fake hallmark repository for testing, monkeypatching the Repo class
+ Install a fake hallmark repository for testing, monkeypatching Repo.find
to return a SimpleNamespace with the given worktree.
Args:
monkeypatch: pytest fixture for monkeypatching functions and attributes.
@@ -55,7 +55,7 @@ def _install_repo(monkeypatch, worktree=Path("worktree")):
worktree=worktree,
plan_download=lambda output=None, **kwargs: _download_plan(
0, output or worktree or "downloads"))
- monkeypatch.setattr(cli_module, "Repo", lambda path: repo)
+ monkeypatch.setattr(cli_module, "Repo", SimpleNamespace(find=lambda start: repo))
return repo
@@ -102,10 +102,10 @@ def test_group_reports_repository_open_error(monkeypatch):
Raises:
GitError: Simulated error to test error handling in the CLI.
"""
- def fail_repo(path):
+ def fail_repo(start):
""" Raise a GitError to simulate a failure to open a hallmark repository."""
raise GitError("not a hallmark repository")
- monkeypatch.setattr(cli_module, "Repo", fail_repo)
+ monkeypatch.setattr(cli_module, "Repo", SimpleNamespace(find=fail_repo))
result = CliRunner().invoke(hallmark, ["info"])
assert result.exit_code != 0, f"Expected non-zero exit code, got {result.exit_code}"
@@ -642,8 +642,11 @@ def fail_download(self, plan, **kwargs):
"clone", str(source.dothm.path), str(tmp_path / "target")],
input="y\n")
assert result.exit_code != 0
- assert "Error: Remote download failed" in result.output
+ assert "Error: Clone incomplete: catalogue at" in result.output
+ assert "Remote download failed" in result.output
+ assert "run hm download --all in" in result.output
assert "Download these files? [y/N]" in result.output
+ assert (tmp_path / "target" / ".hm" / "data.tsv").is_file()
def test_clone_cli_skips_download_when_no_remote_files(monkeypatch, tmp_path):
@@ -691,9 +694,11 @@ def test_clone_rejects_nonpositive_max_workers(max_workers):
@pytest.mark.parametrize(
"arguments, message",
[
- (["download"], "Provide file paths, --tsv, --all, --filter, or --fmt"),
- (["download", "file.dat", "--all"], "--all cannot be combined"),
- (["download", "--tsv", "data", "--all"], "--all cannot be combined")])
+ (["download"], "Choose files with paths or --all"),
+ (["download", "--filter", "*.dat"], "only narrow"),
+ (["download", "--fmt", "{name}.dat", "--dry-run"], "only narrow"),
+ (["download", "file.dat", "--all"], "Give paths or --all, not both"),
+ (["download", "--tsv", "data", "--all"], "Give paths or --all, not both")])
def test_download_cli_rejects_invalid_selection_combinations(
monkeypatch, arguments, message):
"""
@@ -706,10 +711,15 @@ def test_download_cli_rejects_invalid_selection_combinations(
arguments: The list of command line arguments to be tested.
message: The expected error message to be found in the download command output.
"""
- _install_repo(monkeypatch)
+ repo = _install_repo(monkeypatch)
+
+ def reject_plan(*args, **kwargs):
+ raise AssertionError("an invalid selection must not be planned")
+
+ repo.plan_download = reject_plan
result = CliRunner().invoke(hallmark, arguments)
- assert result.exit_code != 0, f"Expected non-zero exit code for download with \
+ assert result.exit_code == 2, f"Expected usage exit code for download with \
arguments {arguments}, got {result.exit_code}"
assert message in result.output, \
f"Expected error message '{message}' in output, got: {result.output}"
@@ -771,7 +781,7 @@ def reject_download(*args, **kwargs):
assert "file-000.dat" in result.output
assert "file-019.dat" in result.output
assert "file-020.dat" not in result.output
- assert "... 3 more file(s)" in result.output
+ assert "... 3 more" in result.output
assert "Download these files?" not in result.output
@@ -783,8 +793,10 @@ def test_download_cli_reports_empty_selection(monkeypatch):
assert "Download these files?" not in result.output
-def test_download_cli_passes_selection_and_options_to_downloader(monkeypatch):
- repo = _install_repo(monkeypatch)
+def test_download_cli_passes_selection_and_options_to_downloader(
+ monkeypatch, tmp_path):
+ monkeypatch.chdir(tmp_path)
+ repo = _install_repo(monkeypatch, worktree=tmp_path)
plan = _download_plan(1)
captured = {}
@@ -910,7 +922,7 @@ def capture_get(url, **kwargs):
else:
target = source.worktree
monkeypatch.chdir(target)
- arguments = ["download", "--filter", "*.fits"]
+ arguments = ["download", "--all", "--filter", "*.fits"]
result = CliRunner().invoke(hallmark, arguments, input=answer)
assert "1 file(s); 4 bytes" in result.output
assert "Download these files? [y/N]" in result.output
@@ -987,3 +999,282 @@ def metadata(context, path):
assert Repo(destination).state.data["path"].tolist() == ["a.fits"]
assert (destination / "a.fits").exists() == (answer == "y\n")
assert (result.exit_code == 0) == (answer == "y\n")
+
+
+### repository discovery tests ###
+
+def test_cli_commands_find_the_repository_from_a_subfolder(monkeypatch, tmp_path):
+ repo = _local_cli_catalog(tmp_path / "project")
+ nested = repo.worktree / "sub" / "dir"
+ nested.mkdir(parents=True)
+ monkeypatch.chdir(nested)
+ runner = CliRunner()
+
+ result = runner.invoke(hallmark, ["info"])
+ assert result.exit_code == 0, result.output
+ assert f'dot-hallmark repo: "{repo.dothm.path}"' in result.output
+ assert f'hallmark worktree: "{repo.worktree}"' in result.output
+
+ result = runner.invoke(hallmark, ["status"])
+ assert result.exit_code == 0, result.output
+ assert "On branch main" in result.output
+
+ result = runner.invoke(hallmark, ["download", "--all", "--dry-run"])
+ assert result.exit_code == 0, result.output
+ assert f"Destination: {repo.worktree}" in result.output
+ assert "tiny.fits" in result.output
+ assert not (nested / ".hm").exists()
+
+
+def test_cli_reports_missing_repository(monkeypatch, tmp_path):
+ monkeypatch.chdir(tmp_path)
+ result = CliRunner().invoke(hallmark, ["status"])
+ assert result.exit_code == 1
+ assert "Not a Hallmark repository (or any parent folder)" in result.output
+ assert not (tmp_path / ".hm").exists()
+
+
+def test_cli_download_paths_are_relative_to_the_current_folder(monkeypatch, tmp_path):
+ repo = _local_cli_catalog(tmp_path / "project")
+ (repo.worktree / "sub").mkdir()
+ monkeypatch.chdir(repo.worktree / "sub")
+ result = CliRunner().invoke(hallmark, ["download", "../tiny.fits", "--dry-run"])
+ assert result.exit_code == 0, result.output
+ assert "1 file(s)" in result.output
+ assert " tiny.fits" in result.output
+
+
+@pytest.mark.parametrize("path, message", [
+ ("../../outside.fits", "outside the repository"),
+ ("../.hm/config.yml", "inside .hm"),
+])
+def test_cli_download_rejects_paths_outside_the_worktree(
+ monkeypatch, tmp_path, path, message):
+ repo = _local_cli_catalog(tmp_path / "project")
+ (repo.worktree / "sub").mkdir()
+ monkeypatch.chdir(repo.worktree / "sub")
+
+ def reject_plan(*args, **kwargs):
+ raise AssertionError("invalid paths must be rejected before planning")
+
+ monkeypatch.setattr(Repo, "plan_download", reject_plan)
+ result = CliRunner().invoke(hallmark, ["download", path, "--dry-run"])
+ assert result.exit_code != 0
+ assert message in result.output
+
+
+@pytest.mark.parametrize("folder, argument", [
+ ("A", "."), ("A", ".."), ("A", "../"), (".", "./"), ("A", ""),
+ ("A/deeper", "../.."),
+])
+def test_cli_add_dot_from_a_subfolder_stages_the_whole_repository(
+ monkeypatch, tmp_path, folder, argument):
+ repo = Repo.init(tmp_path / "project")
+ for group in ("A", "B"):
+ (repo.worktree / group).mkdir()
+ for run in (1, 2):
+ (repo.worktree / group / f"run{run}.txt").write_text(f"{group}{run}")
+ repo.add("{group}/run{run:d}.txt")
+ repo.commit("Record both folders")
+ changed = repo.worktree / "A" / "run1.txt"
+ changed.write_text("A1 fixed")
+ (repo.worktree / folder).mkdir(parents=True, exist_ok=True)
+ monkeypatch.chdir(repo.worktree / folder)
+ if argument == "":
+ argument = str(repo.worktree)
+ result = CliRunner().invoke(hallmark, ["add", argument])
+ assert result.exit_code == 0, result.output
+ reopened = Repo(repo.worktree)
+ assert reopened.state.config["data"][0]["fmt"] == "{group}/run{run:d}.txt"
+ data = reopened.state.data
+ rows = {(row["group"], row["run"]): row["sha1"]
+ for row in data.to_dict(orient="records")}
+ assert sorted(rows) == [("A", "1"), ("A", "2"), ("B", "1"), ("B", "2")]
+ assert rows[("A", "1")] == Repo.checksum(changed)
+ assert rows[("B", "2")] == Repo.checksum(repo.worktree / "B" / "run2.txt")
+
+
+def _nested_cli_catalog(path):
+ """Create a remote catalog whose files are spread over nested folders."""
+ repo = Repo.init(path)
+ repo.state.config = {
+ "data": [{"db": "data.tsv"}],
+ "remote": {"name": "origin", "url": "https://example.test/data/"}}
+ repo.state.data = pd.DataFrame({"path": [
+ "runs/a.fits", "runs/2024/b.fits", "notes/readme.txt"]})
+ repo.dothm.save_state(repo.state)
+ repo.dothm.index.commit("Catalog nested remote files")
+ return repo
+
+
+def test_cli_download_folder_from_inside_it(monkeypatch, tmp_path):
+ repo = _nested_cli_catalog(tmp_path / "project")
+ (repo.worktree / "runs").mkdir()
+ monkeypatch.chdir(repo.worktree / "runs")
+ result = CliRunner().invoke(hallmark, ["download", ".", "--dry-run"])
+ assert result.exit_code == 0, result.output
+ assert "2 file(s)" in result.output
+ assert " runs/a.fits" in result.output
+ assert " runs/2024/b.fits" in result.output
+ assert "readme.txt" not in result.output
+ assert f"Destination: {repo.worktree}" in result.output
+
+
+def test_cli_dry_run_reports_unknown_paths(monkeypatch, tmp_path):
+ repo = _nested_cli_catalog(tmp_path / "project")
+ monkeypatch.chdir(repo.worktree)
+ result = CliRunner().invoke(hallmark, [
+ "download", "runs", "missing.fits", "empty/", "--dry-run"])
+ assert result.exit_code == 0, result.output
+ assert "2 file(s)" in result.output
+ assert "2 path(s) not in the catalog" in result.output
+ assert "Not in the catalog:\n missing.fits\n empty\n" in result.output
+
+
+def test_cli_unknown_paths_stop_before_prompt_or_server(monkeypatch, tmp_path):
+ repo = _nested_cli_catalog(tmp_path / "project")
+ monkeypatch.chdir(repo.worktree)
+
+ def reject(*args, **kwargs):
+ raise AssertionError("unknown paths must not contact the server")
+
+ monkeypatch.setattr(requests, "Session", reject)
+ monkeypatch.setattr("hallmark.remote.download.OperationContext", reject)
+ result = CliRunner().invoke(hallmark, ["download", "runs", "missing.fits"],
+ input="y\n")
+ assert result.exit_code == 1
+ assert "Not in the catalog:\n missing.fits" in result.output
+ assert "1 path(s) not in the catalog; nothing was downloaded" in result.output
+ assert "Download these files?" not in result.output
+ assert not (repo.worktree / "runs").exists()
+
+
+def _checked_cli_catalog(path):
+ """Create a remote catalog whose files have published SHA-256 checksums."""
+ from hashlib import sha256
+
+ repo = Repo.init(path)
+ repo.state.config = {
+ "data": [{"db": "data.tsv"}],
+ "remote": {"name": "origin", "url": "https://example.test/data/"}}
+ repo.state.data = pd.DataFrame([
+ {"path": name, "checksum_algorithm": "sha256",
+ "checksum": sha256(content).hexdigest(), "size_bytes": len(content)}
+ for name, content in [("a.fits", b"aaaa"), ("b.fits", b"bbbb"),
+ ("c.fits", b"cccc")]])
+ repo.dothm.save_state(repo.state)
+ repo.dothm.index.commit("Catalog checked remote files")
+ return repo
+
+
+def test_cli_dry_run_reports_skipped_conflicting_and_unknown_files(
+ monkeypatch, tmp_path):
+ repo = _checked_cli_catalog(tmp_path / "project")
+ (repo.worktree / "a.fits").write_bytes(b"aaaa")
+ (repo.worktree / "b.fits").write_bytes(b"xxxx")
+ monkeypatch.chdir(repo.worktree)
+ result = CliRunner().invoke(hallmark, ["download", ".", "missing.fits",
+ "--dry-run"])
+ assert result.exit_code == 0, result.output
+ assert "Files to download:\n c.fits\n" in result.output
+ assert "Already downloaded (skipped):\n a.fits\n" in result.output
+ assert "b.fits (different contents)" in result.output
+ assert "Delete conflicting files first to replace them" in result.output
+ assert "Not in the catalog:\n missing.fits\n" in result.output
+ assert (repo.worktree / "b.fits").read_bytes() == b"xxxx"
+
+
+def test_cli_conflict_downloads_nothing_and_never_prompts(monkeypatch, tmp_path):
+ repo = _checked_cli_catalog(tmp_path / "project")
+ (repo.worktree / "b.fits").write_bytes(b"xxxx")
+ monkeypatch.chdir(repo.worktree)
+
+ def reject(*args, **kwargs):
+ raise AssertionError("a conflict must not contact the server")
+
+ monkeypatch.setattr(requests, "Session", reject)
+ result = CliRunner().invoke(hallmark, ["download", "--all"], input="y\n")
+ assert result.exit_code == 1
+ assert "b.fits (different contents)" in result.output
+ assert "Delete conflicting files first to replace them" in result.output
+ assert "nothing was downloaded" in result.output
+ assert "Download these files?" not in result.output
+ assert not (repo.worktree / "a.fits").exists()
+ assert (repo.worktree / "b.fits").read_bytes() == b"xxxx"
+
+
+def test_cli_second_download_skips_verified_files(monkeypatch, tmp_path):
+ from mock_server import MockServer
+
+ repo = _checked_cli_catalog(tmp_path / "project")
+ server = MockServer("https://example.test/data/")
+ for name in ("a.fits", "b.fits", "c.fits"):
+ server.add_file(name, name[0].encode() * 4)
+ requested = []
+ original_get = server.get
+
+ def get(url, **kwargs):
+ requested.append(url)
+ return original_get(url, **kwargs)
+
+ server.get = get
+ monkeypatch.setattr(requests, "Session", lambda: server)
+ monkeypatch.chdir(repo.worktree)
+ runner = CliRunner()
+ result = runner.invoke(hallmark, ["download", "a.fits"], input="y\n")
+ assert result.exit_code == 0, result.output
+ assert requested == ["https://example.test/data/a.fits"]
+ result = runner.invoke(hallmark, ["download", "--all"], input="y\n")
+ assert result.exit_code == 0, result.output
+ assert "1 file(s) already downloaded; skipped" in result.output
+ assert sorted(requested[1:]) == ["https://example.test/data/b.fits",
+ "https://example.test/data/c.fits"]
+ result = runner.invoke(hallmark, ["download", "--all"])
+ assert result.exit_code == 0, result.output
+ assert "All selected files are already downloaded." in result.output
+ assert "Download these files?" not in result.output
+ assert len(requested) == 3
+
+
+def test_cli_retry_after_a_failed_clone_downloads_only_missing_files(
+ monkeypatch, tmp_path):
+ from mock_server import MockServer
+
+ source = Repo.init(tmp_path / "source")
+ source.state.config = {
+ "data": [{"db": "data.tsv"}],
+ "remote": {"name": "origin", "url": "https://example.test/data/"}}
+ # Like an SSH export without a checksum manifest: only sizes are known.
+ source.state.data = pd.DataFrame([{"path": "a.fits", "size_bytes": 1},
+ {"path": "b.fits", "size_bytes": 1}])
+ source.dothm.save_state(source.state)
+ source.dothm.index.commit("Catalog files without checksums")
+ server = MockServer("https://example.test/data/")
+ server.add_file("a.fits", b"A")
+ server.add_file("b.fits", b"B")
+ requested, failing = [], {"b.fits"}
+ original_get = server.get
+
+ def get(url, **kwargs):
+ requested.append(url)
+ if any(url.endswith(name) for name in failing):
+ raise requests.ConnectionError("transient network failure")
+ return original_get(url, **kwargs)
+
+ server.get = get
+ monkeypatch.setattr(requests, "Session", lambda: server)
+ target = tmp_path / "copy"
+ result = CliRunner().invoke(hallmark, [
+ "clone", str(source.dothm.path), str(target)], input="y\n")
+ assert result.exit_code != 0
+ assert (target / "a.fits").read_bytes() == b"A"
+ failing.clear()
+ requested.clear()
+ monkeypatch.chdir(target)
+ result = CliRunner().invoke(hallmark, ["download", "--all", "--dry-run"])
+ assert "Present (size matches, not verified):\n a.fits\n" in result.output
+ result = CliRunner().invoke(hallmark, ["download", "--all"], input="y\n")
+ assert result.exit_code == 0, result.output
+ assert "1 file(s) present (size matches, not verified); skipped" in result.output
+ assert requested == ["https://example.test/data/b.fits"]
+ assert (target / "b.fits").read_bytes() == b"B"
diff --git a/test/test_clone_defaults.py b/test/test_clone_defaults.py
index 0cfbb5b..61d4455 100644
--- a/test/test_clone_defaults.py
+++ b/test/test_clone_defaults.py
@@ -1,23 +1,43 @@
from hashlib import sha1
+from pathlib import Path
+import pandas as pd
import pytest
import requests
from click.testing import CliRunner
from hallmark import Repo
from hallmark.cli import hallmark
+from hallmark.error import CloneError, DestinationExistsError
+from hallmark.remote.clone import default_clone_destination
from hallmark.transport.base import DownloadError
from mock_server import MockServer
+def _catalog_remote_files(repo, files, message="Catalogued remote data"):
+ """Commit a catalog of files that stay on the mock HTTP server."""
+ repo.state.config = {
+ "data": [{"db": "data.tsv"}],
+ "remote": {"name": "origin", "url": "https://clone.test/data/"}}
+ repo.state.data = pd.DataFrame([
+ {"path": name, "checksum_algorithm": "sha1",
+ "checksum": sha1(content).hexdigest(), "size_bytes": len(content)}
+ for name, content in files])
+ repo.dothm.save_state(repo.state)
+ repo.dothm.index.commit(message)
+
+
+def _remote_catalog_source(path, files=(("item1.txt", b"science"),)):
+ """Create a repository whose committed catalog lists remote files."""
+ repo = Repo.init(path)
+ _catalog_remote_files(repo, files)
+ return repo
+
+
@pytest.fixture
def clone_source(tmp_path, monkeypatch):
- source = Repo.init(tmp_path / 'source')
payload = b'science'
- (source.worktree / 'item1.txt').write_bytes(payload)
- source.add('item{item:d}.txt')
- source.set_config(remote_url='https://clone.test/data/')
- source.commit('Catalogued data')
+ source = _remote_catalog_source(tmp_path / 'source', [('item1.txt', payload)])
server = MockServer('https://clone.test/data/')
server.add_file('item1.txt', payload)
calls = []
@@ -40,7 +60,7 @@ def test_python_clone_downloads_by_default(clone_source, tmp_path):
target = tmp_path / 'copy'
repo = Repo.clone(str(source.dothm.path), target)
assert (target / 'item1.txt').read_bytes() == b'science'
- assert repo.state.data.iloc[0]['sha1'] == sha1(b'science').hexdigest()
+ assert repo.state.data.iloc[0]['checksum'] == sha1(b'science').hexdigest()
assert repo.download_result['succeeded'] == 1
assert calls == ['https://clone.test/data/item1.txt']
@@ -161,9 +181,8 @@ def test_clone_filtered_to_zero_files_does_not_prompt(clone_source, tmp_path):
def test_clone_download_failure_keeps_catalog_and_valid_files(
clone_source, tmp_path, cli):
source, calls, server = clone_source
- (source.worktree / 'item2.txt').write_bytes(b'science')
- source.add('item{item:d}.txt')
- source.commit('Add another file')
+ _catalog_remote_files(source, [('item1.txt', b'science'),
+ ('item2.txt', b'science')], 'Add another file')
server.add_file('item1.txt', b'invalid')
server.add_file('item2.txt', b'science')
target = tmp_path / 'copy'
@@ -176,13 +195,372 @@ def test_clone_download_failure_keeps_catalog_and_valid_files(
with pytest.raises(DownloadError) as error:
Repo.clone(str(source.dothm.path), target)
message = str(error.value)
- assert 'Failed to download 1 file(s)' in message
+ assert f'Clone incomplete: catalogue at "{target / ".hm"}"' in message
+ assert '1 file(s) failed' in message
assert 'Checksum mismatch' in message
assert 'item1.txt' in message
- assert f'Catalog kept at "{target / ".hm"}"' in message
- assert 'Successfully downloaded files were kept.' in message
+ assert 'Successfully downloaded files were kept' in message
+ assert f'run hm download --all in "{target}" to retry' in message
assert Repo(target).dothm.head.commit.hexsha == source.dothm.head.commit.hexsha
assert not (target / 'item1.txt').exists()
assert (target / 'item2.txt').read_bytes() == b'science'
assert sorted(calls) == [
'https://clone.test/data/item1.txt', 'https://clone.test/data/item2.txt']
+
+
+@pytest.mark.parametrize("source, expected", [
+ ("https://github.com/team/eht-data.git", "eht-data"),
+ ("git@github.com:team/eht-data.git", "eht-data"),
+ ("ssh://git.example.test/srv/eht-data.git/", "eht-data"),
+ ("/data/eht-data/.hm", "eht-data"),
+ ("/data/eht-data/.hm/", "eht-data"),
+ ("/data/eht-data", "eht-data"),
+ ("../catalog.hm", "catalog"),
+ ("https://catalogs.example.org/lab/", "lab"),
+ ("https://catalogs.example.org/published/.hm/", "published"),
+ ("file:///data/eht%20data/.hm", "eht data"),
+])
+def test_default_destination_is_named_after_the_source(source, expected):
+ assert default_clone_destination(source) == Path(expected)
+
+
+@pytest.mark.parametrize("source", ["https://example.test/", "/", ".hm", ".."])
+def test_unnamed_source_requires_a_directory(source):
+ with pytest.raises(CloneError, match="DIRECTORY"):
+ default_clone_destination(source)
+
+
+def test_cli_clone_defaults_to_a_folder_named_after_the_source(
+ tmp_path, monkeypatch):
+ source = _remote_catalog_source(tmp_path / "eht-data")
+ work = tmp_path / "work"
+ work.mkdir()
+ monkeypatch.chdir(work)
+ result = CliRunner().invoke(hallmark, [
+ "clone", str(source.dothm.path), "--no-download"])
+ assert result.exit_code == 0, result.output
+ assert 'Successfully cloned to "eht-data"' in result.output
+ assert Repo(work / "eht-data").dothm.head.commit.hexsha == \
+ source.dothm.head.commit.hexsha
+ other = tmp_path / "other"
+ other.mkdir()
+ monkeypatch.chdir(other)
+ python = Repo.clone(str(source.worktree), download=False)
+ assert python.worktree == other / "eht-data"
+
+
+@pytest.mark.parametrize("cli", [False, True])
+def test_clone_into_an_existing_empty_folder(tmp_path, cli):
+ source = _remote_catalog_source(tmp_path / "source")
+ target = tmp_path / "empty"
+ target.mkdir()
+ if cli:
+ result = CliRunner().invoke(hallmark, [
+ "clone", str(source.dothm.path), str(target), "--no-download"])
+ assert result.exit_code == 0, result.output
+ else:
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert Repo(target).dothm.head.commit.hexsha == source.dothm.head.commit.hexsha
+
+
+def test_failed_clone_empties_an_existing_folder_but_keeps_it(tmp_path):
+ source = _remote_catalog_source(tmp_path / "source")
+ (source.dothm.path / "meta.yml").unlink()
+ source.dothm.index.remove(["meta.yml"])
+ source.dothm.index.commit("Remove required metadata")
+ target = tmp_path / "empty"
+ target.mkdir()
+ with pytest.raises(CloneError, match="missing required file"):
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert target.is_dir()
+ assert list(target.iterdir()) == []
+
+
+def test_failed_clone_keeps_files_added_to_an_existing_folder(
+ tmp_path, monkeypatch):
+ from hallmark.remote.clone import Dothm
+
+ source = _remote_catalog_source(tmp_path / "source")
+ target = tmp_path / "empty"
+ target.mkdir()
+ original = Dothm.clone
+
+ def clone_then_fail(url, destination, **kwargs):
+ original(url, destination, **kwargs)
+ # Someone saves a file in the folder while the clone runs.
+ (target / "notes.txt").write_text("keep me")
+ raise CloneError("simulated failure")
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", clone_then_fail)
+ with pytest.raises(CloneError, match="simulated failure"):
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert sorted(path.name for path in target.iterdir()) == ["notes.txt"]
+ assert (target / "notes.txt").read_text() == "keep me"
+
+
+def test_failed_clone_empties_an_existing_bare_folder(tmp_path):
+ source = _remote_catalog_source(tmp_path / "source")
+ (source.dothm.path / "meta.yml").unlink()
+ source.dothm.index.remove(["meta.yml"])
+ source.dothm.index.commit("Remove required metadata")
+ target = tmp_path / "copy.hm"
+ target.mkdir()
+ with pytest.raises(CloneError, match="missing required file"):
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert target.is_dir() and list(target.iterdir()) == []
+
+
+def test_failed_clone_never_follows_a_swapped_in_link(tmp_path, monkeypatch):
+ from hallmark.remote.clone import Dothm
+
+ source = _remote_catalog_source(tmp_path / "source")
+ target = tmp_path / "copy.hm"
+ target.mkdir()
+ elsewhere = tmp_path / "elsewhere"
+ elsewhere.mkdir()
+ (elsewhere / "keep.txt").write_text("keep me")
+ original = Dothm.clone
+
+ def clone_then_swap(url, destination, **kwargs):
+ original(url, destination, **kwargs)
+ target.rename(tmp_path / "moved.hm")
+ target.symlink_to(elsewhere, target_is_directory=True)
+ raise CloneError("simulated failure")
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", clone_then_swap)
+ with pytest.raises(CloneError, match="simulated failure"):
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert target.is_symlink()
+ assert (elsewhere / "keep.txt").read_text() == "keep me"
+
+
+def test_clone_refuses_a_destination_inside_a_repository(tmp_path, monkeypatch):
+ source = _remote_catalog_source(tmp_path / "source")
+ other = Repo.init(tmp_path / "other")
+ (other.worktree / "sub").mkdir()
+ monkeypatch.chdir(other.worktree / "sub")
+ for arguments in (["clone", str(source.dothm.path), "--no-download"],
+ ["clone", str(source.dothm.path), "nested/copy"]):
+ result = CliRunner().invoke(hallmark, arguments)
+ assert result.exit_code == 1
+ assert "inside the Hallmark repository" in result.output
+ assert str(other.worktree) in result.output
+ assert sorted(path.name for path in (other.worktree / "sub").iterdir()) == []
+
+
+@pytest.mark.parametrize("relative", ["source/copy", "source/.hm/copy"])
+def test_clone_refuses_to_overlap_a_local_source(tmp_path, relative):
+ source = _remote_catalog_source(tmp_path / "source")
+ with pytest.raises(CloneError, match="overlaps the source|inside the Hallmark"):
+ Repo.clone(str(source.dothm.path), tmp_path / relative, download=False)
+ assert not (tmp_path / relative).exists()
+
+
+def test_clone_refuses_a_nonempty_folder_or_symlink(tmp_path):
+ source = _remote_catalog_source(tmp_path / "source")
+ full = tmp_path / "full"
+ full.mkdir()
+ (full / "keep.txt").write_text("keep")
+ empty = tmp_path / "empty"
+ empty.mkdir()
+ alias = tmp_path / "alias"
+ alias.symlink_to(empty, target_is_directory=True)
+ for target in (full, alias):
+ with pytest.raises(DestinationExistsError):
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert (full / "keep.txt").read_text() == "keep"
+ assert list(empty.iterdir()) == []
+
+
+def _local_file_source(path, *, on_branch=None):
+ """Create a repository whose catalog versions local files in .hm/objects."""
+ source = Repo.init(path)
+ if on_branch is not None:
+ source.commit("Empty main", allow_empty=True)
+ source.dothm.git.checkout("-b", on_branch)
+ (source.worktree / "item1.txt").write_bytes(b"science")
+ source.add("item{item:d}.txt")
+ source.set_config(remote_url="https://clone.test/data/")
+ source.commit("Catalogued local data")
+ if on_branch is not None:
+ source.dothm.git.checkout("main")
+ return source
+
+
+@pytest.mark.parametrize("arguments", [[], ["--no-download"]])
+@pytest.mark.parametrize("on_branch", [None, "experiment"])
+def test_cli_refuses_to_clone_local_file_repositories(
+ tmp_path, monkeypatch, arguments, on_branch):
+ source = _local_file_source(tmp_path / "source", on_branch=on_branch)
+ monkeypatch.chdir(tmp_path)
+
+ def reject(*args, **kwargs):
+ raise AssertionError("a local-file source must be refused before copying")
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", reject)
+ result = CliRunner().invoke(hallmark, [
+ "clone", str(source.dothm.path), "copy", *arguments])
+ assert result.exit_code == 1
+ assert "tracks local files" in result.output
+ assert "not supported yet" in result.output
+ assert "Nothing was created." in result.output
+ assert not (tmp_path / "copy").exists()
+
+
+@pytest.mark.parametrize("relative", ["copy", "new/deep/copy"])
+def test_local_file_check_runs_after_cloning_a_git_host(
+ tmp_path, monkeypatch, relative):
+ from hallmark.remote.clone import Dothm
+
+ source = _local_file_source(tmp_path / "source", on_branch="experiment")
+ original = Dothm.clone
+
+ def clone(url, destination, **kwargs):
+ return original(str(source.dothm.path), destination, **kwargs)
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", clone)
+ target = tmp_path / relative
+ with pytest.raises(CloneError, match="'experiment' tracks local files"):
+ Repo.clone("https://git.example.test/team/data.git", target, download=False)
+ assert not (tmp_path / Path(relative).parts[0]).exists()
+
+
+@pytest.mark.parametrize("relative", ["copy", "new/deep/copy"])
+def test_snapshot_of_local_files_is_refused(tmp_path, monkeypatch, relative):
+ from hallmark.transport import OperationContext
+ from hallmark.transport.base import RemoteObjectMissing
+
+ root = "https://example.test/published/"
+ pages = {root + "config.yml": "data:\n- fmt: run{run:d}.txt\n",
+ root + "meta.yml": "{}\n",
+ root + "data.tsv": "sha1\trun\n" + "a" * 40 + "\t1\n"}
+
+ def read_text(context, path):
+ url = context.remote.url.rstrip("/") + "/" + path
+ if url not in pages:
+ raise RemoteObjectMissing("No such metadata object")
+ return pages[url]
+
+ monkeypatch.setattr(OperationContext, "read_text", read_text)
+ target = tmp_path / relative
+ with pytest.raises(CloneError, match="tracks local files"):
+ Repo.clone(root, target, download=False)
+ assert not (tmp_path / Path(relative).parts[0]).exists()
+
+
+def test_empty_repository_with_remote_branch_can_be_cloned(tmp_path):
+ source = Repo.init(tmp_path / "source")
+ source.commit("Empty catalog", allow_empty=True)
+ source.dothm.git.checkout("-b", "remote-data")
+ _catalog_remote_files(source, [("item1.txt", b"science")])
+ source.dothm.git.checkout("main")
+ repo = Repo.clone(str(source.dothm.path), tmp_path / "copy", download=False)
+ assert repo.state.data.empty
+
+
+def test_local_folder_that_is_not_a_repository_reports_the_clone_failure(tmp_path):
+ plain = tmp_path / "plain"
+ plain.mkdir()
+ (plain / "notes.txt").write_text("not a repository")
+ with pytest.raises(CloneError, match="hm init PATH, then hm add URL"):
+ Repo.clone(str(plain), tmp_path / "copy", download=False)
+ assert not (tmp_path / "copy").exists()
+
+
+@pytest.mark.parametrize("name", ["-q", "--output=stolen"])
+@pytest.mark.parametrize("host", [False, True])
+def test_clone_refuses_option_like_branch_names(tmp_path, monkeypatch, name, host):
+ from hallmark.remote.clone import Dothm
+
+ source = _remote_catalog_source(tmp_path / "source")
+ source.dothm.git.update_ref(f"refs/heads/{name}", "HEAD")
+ url = str(source.dothm.path)
+ if host:
+ original = Dothm.clone
+
+ def clone(git_url, destination, **kwargs):
+ return original(str(source.dothm.path), destination, **kwargs)
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", clone)
+ url = "https://git.example.test/team/data.git"
+ work = tmp_path / "work"
+ work.mkdir()
+ monkeypatch.chdir(work)
+ result = CliRunner().invoke(hallmark, ["clone", url, "copy", "--no-download"])
+ assert result.exit_code == 1
+ assert f"Source branch {name!r} has an invalid name" in result.output
+ assert list(work.iterdir()) == []
+
+
+def _commit_on_branch(source, branch, files, message):
+ """Commit raw .hm files on a new branch, then return to main."""
+ source.dothm.git.checkout("-b", branch)
+ for name, text in files.items():
+ if text is None:
+ source.dothm.index.remove([name], working_tree=True)
+ else:
+ (source.dothm.path / name).write_text(text)
+ source.dothm.index.add([name])
+ source.dothm.index.commit(message)
+ source.dothm.git.checkout("main")
+
+
+def test_clone_copies_every_branch_pointer(tmp_path, monkeypatch):
+ source = _remote_catalog_source(tmp_path / "source")
+ for branch, name in (("experiment", "item2.txt"), ("notes", "notes.txt")):
+ source.dothm.git.checkout("-b", branch)
+ _catalog_remote_files(source, [(name, b"x")], f"Catalog {name}")
+ source.dothm.git.checkout("main")
+ repo = Repo.clone(str(source.dothm.path), tmp_path / "copy", download=False)
+ assert {head.name: head.commit.hexsha for head in repo.dothm.heads} == {
+ head.name: head.commit.hexsha for head in source.dothm.heads}
+ assert repo.dothm.active_branch.name == "main"
+ monkeypatch.chdir(repo.worktree)
+ result = CliRunner().invoke(hallmark, ["branch"])
+ assert result.output.split() == ["experiment", "*", "main", "notes"]
+ assert repo.checkout("experiment")
+ assert repo.state.data["path"].tolist() == ["item2.txt"]
+
+
+def test_clone_starts_on_the_source_selected_branch(tmp_path):
+ source = _remote_catalog_source(tmp_path / "source")
+ source.dothm.git.checkout("-b", "experiment")
+ _catalog_remote_files(source, [("item2.txt", b"x")], "Catalog item2")
+ repo = Repo.clone(str(source.dothm.path), tmp_path / "copy", download=False)
+ assert repo.dothm.active_branch.name == "experiment"
+ assert repo.state.data["path"].tolist() == ["item2.txt"]
+ assert {head.name for head in repo.dothm.heads} == {"main", "experiment"}
+
+
+@pytest.mark.parametrize("files, message", [
+ ({"data.tsv": "path\n../escape.txt\n"}, "safe relative path"),
+ ({"data.tsv": "path\tchecksum_algorithm\tchecksum\na.txt\tsha256\txyz\n"},
+ "Invalid sha256 checksum"),
+ ({"data.tsv": "path\tchecksum_algorithm\tchecksum\na.txt\tcrc\tabcd\n"},
+ "Unsupported checksum algorithm"),
+ ({"config.yml": "data: [\n"}, "config.yml"),
+ ({"meta.yml": "- not a mapping\n"}, "meta.yml"),
+ ({"meta.yml": None}, "meta.yml"),
+])
+def test_clone_refuses_an_invalid_catalog_on_another_branch(
+ tmp_path, files, message):
+ source = _remote_catalog_source(tmp_path / "source")
+ _commit_on_branch(source, "broken", files, "Break the catalog")
+ target = tmp_path / "copy"
+ with pytest.raises(CloneError, match="broken") as error:
+ Repo.clone(str(source.dothm.path), target, download=False)
+ assert message in str(error.value)
+ assert not target.exists()
+
+
+def test_incomplete_clone_suggests_retrying_the_same_selection(
+ clone_source, tmp_path):
+ source, calls, server = clone_source
+ server.add_file('item1.txt', b'invalid')
+ target = tmp_path / 'copy'
+ with pytest.raises(DownloadError) as error:
+ Repo.clone(str(source.dothm.path), target, filter='item*.txt',
+ fmt='item{n:d}.txt')
+ assert ("run hm download --all --filter 'item*.txt' --fmt 'item{n:d}.txt' "
+ f'in "{target}" to retry') in str(error.value)
+ assert (target / '.hm/data.tsv').is_file()
diff --git a/test/test_download_plan.py b/test/test_download_plan.py
index aef976c..3f204d9 100644
--- a/test/test_download_plan.py
+++ b/test/test_download_plan.py
@@ -55,12 +55,72 @@ def reject_network(*args, **kwargs):
def test_explicit_paths_retain_catalog_checksums_and_metadata(catalog):
- plan = plan_download(catalog, file_paths=["nested/a.fits", "unlisted.bin"])
+ plan = plan_download(catalog, file_paths=["nested/a.fits", "b.txt"])
assert plan.file_count == 2
assert plan.items[0].checksum == ("sha256", sha256(b"abcdef").hexdigest())
assert plan.items[0].size_bytes == 6
assert plan.items[1].checksum is None
assert plan.items[1].size_bytes is None
+ assert plan.unknown_paths == ()
+
+
+def _folder_catalog(tmp_path):
+ """Create a catalog with nested folders and a sibling sharing a prefix."""
+ metadata = tmp_path / ".hm"
+ metadata.mkdir()
+ frame = pd.DataFrame({"path": [
+ "runs/a.h5", "runs/2024/b.h5", "runs/2024/x/c.h5", "runsx/d.h5", "top.h5"]})
+ frame.to_csv(metadata / "data.tsv", sep="\t", index=False)
+ return SimpleNamespace(
+ dothm=SimpleNamespace(path=metadata), worktree=tmp_path,
+ state=SimpleNamespace(data=frame, config={
+ "data": [{"db": "data.tsv"}],
+ "remote": {"name": "origin", "url": "https://source.test/data/"},
+ }))
+
+
+@pytest.mark.parametrize("requested, expected", [
+ (["runs"], ["runs/a.h5", "runs/2024/b.h5", "runs/2024/x/c.h5"]),
+ (["runs/2024/"], ["runs/2024/b.h5", "runs/2024/x/c.h5"]),
+ (["runs/a.h5", "top.h5"], ["runs/a.h5", "top.h5"]),
+ (["runs/2024", "runs/2024/x/c.h5"], ["runs/2024/b.h5", "runs/2024/x/c.h5"]),
+ (["."], ["runs/a.h5", "runs/2024/b.h5", "runs/2024/x/c.h5", "runsx/d.h5",
+ "top.h5"]),
+])
+def test_folders_select_every_catalogued_file_below_them(
+ tmp_path, requested, expected):
+ plan = plan_download(_folder_catalog(tmp_path), file_paths=requested)
+ assert [item.relative_path.as_posix() for item in plan.items] == expected
+ assert plan.unknown_paths == ()
+
+
+def test_filters_narrow_a_folder_selection(tmp_path):
+ plan = plan_download(_folder_catalog(tmp_path), file_paths=["runs"],
+ filter="**/2024/*.h5")
+ assert [item.relative_path.as_posix() for item in plan.items] == [
+ "runs/2024/b.h5"]
+
+
+def test_unknown_paths_and_empty_folders_fail_before_contacting_server(
+ tmp_path, monkeypatch):
+ catalog = _folder_catalog(tmp_path)
+ plan = plan_download(catalog, file_paths=[
+ "runs/a.h5", "missing.h5", "run", "empty/", "runs/a.h5/inner"])
+ assert [item.relative_path.as_posix() for item in plan.items] == ["runs/a.h5"]
+ assert plan.unknown_paths == ("missing.h5", "run", "empty", "runs/a.h5/inner")
+ assert "4 path(s) not in the catalog" in plan.summary()
+
+ def reject(*args, **kwargs):
+ raise AssertionError("unknown paths must fail before contacting a server")
+
+ monkeypatch.setattr("hallmark.remote.download.OperationContext", reject)
+ monkeypatch.setattr("hallmark.remote.download.RemoteSpec", SimpleNamespace(
+ from_url=reject))
+ for approved in (True, False):
+ with pytest.raises(DownloadError, match="missing.h5, run, empty") as error:
+ execute_download_plan(catalog, plan, approved=approved)
+ assert "nothing was downloaded" in str(error.value)
+ assert not (tmp_path / "runs").exists()
def test_sizes_survive_nullable_numeric_tsv_serialization(catalog):
@@ -273,3 +333,161 @@ def test_missing_plugin_can_be_planned_but_not_executed(catalog):
with pytest.raises(DownloadError, match="missing-survey-plugin"):
execute_download_plan(catalog, plan, approved=True)
assert not (catalog.worktree / "nested/a.fits").exists()
+
+
+def _reject_server(monkeypatch):
+ def reject(*args, **kwargs):
+ raise AssertionError("this download must not contact a server")
+
+ monkeypatch.setattr("hallmark.remote.download.OperationContext", reject)
+ monkeypatch.setattr("requests.sessions.Session.request", reject)
+
+
+def test_verified_existing_files_are_skipped_without_a_transfer(
+ catalog, monkeypatch):
+ (catalog.worktree / "nested").mkdir()
+ existing = catalog.worktree / "nested/a.fits"
+ existing.write_bytes(b"abcdef")
+ _reject_server(monkeypatch)
+ plan = plan_download(catalog, file_paths=["nested"])
+ assert plan.items == ()
+ assert [item.relative_path.as_posix() for item in plan.skipped] == [
+ "nested/a.fits"]
+ assert plan.conflicts == ()
+ assert "1 file(s) already downloaded; skipped" in plan.summary()
+ assert execute_download_plan(catalog, plan) == {
+ "succeeded": 0, "failed": 0, "total_bytes": 0, "errors": []}
+ assert existing.read_bytes() == b"abcdef"
+
+
+@pytest.mark.parametrize("make_existing, reason", [
+ (lambda path: path.write_bytes(b"ABCDEF"), "different contents"),
+ (lambda path: path.write_bytes(b"changed"), "7 bytes, but the catalog records 6"),
+ (lambda path: path.mkdir(), "not a regular file"),
+])
+def test_conflicting_existing_file_stops_the_whole_download(
+ catalog, monkeypatch, make_existing, reason):
+ (catalog.worktree / "nested").mkdir()
+ existing = catalog.worktree / "nested/a.fits"
+ make_existing(existing)
+ plan = plan_download(catalog, file_paths=["nested/a.fits", "empty.fits"])
+ assert [item.relative_path.as_posix() for item in plan.items] == ["empty.fits"]
+ assert [(conflict.item.relative_path.as_posix(), conflict.reason)
+ for conflict in plan.conflicts] == [("nested/a.fits", reason)]
+ assert "1 existing file(s) conflict with the catalog" in plan.summary()
+ _reject_server(monkeypatch)
+ with pytest.raises(DownloadError) as error:
+ execute_download_plan(catalog, plan, approved=True)
+ assert "nested/a.fits" in str(error.value)
+ assert "nothing was downloaded" in str(error.value)
+ assert "Delete conflicting files first to replace them" in str(error.value)
+ assert not (catalog.worktree / "empty.fits").exists()
+
+
+def test_existing_file_without_catalog_checksum_is_a_conflict(catalog):
+ (catalog.worktree / "b.txt").write_bytes(b"cannot be verified")
+ plan = plan_download(catalog, file_paths=["b.txt"])
+ assert plan.items == ()
+ assert plan.conflicts[0].item.relative_path == Path("b.txt")
+ assert plan.conflicts[0].reason == "no catalog checksum or size to verify it"
+
+
+def test_unverifiable_file_with_the_catalog_size_is_skipped(catalog, monkeypatch):
+ present = catalog.worktree / "empty.fits"
+ present.write_bytes(b"")
+ plan = plan_download(catalog, file_paths=["empty.fits", "b.txt"])
+ assert [item.relative_path.as_posix() for item in plan.unverified] == [
+ "empty.fits"]
+ assert [item.relative_path.as_posix() for item in plan.items] == ["b.txt"]
+ assert plan.skipped == () and plan.conflicts == ()
+ assert "1 file(s) present (size matches, not verified); skipped" in plan.summary()
+ present.write_bytes(b"grown")
+ plan = plan_download(catalog, file_paths=["empty.fits"])
+ assert [(conflict.item.relative_path.as_posix(), conflict.reason)
+ for conflict in plan.conflicts] == [
+ ("empty.fits", "5 bytes, but the catalog records 0")]
+
+
+def test_wrong_size_is_a_conflict_without_reading_the_file(catalog, monkeypatch):
+ (catalog.worktree / "nested").mkdir()
+ (catalog.worktree / "nested/a.fits").write_bytes(b"x" * 10)
+
+ def reject(*args, **kwargs):
+ raise AssertionError("a file of the wrong size must not be hashed")
+
+ monkeypatch.setattr("hallmark.remote.download.calculate_file_checksum", reject)
+ plan = plan_download(catalog, file_paths=["nested/a.fits"])
+ assert plan.conflicts[0].reason == "10 bytes, but the catalog records 6"
+
+
+@pytest.mark.parametrize("digest, content, expected", [
+ (sha256(b"abcdef").hexdigest(), b"abcdef", "skipped"),
+ (sha256(b"abcdef").hexdigest(), b"ABCDEF", "different contents"),
+ ("a" * 36, b"abcdef", "unverified"),
+])
+def test_unknown_checksum_algorithm_is_inferred_from_the_digest_length(
+ tmp_path, digest, content, expected):
+ catalog = _folder_catalog(tmp_path)
+ frame = pd.DataFrame([{"path": "run.h5", "checksum_algorithm": "unknown",
+ "checksum": digest, "size_bytes": "6"}])
+ frame.to_csv(catalog.dothm.path / "data.tsv", sep="\t", index=False)
+ (tmp_path / "run.h5").write_bytes(content)
+ plan = plan_download(catalog, file_paths=["run.h5"])
+ if expected == "skipped":
+ assert plan.skipped[0].checksum == ("sha256", digest)
+ elif expected == "unverified":
+ assert plan.unverified[0].checksum is None
+ else:
+ assert plan.conflicts[0].reason == expected
+
+
+def test_existing_files_are_hashed_with_a_bounded_pool(catalog, monkeypatch):
+ from concurrent.futures import ThreadPoolExecutor
+
+ workers = []
+
+ def pool(max_workers=None):
+ workers.append(max_workers)
+ return ThreadPoolExecutor(max_workers=max_workers)
+
+ monkeypatch.setattr("hallmark.remote.download.ThreadPoolExecutor", pool)
+ monkeypatch.setattr("hallmark.remote.download.os.cpu_count", lambda: 64)
+ (catalog.worktree / "empty.fits").write_bytes(b"")
+ plan_download(catalog, file_paths=["empty.fits"])
+ assert workers == [8]
+
+
+def test_file_appearing_after_planning_is_never_replaced(catalog, monkeypatch):
+ plan = plan_download(catalog, file_paths=["nested/a.fits"])
+ assert plan.file_count == 1
+ late = catalog.worktree / "nested/a.fits"
+
+ class Context:
+ def __init__(self, remote, output_root):
+ self.output_root = output_root
+ self.transport = self
+ self.on_bytes = None
+
+ def __enter__(self):
+ return self
+
+ def __exit__(self, *args):
+ return False
+
+ def prepare(self):
+ pass
+
+ def check_cancelled(self):
+ pass
+
+ def fetch(self, relative_path, destination, **kwargs):
+ destination.write_bytes(b"abcdef")
+ # Another program creates the file while the transfer runs.
+ late.write_bytes(b"written meanwhile")
+
+ monkeypatch.setattr("hallmark.remote.download.OperationContext", Context)
+ result = execute_download_plan(catalog, plan, approved=True)
+ assert result["succeeded"] == 0 and result["failed"] == 1
+ assert "appeared after the download was planned" in result["errors"][0]
+ assert late.read_bytes() == b"written meanwhile"
+ assert not list(late.parent.glob("*.part"))
diff --git a/test/test_downloader.py b/test/test_downloader.py
index 8f7e49d..f57990d 100644
--- a/test/test_downloader.py
+++ b/test/test_downloader.py
@@ -419,7 +419,8 @@ def test_download_requires_remote_url(tmp_path):
tmp_path: A pytest fixture providing a temporary directory.
Raises:
DownloadError: If the remote configuration is present but the URL is missing."""
- repo = _repo(tmp_path, {"remote": {"name": "origin"}})
+ repo = _repo(tmp_path, {"remote": {"name": "origin"}},
+ pd.DataFrame([{"path": "data.bin"}]))
with pytest.raises(DownloadError, match="No remote URL"):
plan_download(repo, tmp_path, file_paths=["data.bin"])
@@ -671,7 +672,7 @@ def test_download_rejects_selected_files_without_remote(tmp_path):
Raises:
DownloadError: If selected files are provided but no remote is configured.
"""
- repo = _repo(tmp_path)
+ repo = _repo(tmp_path, data=pd.DataFrame([{"path": "data.bin"}]))
with pytest.raises(DownloadError, match="No remote URL is configured"):
plan_download(repo, tmp_path, file_paths=["data.bin"])
diff --git a/test/test_hallmark.py b/test/test_hallmark.py
index 7a5da12..8d29378 100644
--- a/test/test_hallmark.py
+++ b/test/test_hallmark.py
@@ -1280,13 +1280,22 @@ def record_restore(sha1, destination):
### Repo.clone() tests ###
+def _remote_catalog(path, files):
+ """Commit a catalog of files kept on a data server, with SHA-1 checksums."""
+ repo = Repo.init(path)
+ repo.state.config = {"data": [{"db": "data.tsv"}],
+ "remote": {"url": "https://example.com/data/"}}
+ repo.state.data = pd.DataFrame([
+ {"path": name, "checksum_algorithm": "sha1", "checksum": checksum}
+ for name, checksum in files])
+ repo.dothm.save_state(repo.state)
+ repo.commit("add source data")
+ return repo
+
+
def test_repo_clone_downloads_after_plan_approval(monkeypatch, tmp_path):
- source = Repo.init(tmp_path / "source")
- _write_files(source.worktree, ["a0_i0.h5"])
- source.add("a{a}_i{i}.h5")
- source.set_config(remote_url="https://example.com/data/")
- expected_sha1 = Repo.checksum(source.worktree / "a0_i0.h5")
- source.commit("add source data")
+ expected_sha1 = "a" * 40
+ source = _remote_catalog(tmp_path / "source", [("a0_i0.h5", expected_sha1)])
captured = {}
@@ -1306,7 +1315,7 @@ def fake_download_file(context, relative_path, destination, sha1, chunk_size):
assert captured == {
"url": "https://example.com/data/a0_i0.h5",
- "sha1": expected_sha1,
+ "sha1": ("sha1", expected_sha1),
}
assert (clone.worktree / "a0_i0.h5").read_text(encoding="utf-8") == \
"downloaded\n"
@@ -1315,11 +1324,7 @@ def fake_download_file(context, relative_path, destination, sha1, chunk_size):
def test_repo_clone_can_skip_remote_data_download(monkeypatch, tmp_path):
- source = Repo.init(tmp_path / "source")
- _write_files(source.worktree, ["a0_i0.h5"])
- source.add("a{a}_i{i}.h5")
- source.set_config(remote_url="https://example.com/data/")
- source.commit("add source data")
+ source = _remote_catalog(tmp_path / "source", [("a0_i0.h5", "a" * 40)])
def fail_download(*args, **kwargs):
raise AssertionError("download should not be attempted")
@@ -2796,12 +2801,10 @@ def test_checkout_remote_branch(tmp_path):
source.commit("Initial commit")
- # Clone the source while it only has the main branch.
- clone = Repo.clone(
- str(source.dothm.path),
- tmp_path / "clone",
- download=False,
- )
+ # Clone the source while it only has the main branch. hm clone refuses
+ # repositories that track local files, so copy the history with Git.
+ GitRepo.clone_from(str(source.dothm.path), str(tmp_path / "clone" / ".hm"))
+ clone = Repo(tmp_path / "clone")
# The cloned repository should have the tracked file in its worktree.
(clone.worktree / "data.txt").write_text(
diff --git a/test/test_remote_add.py b/test/test_remote_add.py
index 3e26c3d..27e5c97 100644
--- a/test/test_remote_add.py
+++ b/test/test_remote_add.py
@@ -59,8 +59,10 @@ def test_remote_add_merges_paths_and_leaves_unmatched_rows(tmp_path, remote_list
repo.commit("Second selection")
repo.add(root, filter="*002.fits")
assert repo.dothm.index.diff("HEAD") == []
- repo.add(root, filter="absent*")
+ with pytest.raises(ValueError, match="No remote files matched"):
+ repo.add(root, filter="absent*")
assert repo.dothm.index.diff("HEAD") == []
+ assert repo.state.data.path.tolist() == ["M87_001.fits", "M87_002.fits"]
def test_remote_catalog_clone_and_branches_keep_payloads_separate(
@@ -223,3 +225,214 @@ def test_removed_cli_interfaces_fail_clearly(monkeypatch, tmp_path, arguments):
assert result.exit_code != 0
assert "No such" in result.output
assert Repo(repo.worktree).state.data.empty
+
+
+def _state_bytes(repo):
+ return {name: (repo.dothm.path / name).read_bytes()
+ for name in ("config.yml", "meta.yml", "data.tsv", ".git/index")}
+
+
+@pytest.mark.parametrize("url, message", [
+ ("https://user:secret@example.test/data/{a}.h5", "credentials"),
+ ("https://secret@example.test/data/{a}.h5", "credentials"),
+ ("http://secret@example.test/data/", "credentials"),
+ ("sftp://user:secret@example.test/data/", "credentials"),
+ ("https://example.test/data/{a}.h5?sig=secret", "query or fragment"),
+ ("https://example.test/data/?secret", "query or fragment"),
+ ("https://example.test/data/{a}.h5#secret", "query or fragment"),
+])
+def test_remote_add_rejects_secrets_in_urls_before_network(
+ tmp_path, remote_listing, url, message):
+ repo = Repo.init(tmp_path / "data")
+ before = _state_bytes(repo)
+ with pytest.raises(ValueError, match=message) as error:
+ repo.add(url)
+ assert "secret" not in str(error.value)
+ assert remote_listing[1] == []
+ assert _state_bytes(repo) == before
+
+
+def test_remote_pattern_keeps_ssh_usernames():
+ from hallmark.remote.add import split_remote_pattern
+
+ assert split_remote_pattern("ssh://researcher@campus/srv/data/{run}.h5") == (
+ "ssh://researcher@campus/srv/data/", "{run}.h5")
+
+
+@pytest.mark.parametrize("url", [
+ "https://user:secret@example.test/data/", "https://secret@example.test/data/",
+ "https://example.test/data/?token=secret",
+])
+def test_set_config_rejects_secrets_in_remote_urls(tmp_path, url):
+ repo = Repo.init(tmp_path / "data")
+ before = _state_bytes(repo)
+ with pytest.raises(ValueError) as error:
+ repo.set_config(remote_url=url)
+ assert "secret" not in str(error.value)
+ assert _state_bytes(repo) == before
+ assert Repo(repo.worktree).state.config == repo.state.config
+
+
+@pytest.mark.parametrize("arguments", [
+ ["add", "https://user:hunter2@archive.test/ER2/{a}.h5"],
+ ["add", "https://hunter2@archive.test/ER2/"],
+ ["set-config", "--remote-url", "https://archive.test/ER2/?sig=hunter2"],
+])
+def test_cli_rejects_url_secrets_without_echoing_them(
+ tmp_path, monkeypatch, remote_listing, arguments):
+ repo = Repo.init(tmp_path / "repo")
+ monkeypatch.chdir(repo.worktree)
+ before = _state_bytes(repo)
+ result = CliRunner().invoke(hallmark, arguments)
+ assert result.exit_code != 0
+ assert "Error:" in result.output
+ assert "hunter2" not in result.output
+ assert remote_listing[1] == []
+ assert _state_bytes(repo) == before
+
+
+@pytest.mark.parametrize("url", [
+ "http://user:SECRET@git.example.test/src.git",
+ "https://SECRET@git.example.test/src.git",
+ "ssh://git:SECRET@git.example.test/src.git",
+ "https://SECRET@catalogs.example.test/published/",
+])
+def test_clone_rejects_credentials_before_creating_anything(
+ tmp_path, monkeypatch, url):
+ def reject(*args, **kwargs):
+ raise AssertionError("a source with credentials must not be contacted")
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", reject)
+ monkeypatch.setattr(OperationContext, "read_text", reject)
+ target = tmp_path / "copy"
+ with pytest.raises(ValueError, match="credentials") as error:
+ Repo.clone(url, target, download=False)
+ assert "SECRET" not in str(error.value)
+ assert not target.exists()
+ result = CliRunner().invoke(hallmark, [
+ "clone", url, str(target), "--no-download"])
+ assert result.exit_code != 0
+ assert "credentials" in result.output
+ assert "SECRET" not in result.output
+ assert not target.exists()
+
+
+def test_clone_keeps_ssh_usernames(tmp_path, monkeypatch):
+ from hallmark.error import CloneError
+
+ calls = []
+
+ def stop(url, *args, **kwargs):
+ calls.append(url)
+ raise CloneError("stop after the URL check")
+
+ monkeypatch.setattr("hallmark.remote.clone.Dothm.clone", stop)
+ url = "ssh://git@git.example.test/team/src.git"
+ with pytest.raises(CloneError, match="stop after the URL check"):
+ Repo.clone(url, tmp_path / "copy", download=False)
+ assert calls == [url]
+
+
+def test_cli_remote_add_matching_nothing_keeps_previous_catalog(
+ tmp_path, monkeypatch, remote_listing):
+ repo = Repo.init(tmp_path / "repo")
+ repo.add("https://example.test/data/", filter="*.fits")
+ monkeypatch.chdir(repo.worktree)
+ before = _state_bytes(repo)
+ result = CliRunner().invoke(hallmark, [
+ "add", "https://example.test/data/", "--filter", "absent*"])
+ assert result.exit_code == 1
+ assert "No remote files matched" in result.output
+ assert "absent*" in result.output
+ assert "previous catalogue kept" in result.output
+ assert _state_bytes(repo) == before
+
+
+def test_failed_remote_scan_is_explained_and_keeps_previous_catalog(
+ tmp_path, monkeypatch, remote_listing):
+ from hallmark.transport.base import DownloadError
+
+ repo = Repo.init(tmp_path / "repo")
+ repo.add("https://example.test/data/", filter="*.fits")
+ before = _state_bytes(repo)
+
+ def refuse(*args, **kwargs):
+ raise DownloadError("Cannot establish connection to https://example.test")
+
+ monkeypatch.setattr("hallmark.remote.add.discover_remote_files", refuse)
+ with pytest.raises(DownloadError) as error:
+ repo.add("https://example.test/data/")
+ assert str(error.value) == (
+ "Remote scan failed (previous catalogue kept): "
+ "Cannot establish connection to https://example.test")
+ assert _state_bytes(repo) == before
+ assert repo.state.data.path.tolist() == ["M87_001.fits", "M87_002.fits"]
+
+
+def test_new_pattern_that_drops_catalogued_files_is_refused_offline(
+ tmp_path, remote_listing):
+ repo = Repo.init(tmp_path / "data")
+ root = "https://example.test/data/"
+ repo.add(root + "{src}_{day:03d}.fits")
+ repo.commit("Record fits files")
+ remote_listing[1].clear()
+ before = _state_bytes(repo)
+ with pytest.raises(ValueError) as error:
+ repo.add(root + "run{run:d}.fits")
+ message = str(error.value)
+ assert "M87_001.fits" in message and "M87_002.fits" in message
+ assert "no force option" in message
+ assert remote_listing[1] == []
+ assert _state_bytes(repo) == before
+ assert repo.state.config["data"] == [{"db": "data.tsv",
+ "fmt": "{src}_{day:03d}.fits"}]
+
+
+def test_broader_pattern_is_accepted_and_kept_rows_keep_their_information(
+ tmp_path, remote_listing):
+ pages, _ = remote_listing
+ digest = "a" * 64
+ pages[""] += 'SHA256SUMS'
+ pages["SHA256SUMS"] = f"{digest} M87_001.fits\n"
+ repo = Repo.init(tmp_path / "data")
+ root = "https://example.test/data/"
+ repo.add(root + "M87_{day:03d}.fits", filter="*001*")
+ repo.commit("Record the first file")
+ # The first file disappears from the server before the broader scan.
+ pages[""] = pages[""].replace(
+ 'M87_001.fits', "")
+ repo.add(root + "{src}_{day:03d}.fits")
+ assert repo.state.config["data"] == [{"db": "data.tsv",
+ "fmt": "{src}_{day:03d}.fits"}]
+ rows = repo.state.data.set_index("path")
+ assert rows.index.tolist() == ["M87_001.fits", "M87_002.fits"]
+ assert rows.loc["M87_001.fits", "checksum"] == digest
+ assert rows.loc["M87_001.fits", "checksum_algorithm"] == "sha256"
+ assert rows.loc["M87_001.fits", "src"] == "M87"
+ assert rows.loc["M87_002.fits", "src"] == "M87"
+ assert rows["day"].tolist() == ["1", "2"]
+
+
+def test_bare_url_reuses_the_saved_pattern(tmp_path, remote_listing):
+ repo = Repo.init(tmp_path / "data")
+ root = "https://example.test/data/"
+ repo.add(root + "{src}_{day:03d}.fits")
+ repo.commit("Record fits files")
+ repo.add(root)
+ assert repo.state.config["data"] == [{"db": "data.tsv",
+ "fmt": "{src}_{day:03d}.fits"}]
+ assert repo.state.data.path.tolist() == ["M87_001.fits", "M87_002.fits"]
+ assert repo.dothm.index.diff("HEAD") == []
+
+
+def test_pattern_refusal_lists_only_a_few_files():
+ import pandas as pd
+ from hallmark.repo.config import check_pattern_keeps_catalogue
+ from hallmark.repo.state import State
+
+ state = State(data=pd.DataFrame({"path": [f"f{i}.txt" for i in range(5)]}))
+ with pytest.raises(ValueError, match=r"5 catalogued file\(s\) "
+ r"\(f0.txt, f1.txt, f2.txt and 2 more\)"):
+ check_pattern_keeps_catalogue(state, "{name}.fits")
+ check_pattern_keeps_catalogue(state, "{name}.txt")
+ check_pattern_keeps_catalogue(state, None)
diff --git a/test/test_repo_open.py b/test/test_repo_open.py
new file mode 100644
index 0000000..c2f045a
--- /dev/null
+++ b/test/test_repo_open.py
@@ -0,0 +1,137 @@
+"""Test how existing repositories are located and opened."""
+
+from pathlib import Path
+
+import pytest
+from git.exc import GitError
+
+from hallmark import Repo
+from hallmark.error import DothmError
+
+
+def test_find_walks_parent_folders(tmp_path):
+ repo = Repo.init(tmp_path / "project")
+ nested = repo.worktree / "sub" / "dir"
+ nested.mkdir(parents=True)
+ for start in (repo.worktree, repo.worktree / "sub", nested):
+ found = Repo.find(start)
+ assert found.worktree == repo.worktree
+ assert found.dothm.path == repo.dothm.path
+
+
+def test_find_prefers_the_nearest_repository(tmp_path):
+ outer = Repo.init(tmp_path / "outer")
+ inner = Repo.init(outer.worktree / "inner")
+ (inner.worktree / "data").mkdir()
+ assert Repo.find(inner.worktree / "data").worktree == inner.worktree
+ assert Repo.find(outer.worktree).worktree == outer.worktree
+
+
+def test_find_opens_bare_repositories(tmp_path):
+ bare = Repo.init(tmp_path / "catalog.hm")
+ found = Repo.find(bare.dothm.path)
+ assert found.worktree is None
+ assert found.dothm.path == bare.dothm.path
+
+
+def test_find_explains_missing_repository(tmp_path):
+ folder = tmp_path / "plain" / "folder"
+ folder.mkdir(parents=True)
+ with pytest.raises(DothmError) as error:
+ Repo.find(folder)
+ message = str(error.value)
+ assert "Not a Hallmark repository (or any parent folder)" in message
+ assert "hm init" in message and "hm clone" in message
+ assert not (tmp_path / "plain" / ".hm").exists()
+
+
+def test_repo_path_stays_exact(tmp_path):
+ repo = Repo.init(tmp_path / "project")
+ (repo.worktree / "sub").mkdir()
+ with pytest.raises(GitError):
+ Repo(repo.worktree / "sub")
+ assert not (repo.worktree / "sub" / ".hm").exists()
+ assert Path(Repo(repo.worktree).worktree) == repo.worktree
+
+
+def _snapshot(root):
+ """Return the bytes of every file below root, keyed by relative path."""
+ return {path.relative_to(root).as_posix(): path.read_bytes()
+ for path in sorted(Path(root).rglob("*")) if path.is_file()}
+
+
+def _committed_local_repo(path):
+ repo = Repo.init(path)
+ (repo.worktree / "run1.txt").write_text("one")
+ repo.add("run{run:d}.txt")
+ repo.commit("Record run")
+ return repo
+
+
+DAMAGE = {
+ "missing data.tsv": (lambda hm: (hm / "data.tsv").unlink(), "data.tsv"),
+ "missing config.yml": (lambda hm: (hm / "config.yml").unlink(), "config.yml"),
+ "missing meta.yml": (lambda hm: (hm / "meta.yml").unlink(), "meta.yml"),
+ "invalid config.yml": (
+ lambda hm: (hm / "config.yml").write_text("data: [\n"), "config.yml"),
+ "meta.yml list": (lambda hm: (hm / "meta.yml").write_text("- a\n"), "meta.yml"),
+ "ragged data.tsv": (
+ lambda hm: (hm / "data.tsv").write_text("sha1\trun\nx\t1\na\tb\tc\n"),
+ "data.tsv"),
+ "binary data.tsv": (
+ lambda hm: (hm / "data.tsv").write_bytes(b"sha1\n\xff\xfe\n"), "data.tsv"),
+ "empty data.tsv": (lambda hm: (hm / "data.tsv").write_bytes(b""), "data.tsv"),
+ "not a Git worktree": (
+ lambda hm: __import__("shutil").rmtree(hm / ".git"), "Git"),
+ "dangling .hm link": (
+ lambda hm: (__import__("shutil").rmtree(hm),
+ hm.symlink_to(hm.parent / "moved-away")),
+ "links to a folder that does not exist"),
+}
+
+
+@pytest.mark.parametrize("damage", sorted(DAMAGE))
+def test_damaged_repository_is_explained_and_left_unchanged(tmp_path, damage):
+ repo = _committed_local_repo(tmp_path / "project")
+ apply_damage, detail = DAMAGE[damage]
+ apply_damage(repo.dothm.path)
+ before = _snapshot(repo.worktree)
+ with pytest.raises(DothmError) as error:
+ Repo(repo.worktree)
+ message = str(error.value)
+ assert "is damaged" in message
+ assert detail in message
+ assert "nothing was changed" in message
+ assert _snapshot(repo.worktree) == before
+
+
+@pytest.mark.parametrize("damage", [
+ "missing data.tsv", "invalid config.yml", "dangling .hm link"])
+def test_cli_status_stops_on_damaged_repository(tmp_path, monkeypatch, damage):
+ from click.testing import CliRunner
+ from hallmark.cli import hallmark
+
+ repo = _committed_local_repo(tmp_path / "project")
+ DAMAGE[damage][0](repo.dothm.path)
+ (repo.worktree / "sub").mkdir()
+ monkeypatch.chdir(repo.worktree / "sub")
+ before = _snapshot(repo.worktree)
+ result = CliRunner().invoke(hallmark, ["status"])
+ assert result.exit_code == 1
+ assert "is damaged" in result.output
+ assert DAMAGE[damage][1] in result.output
+ assert "Traceback" not in result.output
+ assert _snapshot(repo.worktree) == before
+
+
+def test_opening_a_linked_worktree_writes_nothing(tmp_path):
+ repo = _committed_local_repo(tmp_path / "project")
+ repo.add_worktree("experiment")
+ linked = tmp_path / "experiment"
+ objects = linked / ".hm" / "objects"
+ assert objects.is_symlink()
+ assert objects.resolve() == (repo.dothm.path / "objects").resolve()
+ objects.unlink()
+ opened = Repo(linked)
+ assert not objects.exists() and not objects.is_symlink()
+ assert opened.objects.contains(opened.state.data.iloc[0]["sha1"])
diff --git a/test/test_ssh_integration.py b/test/test_ssh_integration.py
index c5b272b..afa8e18 100644
--- a/test/test_ssh_integration.py
+++ b/test/test_ssh_integration.py
@@ -175,6 +175,8 @@ def test_explicit_endpoint_and_missing_file(ssh_server, tmp_path):
output.write_bytes(b"keep")
result = download_selection(repo, repo.worktree, [(Path('missing'), None)])
assert result["failed"] == 1
+ # The transfer itself fails; the existing-file guard is never reached.
+ assert "appeared after" not in result["errors"][0]
assert output.read_bytes() == b"keep"
assert not list(Path(repo.worktree).glob("*.part"))
@@ -403,6 +405,8 @@ def test_permission_denied_preserves_existing_file(ssh_server, tmp_path):
try:
result = download_selection(repo, repo.worktree, [(Path('private'), None)])
assert result["failed"] == 1
+ # The transfer itself fails; the existing-file guard is never reached.
+ assert "appeared after" not in result["errors"][0]
assert destination.read_bytes() == b"original"
assert not list(Path(repo.worktree).glob("*.part"))
finally:
@@ -444,12 +448,14 @@ def slow_prepare(self):
for transport in transports:
transport.context.cancel()
assert result["failed"] == 1
+ # The transfer itself fails; the existing-file guard is never reached.
+ assert "appeared after" not in result["errors"][0]
assert destination.read_bytes() == b"original"
assert not list(Path(repo.worktree).glob("*.part"))
assert not transports[0]._processes
-def test_add_commit_download_clone_workflow(ssh_server, tmp_path):
+def test_add_commit_download_clone_workflow(ssh_server, tmp_path, monkeypatch):
root = ssh_server["root"]
(root / "nested").mkdir()
(root / "nested/item_1.dat").write_bytes(b"science")
@@ -467,6 +473,17 @@ def test_add_commit_download_clone_workflow(ssh_server, tmp_path):
assert (repo.worktree / "nested/item_1.dat").read_bytes() == b"science"
assert not (repo.worktree / "bad_2.dat").exists()
assert not list(repo.worktree.rglob("*.part"))
+ downloaded = (repo.worktree / "nested/item_1.dat").stat()
+ monkeypatch.chdir(repo.worktree)
+ result = CliRunner().invoke(hallmark, ["download", "--all"], input="y\n")
+ # The verified file is skipped; only the failed one is tried again.
+ assert "1 file(s) already downloaded; skipped" in result.output
+ assert "Failed to download 1 file(s)" in result.output
+ assert "bad_2.dat" in result.output
+ assert (repo.worktree / "nested/item_1.dat").stat().st_mtime_ns == \
+ downloaded.st_mtime_ns
+ assert (repo.worktree / "nested/item_1.dat").stat().st_ino == downloaded.st_ino
+ monkeypatch.chdir(tmp_path)
clone = Repo.clone(str(repo.dothm.path), tmp_path / "python-clone", download=False)
assert clone.dothm.head.commit.hexsha == repo.dothm.head.commit.hexsha
assert not (clone.worktree / "nested").exists()