On Sun, Jan 17, 2021 at 11:55 AM Jeff King [off-list ref] wrote:

On Sat, Jan 16, 2021 at 04:35:45PM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

      for subtree in $subtrees
      do
+             (
+                     cd "$subtree"
+                     generate_expected_cache_tree_rec "$dir$subtree" || return 1
+             )
      done

We don't check that "cd" worked either before or after your patch.
Should we?

I'd say "yes".

After your patch, we "return" from inside a subshell. Is that portable?
ISTR issues around that before, but it just have been when we are not in
a function at all. Still, I wonder if:

  for ...
  do
        (
                cd "$subtree" &&
                generate_expected_cache_tree_rec "$dir$subtree"
        ) || return 1
  done

might be more obvious.

Yes, a good recommendation. Normally, we `exit 1` from within
subshells[1], but that wouldn't help us exit this loop early, so the
`|| return 1` outside the subshell seems a good solution.

[1]: https://lore.kernel.org/git/20150325052952.GE31924@peff.net/ (local)