Repository navigation
Conversation
Test Results 23 files 23 suites 3d 10h 5m 36s ⏱️ For more details on these failures, see this check. Results for commit 4a3cce2. ♻️ This comment has been updated with latest results. |
4f6e184 to
4a3cce2
Compare
|
Will this PR be backported to 6.42? |
On my side I see no any drawbacks. |
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
Thanks @linev, this is fantastic! I just have a few small suggestions to make things really work on an internet-free sandbox and to iron out some inconsistencies.
|
If we want to provide convenient way to build root without downloading many packages from network - we need to extend functionality of But I do not think that we want to make it for 6.42. This PR (if we decide to backport it) just provide fallback solution if install is not possible. And solution was exists since ever - but it was not configured automatically by build |
It seems late to do this, but in my opinion the backport would be quite important. The drawback is clearly that the build system and required configuration packages changes significantly for packagers after the release candidate. The advantage though is that we avoid churn for the packagers. With the |
|
Actually, |
Sure, I don't advocate for more large scale changes for 6.42.
That's true for |
|
I am really in favor of using Actually I never actively used Solution with It may be useful to provide exclusive alternatives for |
4a3cce2 to
cdb0780
Compare
JSROOT will be installed from github When git clone is used, many files can be deleted which are not necessary - like demos or examples. Let use $JSROOTSYS as custom source for JSROOT Add special dependency that git cloning happens before additional files copied to js/files/ directory
For released JSROOT version one provides tarballs which more easy to download compared with the git clone But for fast development one also need to have possibility to use jsroot git repository Therefore keep both variants in the code
Like jsroot, configure OPENUI5_CUSTOM_LOCATION and use it when generate system.rootrc script
It will also include installation of mathjax Without builtin_jsroot installation of mostly empty js/ directory make no sence - batch files works only with locally installed JSROOT
Install will be performed together with jsroot
Do not try to copy some dirs during ROOT building - there is no version check and therefore provided code can be corrupted
cdb0780 to
3c5e33d
Compare
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
LGTM, thank you very much for the PR!
It's good that there are no debugging environment variables anymore that change the configuration behavior.
As I mentioned in #23877 (comment), I think this PR should be merged and backported to 6.42 ASAP, so we avoid ending up with a release where the JavaScript packages mathjax, openui5, and jsroot are treated in an inconsistent way.
This should also be followed up by a PR that makes sure that ROOT can be built in an environment without internet connection, using the DOWNLOAD_DIR mechanism outlined by Sergey in #23877 (comment)
Build ROOT from root-project/root#23877, which removes JSROOT from the ROOT sources. JSROOT is now a separate jsroot package made from the release tarball, with mathjax 3.2.0 next to it as JSROOT expects. ROOT is pointed to it with the JSROOTSYS environment variable at configure time, which ends up in etc/system.rootrc. The openui5 runtime distribution is passed via the OPENUI5DIR environment variable instead of the OPENUI5_SOURCE_DIR patch. ROOT does not install it in that case, so it is symlinked into the output in postInstall. The mathjax patches are dropped, as mathjax is no longer handled by the ROOT build when JSROOT is external.
Instead of pointing ROOT to an external JSROOT and openui5 with the JSROOTSYS and OPENUI5DIR environment variables, keep builtin_jsroot, builtin_openui5 and builtin_mathjax enabled and give ROOT the archives it would download. A new ROOT_DOWNLOAD_DIR CMake variable (added by a patch) is passed as DOWNLOAD_DIR to ExternalProject_Add, which skips the download when the archive with the expected name and checksum is already there, and the connectivity check is skipped in that case as well. A second patch installs the js directory from the build tree, as otherwise the downloaded JSROOT and mathjax would not be installed. This makes the separate jsroot package unnecessary again, and nothing is needed at runtime outside of the ROOT output. Also drop the lcgpackages substitution: the URLs stay valid, and no download happens as long as the archives are present. See root-project/root#23877 (comment)
| @@ -0,0 +1,32 @@ | |||
| # Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. | |||
There was a problem hiding this comment.
We do not usually rely on GitHub for downloading tarballs. Can this become, also in another PR, an LCG provided tarball?
There was a problem hiding this comment.
Of course.
If you put JSROOT release on LCG, I will directly modify this PR.
|
@linev I have another question: I noticed that some files that were previously in ROOT's
Can these reference be removed because they are obsolete? Or do the files need to be upstreamed to the jsroot repo? |
Like many other components, download jsroot instead maintain of code copy.
In the cmake file provide two ways - URL download of version 7.12.0 (active) or git clone from github repository (commented out). Last case must be there when fast development with ROOT and JSROOT should be done without producing next JSROOT release.
If
builtin_jsroot=OFFproduce warning and switch ROOT to use https://root.cern/js/7.12.0/ for all widgets.This means that such ROOT build requires internet to run webgui widgets and THttpServer UI.
Also when
builtin_openui5=OFF, issue warning and switch to https://sdk.openui5.org/1.152.0/This again require internet to run widgets.
Configure correct dependency between jsroot, mathjax and extra file copies.