latex3 / l3build

A testing and building system for LaTeX
LaTeX Project Public License v1.3c
84 stars 14 forks source link

Drop a shadowed `testdir` local to `then` body #342

Closed muzimuzhi closed 6 months ago

muzimuzhi commented 6 months ago

In 8949ef3 (Improve log for failed checks with no diff files, 2023-11-06), in checkdiff(config), I mistakenly shadowed testdir inside a then body (if ... then <then body> end). This makes the assignments to that testdir local to then body, hence its new value is not used by statements after if block (see https://www.lua.org/pil/4.2.html).

Wrong way

function checkdiff(config)
  if ... then
    local testdir = testdir
    testdir = testdir .. "-" .. config
  end
  print(testdir) -- uses outer "testdir"
end

Right way

function checkdiff(config)
  local testdir = testdir
  if ... then
    testdir = testdir .. "-" .. config
  end
  print(testdir) -- uses "testdir" local to this function
end

Recently this problem was found and fixed in 761a672 (Print failures correctly when these occur in multiple configs, 2024-01-04) and 29a6933 (Exclude "build" as a config, 2024-01-08). But the fixes leave two copies of similar logic in checkdiff(config), see lines 1060-1063 and 1068-1071.

This PR removes the second copy, the one inside then body. I decided to leave the assignment to local testdir at the top of function body, rather than inside nested if blocks, to make the function's main logic a little easier to understand.

https://github.com/latex3/l3build/blob/0d592910a068fb2e6b4db8d06ab5ae5944338b32/l3build-check.lua#L1059-L1072