fix: fluid split left a moved $ref pointing off, wrote over files in --out, and half-undid a refused split

CHANGELOG.md

@@ -57,6 +57,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   and docs link stay hidden.
 - **`fluid validate missing.fluid.yaml` said "Contract file not found: None"**; it names the
   file again.
+- **`fluid split` rebases a relative `$ref` inside a part it moves.** A ref such as
+  `sql: {$ref: ./sql/transform.sql}` was copied unchanged into `fragments/builds/transform.yaml`,
+  where it named a file one or two directories off, so split refused the contract, and a split in
+  two passes (`--parts sql`, then `--parts builds`) could not work. The ref now names the same file
+  from the fragment (`../../sql/transform.sql`, a `#/pointer` kept); a URL, an absolute path and a
+  same-document pointer are left as written.
+- **A refused `fluid split` leaves the tree as it was**, as its message says: it took back the
+  files it wrote but left the directories it made (an empty `fragments/` changed what other
+  commands did), and after a failed write the empty file. A read-only root ended in "Unexpected
+  error" (exit 2); it is now refused like any failed write (exit 1), as is a JSON root with a
+  character that has no UTF-8 form. An interrupt (Ctrl-C) halfway through the writes is taken
+  back too. The dry run refuses a symlinked directory on the way, as the real run already did.
+- **A part in a container a YAML alias or `<<` merge shares is split from its own place.** Two
+  builds merging the same defaults (`<<: *defaults`) made `fluid split --parts sql` fail with
+  "write() argument must be str, not dict" and leave an empty `sql/b.sql`; each build's sql is now
+  a file of its own.
+- **`fluid split --out DIR` never writes over another file.** An existing `DIR/contract.fluid.yaml`
+  that is not the contract being split (or a byte-for-byte copy of it that is no hard link to
+  another file) was replaced without a word; a second split into the same `--out` wrote `-2`
+  copies and left the first run's fragments unreferenced. Both are refused now, the dry run
+  included, with the remedy.
+- **A long id gets a file name a file system takes.** An id of 300 characters gave a 305-byte
+  name: the dry run listed it and the real run failed. A slug over 100 characters now ends in 8 hex
+  digits of a hash of the whole id, so two long ids that start alike keep names of their own.
+- **`fluid split` keeps more of the root as written:** a comment on the last line of a moved block
+  keeps the line's CR in a CRLF file; a keep-chomped block scalar (`|+`, `>+`) keeps its trailing
+  blank lines; a tag or an unused anchor on a moved section's key line no longer makes the splice
+  write unparseable YAML; and a comment on a block scalar's header line stays beside its `$ref`.
 
 ### Upgrade notes
 

fluid_build/cli/_split_text.py

