\newcommand in latexrelease.sty

[aminophen]

Brief outline of the bug

I guess \newcommand should not be used inside latexrelease.sty. With 'develop' branch, I get the following error when processing the MWE:

! LaTeX Error: Command \DeclareFontSeriesDefault already defined.
               Or name \end... illegal, see p.192 of the manual.

Currently there are four:

\newcommand\DeclareFontSeriesDefault[3][]{%
\newcommand\ulcdefault{ulc}
\newcommand\swdefault{sw}
\newcommand\sscdefault{ssc}

Minimal example showing the bug

\RequirePackage[2020/02/02]{latexrelease}
\stop

Log file (required) and possibly PDF file

• test.log

[FrankMittelbach]

Looks to me more like a bug in the logic: if we have the following sequence of \IncludeInRelease

\IncludeInRelease{2019/10/01}{\filename@simple}
                             {Final dot for extension}%
\IncludeInRelease{0000/00/00}{\filename@simple}
                             {Final dot for extension}%

And we are asking for reverting to 2020-02-02 and the current format is 2020-10-01 then it contains the 2019/10/01 patch already and so we should not "reapply" it (but we do).

In contrast if we ask for 2018-01-01 we skip the 2019/10/01 code but execute the 0000/00/00 code.

In other words, if we move backwards I need to skip the first patch (because that is only there for forward patching).

\newcommand is one example where reapplying fails (and could be correct by not using it) but others are allocating registers again etc. So perhaps better to think once more how to get the logic fully right.

[aminophen]

Looks to me more like a bug in the logic

Oh... I guess so too. I wasn't aware of that, thanks

[FrankMittelbach]

well, both really. @davidcarlisle correctly pointed out to me that we need to take care of the newcommands nevertheless because if something is changed twice and both use newcommand it still breaks even if we skip over the last patch which is equal to what is in the format.

So I will go and fix that first. The logic part is not so important, eg reapplying definitions is then not a problem and burning a few registers is not a problem either given that we have enough these days.

[FrankMittelbach]

Added the necessary \@undefined and discovered a second bug in latexrelease: it attempts to reload expl3.ltx. I turned that off, but I'm not sure the way I did it is the right solution. Can you @josephwright, @davidcarlisle have a second look please?

[PhelypeOleinik]

About expl3.ltx, the change seems okay to me. However, shouldn't
expl3.ltx be made reload-safe safe, same as expl3.sty? Granted, it
probably won't be reloaded as frequently as expl3.sty, but still...

[FrankMittelbach]

@PhelypeOleinik I'm not sure it is necessary as it should never get loaded other than in the format or in a rollback situation (or should it?), but please feel free to check in an improved version into that branch. It would certainly be safer.

[PhelypeOleinik]

@FrankMittelbach Yes, that file should only be loaded by the format and
the rollback, but the rollback seems enough to make it reload-safe.
Shouldn't be hard: most of it already is so because of expl3.sty. I'll do it

[davidcarlisle]

@FrankMittelbach looks Ok to me and worked with works with some additional simple tests, merge back to develop?

[FrankMittelbach]

@PhelypeOleinik wanted to make it reload safe. Not sure it is really needed but it doesn't hurt either. So I guess we'll wait until he is done with that. No urgency .

[PhelypeOleinik]

@FrankMittelbach That I do at the expl3 side, not here. The code is
common to expl3.sty, so you can merge this back (I think). I'll do the
re-load thing in a bit

[davidcarlisle]

the false branch has \@gobbletwo to remove this, so it's Ok I think

…

On Mon, 2 Mar 2020 at 20:47, Phelype Oleinik ***@***.***> wrote: Just a small thing: shoudn't this line: https://github.com/latex3/latex2e/blob/743b9a75a3a92b7d8e931a77277372636ba52504/base/ltexpl.dtx#L97 be in the true branch of \IfFileExists? Or is the error intentional, in case expl3.ltx does not exist? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#295?email_source=notifications&email_token=AAJVYAXH7WELVDOF4P7EHW3RFQLORA5CNFSM4K5VZXA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENQ5GEI#issuecomment-593613585>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJVYASXLB6HPM2OTD7L5KTRFQLORANCNFSM4K5VZXAQ> .

[PhelypeOleinik]

@davidcarlisle Yeah, I noticed this right after commenting :-)
Sorry about the noise