CI template: check a parity repository out outside the clone (#48)

docs/adopting.md

@@ -121,7 +121,13 @@ pull requests, and the full list is required on the default branch and before a
 job checks out the whole history so that the merge base with the base branch is found. When the
 base cannot be resolved (a missing or empty base branch name, or no merge base in the history),
 the pull-request job fails with the refusal before any check runs rather than falling back to the
-whole list; a push to the default branch runs the whole list whatever the mapping says. Its handover job rejects removals from boundary paths unless the pull
+whole list; a push to the default branch runs the whole list whatever the mapping says. Any
+untracked path that is not ignored and that a workflow step creates inside the clone (a checkout of
+another repository for a parity step, a download, a build output) is a changed path to `--changed`:
+unless a rule maps it, every pull request runs the whole list. Such a path must live outside the
+clone or be listed in `.gitignore`; the template's commented parity example checks the other
+repository out and moves it under the runner's temporary directory before any check runs, and the
+selection report names such a path as untracked and not mapped. Its handover job rejects removals from boundary paths unless the pull
 request body contains `Boundary change: yes`, then runs `handover.schema_probe` when present through
 the vendored `scripts/dev/handover_check.sh`. Copy
 `templates/update-branches.yml` to `.github/workflows/update-branches.yml` to update open branches
@@ -175,7 +181,9 @@ node on PATH (the kit puts the manifest's node first on PATH for every step; the
 with `--disable-wasm-trap-handler`, which is what lets it run under the Go address-space cap).
 Its last check, `parity`, compares the package with a checkout of the Desk and is skipped
 unless `JPDEV_DESK` names one; it is marked `optional` because that repository's CI does not
-check the Desk out.
+check the Desk out. A repository whose CI does check the other repository out keeps that checkout
+outside the clone (see [step 4](#4-add-the-ci-workflow)), or `--changed` runs the whole list on
+every pull request.
 
 To run it against a clone of that repository:
 
@@ -244,5 +252,9 @@ no mapping (no `jpdev.json`, or one without `changed`). Map only paths whose eff
 known, and put the checks that read every file (whitespace, path and name checks) in every rule.
 Selection sees only what git reports in this clone: an edit hidden by assume-unchanged or
 skip-worktree, an ignored file and a path outside the clone are not seen, and the full list remains
-the guard for them. The [policy](policy.md#testing) says where a scoped run is allowed and where the
+the guard for them. Conversely, any untracked path that is not ignored is seen, including one a CI
+workflow creates inside the clone: unless a rule maps it, it defeats `--changed` on every run, so a
+path a workflow creates belongs outside the clone or in `.gitignore`. The report names an unmapped
+untracked path as `untracked path PATH is not mapped; a workflow-created path belongs outside the
+clone or in .gitignore: whole list`. The [policy](policy.md#testing) says where a scoped run is allowed and where the
 full list is required.

lib/changed_checks.py

@@ -15,7 +15,8 @@
 `?` stay within one path segment and a `**` segment matches any number of segments. The whole
 list runs when a changed path matches no rule, when a selection input changed (the mapping file,
 the manifest, scripts/dev/check.sh, scripts/dev/changed_checks.py), when nothing changed, or when
-there is no mapping.
+there is no mapping. An unmapped untracked path is named as such: a path a workflow creates
+inside the clone defeats the selection on every run and belongs outside the clone or in .gitignore.
 
 The first line of the output is the selection, `whole` or `only NAME,NAME`; the lines after it
 are the report the runner prints, headed and refused with --label (default `jpdev check
@@ -38,6 +39,9 @@
 RULE_MEMBERS = {'paths', 'checks'}
 UNSUPPORTED = '\\[]{}'
 SHOWN = 20
+UNMAPPED = 'unmapped path %s: whole list'
+UNTRACKED = ('untracked path %s is not mapped; a workflow-created path belongs outside the clone or in '
+             '.gitignore: whole list')
 
 
 class Refused(Exception):
@@ -159,7 +163,8 @@ def resolve_base(root, base):
 
 
 def changed_paths(root, merge_base):
-    """The paths HEAD, the index or the working tree changed since the merge base, and untracked."""
+    """Return (the paths HEAD, the index or the working tree changed since the merge base or that are
+    untracked, sorted; the untracked ones among them)."""
     raw = set()
     for side, args in (('the working tree', (merge_base,)), ('the index', ('--cached', merge_base)),
                        ('HEAD', (merge_base, 'HEAD'))):
@@ -170,8 +175,11 @@ def changed_paths(root, merge_base):
     status, untracked = git(root, 'ls-files', '--others', '--exclude-standard', '--full-name', '-z')
     if status:
         raise Refused('git ls-files of the untracked files failed')
-    raw |= set(untracked.split(b'\0'))
-    return sorted(p.decode('utf-8', 'replace') for p in raw - {b''})
+    # Untracked: a path only ls-files --others names, not one a diff names too.
+    others = set(untracked.split(b'\0')) - raw
+    raw |= others
+    return (sorted(p.decode('utf-8', 'replace') for p in raw - {b''}),
+            {p.decode('utf-8', 'replace') for p in others - {b''}})
 
 
 def selection_inputs(root, manifest):
@@ -198,7 +206,7 @@ def select(root, manifest, base=None, runner='jpdev check --changed'):
     steps = step_names(manifest)
     rules, no_mapping = load_rules(root, steps, os.path.basename(manifest))
     base_label, merge_base = resolve_base(root, base)
-    paths = changed_paths(root, merge_base)
+    paths, untracked = changed_paths(root, merge_base)
     short = git(root, 'rev-parse', '--short', merge_base)[1].decode().strip() or merge_base[:12]
     noun = 'path' if len(paths) == 1 else 'paths'
     report = ['== %s: %d changed %s since %s (merge base %s) ==' % (runner, len(paths), noun, base_label, short)]
@@ -224,7 +232,7 @@ def select(root, manifest, base=None, runner='jpdev check --changed'):
                     reasons.setdefault(name, set()).update(matched)
             if not hit:
                 unmapped.append(path)
-        why = ['unmapped path %s: whole list' % path for path in unmapped[:SHOWN]]
+        why = [(UNTRACKED if path in untracked else UNMAPPED) % path for path in unmapped[:SHOWN]]
         if len(unmapped) > SHOWN:
             why.append('... %d more unmapped paths' % (len(unmapped) - SHOWN))
         if not why and not reasons:

scripts/dev/changed_checks.py

@@ -16,7 +16,8 @@
 `?` stay within one path segment and a `**` segment matches any number of segments. The whole
 list runs when a changed path matches no rule, when a selection input changed (the mapping file,
 the manifest, scripts/dev/check.sh, scripts/dev/changed_checks.py), when nothing changed, or when
-there is no mapping.
+there is no mapping. An unmapped untracked path is named as such: a path a workflow creates
+inside the clone defeats the selection on every run and belongs outside the clone or in .gitignore.
 
 The first line of the output is the selection, `whole` or `only NAME,NAME`; the lines after it
 are the report the runner prints, headed and refused with --label (default `jpdev check
@@ -39,6 +40,9 @@
 RULE_MEMBERS = {'paths', 'checks'}
 UNSUPPORTED = '\\[]{}'
 SHOWN = 20
+UNMAPPED = 'unmapped path %s: whole list'
+UNTRACKED = ('untracked path %s is not mapped; a workflow-created path belongs outside the clone or in '
+             '.gitignore: whole list')
 
 
 class Refused(Exception):
@@ -160,7 +164,8 @@ def resolve_base(root, base):
 
 
 def changed_paths(root, merge_base):
-    """The paths HEAD, the index or the working tree changed since the merge base, and untracked."""
+    """Return (the paths HEAD, the index or the working tree changed since the merge base or that are
+    untracked, sorted; the untracked ones among them)."""
     raw = set()
     for side, args in (('the working tree', (merge_base,)), ('the index', ('--cached', merge_base)),
                        ('HEAD', (merge_base, 'HEAD'))):
@@ -171,8 +176,11 @@ def changed_paths(root, merge_base):
     status, untracked = git(root, 'ls-files', '--others', '--exclude-standard', '--full-name', '-z')
     if status:
         raise Refused('git ls-files of the untracked files failed')
-    raw |= set(untracked.split(b'\0'))
-    return sorted(p.decode('utf-8', 'replace') for p in raw - {b''})
+    # Untracked: a path only ls-files --others names, not one a diff names too.
+    others = set(untracked.split(b'\0')) - raw
+    raw |= others
+    return (sorted(p.decode('utf-8', 'replace') for p in raw - {b''}),
+            {p.decode('utf-8', 'replace') for p in others - {b''}})
 
 
 def selection_inputs(root, manifest):
@@ -199,7 +207,7 @@ def select(root, manifest, base=None, runner='jpdev check --changed'):
     steps = step_names(manifest)
     rules, no_mapping = load_rules(root, steps, os.path.basename(manifest))
     base_label, merge_base = resolve_base(root, base)
-    paths = changed_paths(root, merge_base)
+    paths, untracked = changed_paths(root, merge_base)
     short = git(root, 'rev-parse', '--short', merge_base)[1].decode().strip() or merge_base[:12]
     noun = 'path' if len(paths) == 1 else 'paths'
     report = ['== %s: %d changed %s since %s (merge base %s) ==' % (runner, len(paths), noun, base_label, short)]
@@ -225,7 +233,7 @@ def select(root, manifest, base=None, runner='jpdev check --changed'):
                     reasons.setdefault(name, set()).update(matched)
             if not hit:
                 unmapped.append(path)
-        why = ['unmapped path %s: whole list' % path for path in unmapped[:SHOWN]]
+        why = [(UNTRACKED if path in untracked else UNMAPPED) % path for path in unmapped[:SHOWN]]
         if len(unmapped) > SHOWN:
             why.append('... %d more unmapped paths' % (len(unmapped) - SHOWN))
         if not why and not reasons:

templates/ci-check.yml

@@ -77,7 +77,11 @@ jobs:
       # A step with requires_env fails under CI when its variable is empty, unless the step is
       # marked optional in dev.json. A repository whose dev.json has a parity step sets the
       # variable here: a shallow checkout of the other repository, then the variable pointing at
-      # it (a private repository needs a token with read access). Uncomment and adapt:
+      # it (a private repository needs a token with read access). The checkout must end up OUTSIDE
+      # this clone: an untracked, un-ignored path inside it is a changed path to `check.sh
+      # --changed`, which then runs the whole list on every pull request. actions/checkout places
+      # a checkout only under the workspace, so the step after it moves it under the runner's
+      # temporary directory before any check runs. Uncomment and adapt:
       #
       # - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       #   with:
@@ -87,8 +91,12 @@ jobs:
       #     fetch-depth: 1
       #     persist-credentials: false
       #     token: ${{ secrets.DESK_READ_TOKEN }}
-      # - name: Name the checkout for the parity step
-      #   run: echo "JPDEV_DESK=$GITHUB_WORKSPACE/desk-checkout" >> "$GITHUB_ENV"
+      # - name: Move the checkout outside the clone and name it for the parity step
+      #   env:
+      #     OUTSIDE: ${{ runner.temp }}/desk-checkout
+      #   run: |
+      #     mv desk-checkout "$OUTSIDE"
+      #     echo "JPDEV_DESK=$OUTSIDE" >> "$GITHUB_ENV"
       - name: Setup steps
         run: scripts/dev/check.sh --setup
       - name: Checks

tests/changed_checks_test.py

@@ -18,6 +18,7 @@
 c = importlib.util.module_from_spec(SPEC)
 SPEC.loader.exec_module(c)
 ALL = {'docs-lint', 'go-vet', 'go-test', 'python-tests', 'whitespace'}
+WORKFLOW_PATH = 'a workflow-created path belongs outside the clone or in .gitignore: whole list'
 # What jpdev check exports to its steps, and settings a caller's machine may hold.
 DROPPED = ('CI', 'JPDEV_ROOT', 'JPDEV_WRAP', 'JPDEV_NODE_BIN', 'JPDEV_GO_BIN', 'JPDEV_LOGDIR',
            'JPDEV_STEP_TIMEOUT', 'JPDEV_VENV', 'JPDEV_NAME_PATTERNS', 'JPDEV_GO_SDK_DIR', 'JPDEV_NVM_DIR')
@@ -166,10 +167,37 @@ def test_unmapped_path_runs_the_whole_list(self):
         self.write('scripts/new.sh', 'true\n')
         status, out, err = self.check('--changed')
         self.assertEqual(status, 0, out + err)
-        self.assertIn('unmapped path scripts/new.sh: whole list', out)
+        self.assertIn('untracked path scripts/new.sh is not mapped; ' + WORKFLOW_PATH, out)
         self.assertIn('== whole list: 5 checks ==', out)
         self.assertEqual(self.ran(out), ALL, out)
 
+    def test_an_unmapped_untracked_path_says_where_a_workflow_path_belongs(self):
+        # A workflow that checks another repository out inside the clone leaves an untracked path.
+        self.write('docs/guide.md', 'guide, edited\n')
+        self.write('desk-checkout/package.json', '{}\n')
+        self.write('scripts/new.sh', 'true\n')
+        git(self.clone, 'add', 'scripts/new.sh')
+        status, out, err = self.check('--changed')
+        self.assertEqual(status, 0, out + err)
+        self.assertIn('\nuntracked path desk-checkout/package.json is not mapped; a workflow-created path '
+                      'belongs outside the clone or in .gitignore: whole list\n', out)
+        self.assertNotIn('unmapped path desk-checkout/', out)
+        # A tracked unmapped path keeps the plain reason.
+        self.assertIn('\nunmapped path scripts/new.sh: whole list\n', out)
+        self.assertNotIn('untracked path scripts/new.sh', out)
+        self.assertIn('== whole list: 5 checks ==', out)
+        self.assertEqual(self.ran(out), ALL, out)
+
+    def test_an_ignored_workflow_path_leaves_the_selection_scoped(self):
+        self.write('.gitignore', 'desk-checkout/\n')
+        base = self.commit_base('ignore the workflow checkout')
+        self.write('desk-checkout/package.json', '{}\n')
+        self.write('docs/guide.md', 'guide, edited\n')
+        status, out, err = self.check('--changed', base)
+        self.assertEqual(status, 0, out + err)
+        self.assertNotIn('desk-checkout', out)
+        self.assertEqual(self.ran(out), {'docs-lint', 'whitespace'}, out)
+
     def test_paths_of_several_rules_select_the_union(self):
         self.write('docs/guide.md', 'guide, edited\n')
         self.write('tests/unit_test.py', 'assert True\n')