@@ -19,39 +19,75 @@
 lines. Each moved part takes its own text to its fragment, comments inside
 it included, and a ``$ref`` takes its place. A part inside another (a
 build's SQL) is cut out of its container's text first, so the container's
-fragment holds the ref to it.
+fragment holds the ref to it. A relative ``$ref`` already in a moved part is
+rewritten in its text to name the same file from the fragment, in the
+quoting it was written in where the new path allows it: a plain one gets
+double quotes when the path needs them (a ``#`` or a blank in it). The
+comment on a block scalar's header line (``sql: |  # the query``) stays
+beside the ref that replaces the scalar: a text file cannot hold it.
 
 PyYAML's compose marks give each part's span; PyYAML is a base dependency,
 so no round-trip library is needed. A block node's end mark runs past its
 trailing comments and the next entry's indentation, so a span ends at the
 last character of the node's own content instead, plus a comment that ends
-its last line.
+its last line. The blank lines after a keep-chomped block scalar (``|+``)
+are its content, so they move with it. A node's start mark is at its tag or
+anchor, when it has one: written on its key's line, the ref takes the key's
+line, as a flow mapping.
 """
 
 from __future__ import annotations
 
+import re
 from typing import Any, Dict, List, Optional, Sequence, Tuple
 
 import yaml
 
-from fluid_build.cli.forge_contract_fragments import FragmentSite, container_of, ref_between
+from fluid_build.cli.forge_contract_fragments import (
+    FragmentSite,
+    container_of,
+    rebased_ref,
+    ref_between,
+)
 from fluid_build.util.safe_yaml import compose_yaml_safe
 
 __all__ = ["SpliceRefused", "split_yaml_text"]
 
+#: The comment on a block scalar's header line, the blanks before it included:
+#: a ``#`` that follows a blank, so a ``#`` inside a tag is not one. A match
+#: starts only where a run of blanks starts: tried from every blank of a long
+#: run (a line of blanks with no ``#``), the search took time quadratic in it.
+_HEADER_COMMENT = re.compile(r"(?<![ \t])[ \t]+#.*$")
+
+#: The tag of a node that parses as a string.
+_STR = "tag:yaml.org,2002:str"
+
+#: A line width no scalar reaches: the dumper breaks none over two lines.
+_ONE_LINE = 2**31 - 1
+
 
 class SpliceRefused(Exception):
     """The text cannot be spliced safely; the caller splits the parsed contract instead."""
 
 
-class _Cut:
-    def __init__(self, site: FragmentSite, node: Any, start: int, end: int, replacement: str):
-        self.site = site
-        self.node = node
+class _Edit:
+    """A span of the text and what takes its place."""
+
+    def __init__(self, start: int, end: int, replacement: str):
         self.start = start
         self.end = end
         self.replacement = replacement
-        self.children: List["_Cut"] = []
+
+
+class _Cut(_Edit):
+    """A part that moves: its span, the ref that takes its place, and the edits
+    inside it (the parts cut out of it, and its rebased refs)."""
+
+    def __init__(self, site: FragmentSite, node: Any, start: int, end: int, replacement: str):
+        super().__init__(start, end, replacement)
+        self.site = site
+        self.node = node
+        self.children: List[_Edit] = []
 
 
 def split_yaml_text(
@@ -63,8 +99,9 @@ def split_yaml_text(
 
     Raises:
         SpliceRefused: the text holds YAML aliases (moving an anchor or an
-            alias would change what the other one means), or a site is not
-            where the parsed contract says it is.
+            alias would change what the other one means), a site is not
+            where the parsed contract says it is, or a ``$ref`` to rebase is
+            a block scalar.
     """
     if _has_aliases(text):
         raise SpliceRefused("the contract uses YAML aliases")
@@ -92,6 +129,7 @@ def split_yaml_text(
         if cut.site.text:
             fragments[cut.site.target] = _value(contract, cut.site.path)
         else:
+            cut.children += _rebased_refs(cut)
             fragments[cut.site.target] = _fragment(text, cut)
     return _render(text, 0, len(text), top), fragments
 
@@ -137,8 +175,12 @@ def _span(
     end = _content_end(text, node)
     flow = "{$ref: " + _yaml_string(ref) + "}"
     if is_text or isinstance(node, yaml.ScalarNode) or node.flow_style:
-        return start, end, flow
+        return start, end, flow + _header_comment(text, node)
     end = _with_trailing_comment(text, end)
+    if key is not None and node.start_mark.line == key.start_mark.line:
+        # A tag or an anchor on the key's line ("key: &a", the mapping below):
+        # they go to the fragment with the rest, and the ref takes their place.
+        return start, end, flow
     if key is not None and node.start_mark.column <= key.start_mark.column:
         # A block sequence at its key's own indentation ("key:\n- item"): a
         # mapping in its place must sit deeper than the key.
@@ -147,11 +189,30 @@ def _span(
     return start, end, "$ref: " + _yaml_string(ref)
 
 
+def _header_comment(text: str, node: Any) -> str:
+    """The comment on a block scalar's header line, as written; ``""`` for none.
+
+    The ref that replaces the scalar keeps it: a text file cannot hold it.
+    """
+    if not (isinstance(node, yaml.ScalarNode) and node.style in ("|", ">")):
+        return ""
+    start = node.start_mark.index
+    newline = text.find("\n", start)
+    header = text[start : len(text) if newline < 0 else newline].rstrip("\r")
+    found = _HEADER_COMMENT.search(header)
+    return found.group(0) if found else ""
+
+
 def _content_end(text: str, node: Any) -> int:
     """Just past the last character of ``node``'s own content."""
     if isinstance(node, yaml.ScalarNode):
         end = node.end_mark.index
         if node.style in ("|", ">"):
+            if node.value.endswith("\n\n"):
+                # Keep chomping (|+, >+): the blank lines after the text are
+                # in its value and in its span; the line break that ends the
+                # last of them ends the line of whatever replaces the scalar.
+                return _before_line_break(text, end)
             # A block scalar's end mark runs over the blank lines after it.
             start = node.start_mark.index
             end = start + len(text[start:end].rstrip())
@@ -167,28 +228,106 @@ def _content_end(text: str, node: Any) -> int:
     return max(_content_end(text, child) for child in children)
 
 
+def _before_line_break(text: str, end: int) -> int:
+    """``end``, moved back over the line break just before it (``\\n`` or ``\\r\\n``)."""
+    if text.endswith("\r\n", 0, end):
+        return end - 2
+    if text.endswith("\n", 0, end):
+        return end - 1
+    return end
+
+
 def _with_trailing_comment(text: str, end: int) -> int:
-    """``end``, moved over a comment that ends the same line."""
+    """``end``, moved over a comment that ends the same line, not over its line break."""
     newline = text.find("\n", end)
     line_end = len(text) if newline < 0 else newline
+    if line_end > end and text[line_end - 1] == "\r":
+        line_end -= 1
     rest = text[end:line_end]
     return line_end if rest.strip().startswith("#") else end
 
 
+def _rebased_refs(cut: _Cut) -> List[_Edit]:
+    """An edit for each ``$ref`` in the part ``cut`` moves (not in a part cut out
+    of it) that names a file relative to the root: it is rewritten to name that
+    file from the part's own file (:func:`rebased_ref`), in the quoting it had
+    where the new path allows it (:func:`_ref_edit`)."""
+    inner = {id(child.node) for child in cut.children if isinstance(child, _Cut)}
+    edits: List[_Edit] = []
+    stack = [cut.node]
+    while stack:
+        node = stack.pop()
+        if id(node) in inner:
+            continue
+        if isinstance(node, yaml.SequenceNode):
+            stack.extend(node.value)
+        elif isinstance(node, yaml.MappingNode):
+            for key, value in node.value:
+                if _is_ref_value(key, value):
+                    ref = rebased_ref(value.value, cut.site.target)
+                    if ref != value.value:
+                        edits.append(_ref_edit(value, ref))
+                else:
+                    stack.append(value)
+    return edits
+
+
+def _is_ref_value(key: Any, value: Any) -> bool:
+    """``key: value`` is a ``$ref`` whose value is a string."""
+    return (
+        isinstance(key, yaml.ScalarNode)
+        and key.tag == _STR
+        and key.value == "$ref"
+        and isinstance(value, yaml.ScalarNode)
+        and value.tag == _STR
+    )
+
+
+def _ref_edit(node: Any, ref: str) -> _Edit:
+    """The edit that writes ``ref`` over the scalar ``node``."""
+    if node.style in ("|", ">"):
+        # A block scalar's span runs into the lines after it; the data split
+        # rewrites the ref instead.
+        raise SpliceRefused("a $ref value is written as a block scalar")
+    if node.style == "'" and ref.isprintable():
+        text = "'" + ref.replace("'", "''") + "'"
+    elif node.style == '"':
+        text = _double_quoted(ref)
+    else:
+        text = _yaml_string(ref)
+    return _Edit(node.start_mark.index, node.end_mark.index, text)
+
+
 def _yaml_string(value: str) -> str:
     """``value`` as a YAML scalar: plain when it is safe, quoted otherwise."""
     if value and all(c.isalnum() or c in "._/-" for c in value):
         return value
-    return yaml.safe_dump(value, default_style='"').strip()
+    return _double_quoted(value)
 
 
-def _render(text: str, lo: int, hi: int, cuts: Sequence[_Cut]) -> str:
+def _double_quoted(value: str) -> str:
+    """``value`` as a double-quoted YAML scalar, on one line however long it is.
+
+    A value that is all printable keeps its letters that are not ASCII
+    (``é``), as the contract had them. One that holds anything else is escaped
+    whole, as the single-quoted rule has it: PyYAML would write a bidi control
+    (U+202E), a zero-width or soft hyphen, a private-use or unassigned code
+    point as it is, and the line would show other text than it holds (Trojan
+    Source). NEL, LS and PS are escaped either way, so the value reads back
+    exactly.
+    """
+    return yaml.safe_dump(
+        value, default_style='"', width=_ONE_LINE, allow_unicode=value.isprintable()
+    ).strip()
+
+
+def _render(text: str, lo: int, hi: int, edits: Sequence[_Edit]) -> str:
     out: List[str] = []
     pos = lo
-    for cut in sorted(cuts, key=lambda c: c.start):
-        out.append(text[pos : cut.start])
-        out.append(cut.replacement)
-        pos = cut.end
+    for edit in sorted(edits, key=lambda e: e.start):
+        out.append(text[pos : edit.start])
+        out.append(edit.replacement)
+        pos = edit.end
     out.append(text[pos:hi])
     return "".join(out)
 
@@ -202,14 +341,26 @@ def _value(document: Any, path: Sequence[Any]) -> Any:
 def _fragment(text: str, cut: _Cut) -> str:
     """A YAML part's file: its text, moved to column 0, under its header."""
     body = _render(text, cut.start, cut.end, cut.children)
-    column = cut.node.start_mark.column
     lines = body.split("\n")
+    column = cut.node.start_mark.column
+    if text.startswith(("&", "!"), cut.start) and not isinstance(cut.node, yaml.ScalarNode):
+        # A tag or an anchor starts the part: its entries are indented as
+        # the first line after it that is not blank or a comment.
+        column = next((_width(line) for line in lines[1:] if _is_content(line)), column)
     lines[1:] = [_dedent(line, column) for line in lines[1:]]
     newline = "\r\n" if "\r\n" in text else "\n"
     header = cut.site.header.replace("\n", newline) + newline if cut.site.header else ""
     return header + "\n".join(lines).rstrip("\r") + newline
 
 
+def _is_content(line: str) -> bool:
+    stripped = line.strip()
+    return bool(stripped) and not stripped.startswith("#")
+
+
+def _width(line: str) -> int:
+    return len(line) - len(line.lstrip(" "))
+
+
 def _dedent(line: str, column: int) -> str:
-    width = len(line) - len(line.lstrip(" "))
-    return line[min(width, column) :]
+    return line[min(_width(line), column) :]