Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recent change seems to have rendered documented way to call biber not working #327

Closed
gusbrs opened this issue Dec 15, 2023 · 16 comments
Closed

Comments

@gusbrs
Copy link

gusbrs commented Dec 15, 2023

I think the commit responsible is 4d8e5d9 .

In practice, I observed all of my tests involving biblatex to fail (they had been working a couple of days prior). Diffs and some investigation suggested biber was not being called at all.

My build.lua is pretty standard in that regard, and follows almost to the letter the documented recipe for the purpose:

function runtest_tasks (name,run)
  if run == 1 then
    return "biber " .. name
  else
    return "" 
  end
end

I modified this to include:

function runtest_tasks(name, run)
  local run = run or 1
  if run == 0 then
    run = 1
  end
  if run == 1 then
    return "biber " .. name
  else
    return ""
  end
end

With this change the tests passed again (meaning biber ran).

So the fact that runtest_tasks is being called with args ("X",0) makes if run == 1 return false.

Also, in my actual use case, I also test for the name arg, with:

function runtest_tasks(name, run)
  if run == 1 and string.match(name, "^pn%-biblatex") then
    return "biber -q " .. name
  else
    return ""
  end
end

In this case, even if I force run = 1 as above that fails again (biber is not called) because, not surprisingly, "X" does not match the name of the test.

@josephwright
Copy link
Member

I was using 'filler' text here as it seemed safe, but we can change that. But that still leaves the issue that if you are trying in the hook to save state changes, we can't check if it's got an empty return value. Should we document something?

@gusbrs
Copy link
Author

gusbrs commented Dec 15, 2023

I'm not sure I understand the reason for the commit, but if the comment:

This avoids an unrequired shell call.

is the reason, shouldn't you use the same arguments as the actual call?

    if errlevels[i] == 0 and runtest_tasks(jobname(lvtfile),i) ~= "" then
      local errorlevel =
        runcmd(preamble .. runtest_tasks(jobname(lvtfile),i),testdir)
      if errorlevel ~= 0 then errlevels[i] = errorlevel end
    end

I'm not sure what you were trying to do, or what you mean by "if you are trying in the hook to save state changes" (I assume you have a use case in mind). I just know the change rendered the documented way of doing things not to work anymore, and that's what I reported.

@gusbrs
Copy link
Author

gusbrs commented Dec 15, 2023

Thank you!

@josephwright
Copy link
Member

I'd forgotten we'd documented that run could be used in a conditional - sorry about that.

@moewew
Copy link

moewew commented Dec 20, 2023

I just (well, the test started failing a few days ago, but I just got round to looking at it) ran into a very similar issue with my biblatex-ext tests. Evidently Biber is not being called. I didn't manage to get any sensible redefinition of runtest_tasks going. Is there something else that changed here?


I even tried

function runtest_tasks (name,run)
  return "biber " .. name
end

but no joy.

@gusbrs
Copy link
Author

gusbrs commented Dec 20, 2023

@moewew Theoretically, it should be working, as long as you have the latest update. The fix did work. However, in my experience, after I got the update, most of my biblatex related tests were back to normal (meaning biber is running), but some of them did not. Tracking it down, it turned out that biber was not running if some error occurred along the way. In my case I had some stray \counterwithin*{postnote}{chapter} in tests in the article class. They were completely unrelated to the tests, occurring before \START with no real effect on the document, but the error they generated prevented somehow biber from running (I don't understand why really, or why this behavior changed...). Once I fixed those errors, the remaining tests were back to working (see gusbrs/postnotes@cd5f164). Perhaps you have something of the sort on your side?

@josephwright
Copy link
Member

@moewew, @gusbrs What's happening here is that we needed to split the test run into the main part and (optionally) the extra tasks; in the past the extra tasks always ran. So if the main run gives a non-zero errorlevel, we now don't run the extra tasks. That doesn't seem unreasonable - if you need post-run tasks, they likely need the main run to finish correctly.

@gusbrs
Copy link
Author

gusbrs commented Dec 20, 2023

So if the main run gives a non-zero errorlevel, we now don't run the extra tasks. That doesn't seem unreasonable - if you need post-run tasks, they likely need the main run to finish correctly.

My first reaction was in agreement, and I'm happy I corrected the blatant errors in my test files. But isn't deliberately generating an error a legitimate technique in tests (e.g. the l3build documented \ERROR macro)?

@moewew
Copy link

moewew commented Dec 20, 2023

Hmm, I'm using lots of \showbox which I think halts the TeX run and probably lets it end with a non-zero exit code. Is there an alternative for that?

@u-fischer
Copy link
Member

u-fischer commented Jan 2, 2024

@moewew you could redirect the box content into an external file and read that in again. Imho that shouldn't led a non-zero exit code:

\documentclass{article}
\input{regression-test}
\begin{document}
\START
\newwrite\boxout
\immediate\openout\boxout{box.txt}
\showstream\boxout
\setbox0\hbox{duck}
\showbox0
\setbox0\hbox{bär}
\showbox0
\immediate\closeout\boxout
\SHOWFILE{box.txt}
xxx
\end{document}

@FrankMittelbach
Copy link
Member

If that works, we could provide a command like \SHOWBOX that encaps that workflow, couldn't we?

moewew added a commit to moewew/biblatex-ext that referenced this issue Jan 2, 2024
@moewew
Copy link

moewew commented Jan 2, 2024

@u-fischer Thanks. That appears to work and is a very clever trick. I hope you don't mind me saying that it doesn't feel particularly elegant, but since I've packed it up into a macro, I don't have to worry about that too much.

@FrankMittelbach
Copy link
Member

@moewew that's why I suggested that the test support code should offer such a macro so that one has a clean interface for such testing. I therefore reopen that issue, to consider this as an enhancement

@muzimuzhi
Copy link
Contributor

muzimuzhi commented Jan 3, 2024

The problem described in (current) title of this issue has been resolved. Maybe trace the new/follow-up problem reported in #327 (comment) in a new issue?

Hmm, I'm using lots of \showbox which I think halts the TeX run and probably lets it end with a non-zero exit code. Is there an alternative for that?

Update: I opened #336.

@josephwright
Copy link
Member

One option would be to change back to running any extra tasks ignoring the exit code of the main run.

I included the check as I'd imaged that people using an external tool would arrange that any tests only took place after the external tool had run, so something like

\IfFileExists{output-of-tool}
  {\def\testoutput#1{Some-useful-payload}}
  {\def\testoutput#1{}}

but that was likely naïve.

Almost all of the time there are no extras, so simply running irrespective of errorlevel is not going to impact on almost all users of l3build.

@josephwright
Copy link
Member

One option would be to change back to running any extra tasks ignoring the exit code of the main run.

I'm going to go with this plan - I think on balance it's safest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants