Bioconductor / BiocStyle

Issues and pull requests for BiocStyle should go here.
12 stars 19 forks source link

Test fails with TeXLive 2023 #105

Closed tillea closed 2 months ago

tillea commented 10 months ago

Hi, there is a bug report in Debian about a failure with TeXLive 2023. Just see the bug log and the attached workaround (which probably needs enhancement). Kind regards, Andreas.

bschilder commented 10 months ago

Ah actually my previously reported issue seems to be more directly related to this! ☝️

179s   Running 'texi2dvi' on 'maketitle_test_1.tex' failed.
179s LaTeX errors:
179s ! LaTeX Error: Command \@raggedtwoe@everyselectfont undefined.
bschilder commented 9 months ago

@grimbough any thoughts on how to resolve this?

hpreusse commented 9 months ago

With the patch described in Debian bug #1050807 the Bioconductor.sty becomes incompatible to ragged2e < v3.5. On the one hand one could make sure not to use the old ragged2e (if this is possible somehow) after applying the patch. On the other hand one could try to improve the patch by doing something like:

\ifdefined\@raggedtwoe@everyselectfont
  \renewcommand{\@raggedtwoe@everyselectfont}{%
  ...more lines
  <snip>
\else
  \newcommand{\@raggedtwoe@everyselectfont}{%
  ...more lines
  <snip>
\fi

This suggestion is untested. Finally one could evaluate if the hack in Bioconductor.sty (which is 9 years old) is really still needed.

P.S.: this style file contains lots of \makeatletter / \makeatother pairs, which are really not meant to be there.

grimbough commented 9 months ago

Just confirming we also see this on our CI runs at: https://github.com/Bioconductor/BiocStyle/actions/runs/6185247528/job/16790480792

bschilder commented 9 months ago

Just confirming we also see this on our CI runs at: https://github.com/Bioconductor/BiocStyle/actions/runs/6185247528/job/16790480792

Aha, this is super helpful, it seems the pandoc version plays a big role here. Let me add pandoc_version as an arg in rworkflows. I'll make 2.17 the default for now, since that's the latest version that seems to be working.

grimbough commented 9 months ago

Those tests aren't comprehensive. They confound both tinytex & pandoc versions. Given the other reports here I'm sure it's the latest Tinytex (or texlive) version that is the issue

bschilder commented 9 months ago

Those tests aren't comprehensive. They confound both tinytex & pandoc versions. Given the other reports here I'm sure it's the latest Tinytex (or texlive) version that is the issue

Oh OK I see, pandoc 2.17 uses the tinytex 2022.01, thanks for pointing that out.

grimbough commented 9 months ago

Hopefully this is now solved in https://github.com/Bioconductor/BiocStyle/commit/0858890b914dc453b4d547d294f9f28a5691d3f4

I've no idea whether the issue the patch was trying to solve is still around, so for now I've elected to only apply they patch if we're using ragged2e version 3.5 or older. We'll see if anyone complains of unexpected rendering going forwards.

This fix should be available in both release and devel version of the package (2.28.1 and 2.29.2 respectively).

Thank you all for the pointers, please report here if it continues to be an issue.

hpreusse commented 9 months ago

Hopefully this is now solved in 0858890

I've afraid this is the wrong direction: with ragged2e later than 2023/04/03 the command \@raggedtwoe@everyselectfont does not exist any more and hence \renewcommand fails. See here.

grimbough commented 9 months ago

Isn't that what this line is doing?

https://github.com/Bioconductor/BiocStyle/blob/3ae5fa0ab6e840d67ea9eab4a664fbacb52897f4/inst/resources/tex/Bioconductor.sty#L529

If ragged2e is later than 2023-06-01 then it enters {} i.e. it does nothing, and if it's not later than 2023-06-01 it implements the old behaviour. Maybe I've misunderstood something.

hpreusse commented 9 months ago

You are correct, sorry I missed that {}.