Skip to content

[builtins] mv mathjax to LCG - #23099

Open
ferdymercury wants to merge 4 commits into
root-project:masterfrom
ferdymercury:patch-5
Open

[builtins] mv mathjax to LCG#23099
ferdymercury wants to merge 4 commits into
root-project:masterfrom
ferdymercury:patch-5

Conversation

@ferdymercury

@ferdymercury ferdymercury commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

@ferdymercury
ferdymercury requested a review from bellenot as a code owner August 19, 2026 06:32
@ferdymercury ferdymercury added the clean build Ask CI to do non-incremental build on PR label Aug 19, 2026
@ferdymercury ferdymercury reopened this Aug 19, 2026
@ferdymercury
ferdymercury requested a review from linev August 19, 2026 06:33
@ferdymercury ferdymercury added the pr:emergency🚨 This PR is fixing breakage, it's urgent and may overtake others label Aug 19, 2026
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 12h 47m 1s ⏱️
 3 856 tests  3 855 ✅ 0 💤 1 ❌
75 719 runs  75 718 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 570d0b0.

♻️ This comment has been updated with latest results.

@ferdymercury

Copy link
Copy Markdown
Collaborator Author

Optional improvement: LCG contains a mathjax package, so the link could be moved to LCG https://lcgpackages.web.cern.ch/tarFiles/sources/mathjax.tar.gz
but that mathjax version on LCG seems old from 2018

@linev

linev commented Aug 19, 2026

Copy link
Copy Markdown
Member

If we do not remove documentation/doxygen/mathjax.tar.gz file - there is no sense to download extra version from root.cern website.

And one need to understand reason why CI fails.

@andresailer

Copy link
Copy Markdown
Contributor

https://lcgpackages.web.cern.ch/tarFiles/sources/mathjax-3.2.0.tar.gz

@ferdymercury

Copy link
Copy Markdown
Collaborator Author

If we do not remove documentation/doxygen/mathjax.tar.gz file - there is no sense to download extra version from root.cern website.

It was removed in #23095

And one need to understand reason why CI fails.

After #23095

@ferdymercury ferdymercury changed the title [builtins] update link to non-vendored mathjax [builtins] update link to non-vendored mathjax and mv link to LCG Aug 19, 2026
@linev

linev commented Aug 19, 2026

Copy link
Copy Markdown
Member

And one need to understand reason why CI fails.

After #23095

@dpiparo
Why CI was not triggered in this PR? Because of doxygen in the title?

@ferdymercury ferdymercury changed the title [builtins] update link to non-vendored mathjax and mv link to LCG [builtins] fix JS link to removed mathjax and update link to LCG Aug 19, 2026

@linev linev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me

@dpiparo

dpiparo commented Aug 19, 2026

Copy link
Copy Markdown
Member

That was fast. Thanks. I think the technical implementation of this is perfect, however by policy perhaps we are not yet ready for this change, see #23100

@ferdymercury
ferdymercury marked this pull request as draft August 19, 2026 07:11
@ferdymercury

Copy link
Copy Markdown
Collaborator Author

however by policy perhaps we are not yet ready for this change, see #23100

Right. (Although mathjax is maybe an optional dependency of webgui meaning it can still build and show several stuff without it?)

Could an alternative be to have Mathjax as a normal builtin_mathjax option? Installed by the user from system packages and backed-up by LCG link?

I think it would be for the different platforms

apt install libjs-mathjax
zypper install mathjax
dnf install mathjax3
npm install mathjax@3

whereas Windows would need builtin_mathjax=ON (internet connection)

@linev

linev commented Aug 19, 2026

Copy link
Copy Markdown
Member

zypper install mathjax

I checked that it install.
It is node.js module in the /usr/lib/node_modules/mathjax directory.
Probably it can be reused for jsroot and webgui (via symbolic link),
but not sure how it can be used for doxygen.

Anyway we download lot of packages from LCG,
mathjax will be one more.
That I like to avoid - having different URLs for different packages.

@ferdymercury

Copy link
Copy Markdown
Collaborator Author

but not sure how it can be used for doxygen.

I guess via:

MATHJAX_RELPATH = /usr/share/javascript/mathjax/

or wherever the system installs it

@ferdymercury ferdymercury removed the pr:emergency🚨 This PR is fixing breakage, it's urgent and may overtake others label Aug 19, 2026
@ferdymercury ferdymercury changed the title [builtins] fix JS link to removed mathjax and update link to LCG [builtins] mv mathjax to LCG Aug 19, 2026
@dpiparo

dpiparo commented Aug 19, 2026

Copy link
Copy Markdown
Member

Thanks for these exchanges. Very useful. Now we can discuss with a bit of calm with the builds un-broken...

but not sure how it can be used for doxygen.

That is the very least of the problems. Doxygen doc generation is special and we can afford to fetch mathjax from LCG w/o problems if needed I think.

Could an alternative be to have Mathjax as a normal builtin_mathjax option? Installed by the user from system packages and backed-up by LCG link?

Definitively yes, I think: that would be a good way to treat MathJax as any other 3rd party dependency, coherently. Two questions are perhaps important:

  • Do all platform propose a streamlined way to install mathjax, including macOS via brew?
  • If yes, can the webgui use those installations?

@linev

linev commented Aug 19, 2026

Copy link
Copy Markdown
Member

If yes, can the webgui use those installations?

This need to be investigated.
MathJax is optional part for jsroot - used for complex TLatex or for TMathText rendering.
But also without mathjax jsroot able to render all kinds of latex.

Main complication I see - correctly recognize installed mathjax module on different platforms.
It is not library with include files, it is set of JavaScript/JSON/fonts files installed in custom location.
So one need to have robust FindMathjax.cmake macro which able to find it on all supported platforms.

Down line for this - it is not trivial task.
Switching to LCG package as first step is much easier.

@ferdymercury
ferdymercury requested a review from linev August 19, 2026 08:28
@ferdymercury
ferdymercury marked this pull request as ready for review August 19, 2026 08:29
@dpiparo

dpiparo commented Aug 19, 2026

Copy link
Copy Markdown
Member

Switching to LCG package as first step is much easier.

Yes. However that would be a change with respect to the current policy: we would impose internet connection and download of a builtin for the standard ROOT build, unless we decide that the standard rendering is what jsroot provides, and, as a plus, one can decide to go for mathjax, which will be fetch from the LCG src tarballs.
(doxygen continues to stay out of the equation: special case treated specially)

@ferdymercury

ferdymercury commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author
  • Do all platform propose a streamlined way to install mathjax, including macOS via brew?

Brew was missing, I proposed it now: Homebrew/homebrew-core#299582

  • If yes, can the webgui use those installations?

Probably not yet, I think it should handle at least v2, v3 and v4 of MathJax since those are common in today's distributions. Right now it would only handle v3 I guess.

So one need to have robust FindMathjax.cmake macro which able to find it on all supported platforms.

I proposed a draft here: #23101

standard rendering is what jsroot provides, and, as a plus, one can decide to go for mathjax

I implemented this idea in this PR by using a warning if no-connection is detected rathar than a failure.

@linev

linev commented Aug 19, 2026

Copy link
Copy Markdown
Member

jsroot not yet work with matjax 4. There are some initialization problems.

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

Labels

clean build Ask CI to do non-incremental build on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants