-
Notifications
You must be signed in to change notification settings - Fork 5
Make several changes to make installation more consistent across platforms and improve usability #5
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
base: main
Are you sure you want to change the base?
Conversation
Xerces and ICU seem to be common missing dependencies, so I've added them into the installation. The sheband in setup.sh now uses '/usr/bin/env bash', which should get the first instance of bash in the PATH variable. This should avoid problems with multiple installations of bash (a particular problem with homebrew) and is a portable method across systems.
Installs on mac were failing due to using 'dylib' instead of 'so' and a few other things. Also added an "--arm64" option for building on arm64 architecture. I wasn't able to include tensorflow for this. Added tests for every install stage. Ratpac requires some edits to be installed on macs. Here, I've made the edits using sed after ratpac is cloned. This isn't ideal. Hopefully we can make ratpac more universal so these edits aren't needed.
setup.sh should now quit as soon as it runs into an error and will print a helpful message saying what part of the install failed.
Was using wrong syntax for the if line that checked if we were using a mac
Hard-coded dylib as the default library. Now dylib is only used if a mac install is specified.
Was using the wrong variable for finding the xerces library
…tup into WilfS/devForMacAndXerces
Used shellcheck to help improve the shell script formatting. Mainly things like adding double quotes around expanded variables and adding '|| exit' to certain commands to catch failures. Should help catch install issues and aid future development
This allows the testMain program to be run.
Escaped the $ character in one of the sed commands
Print help message and read in arguments before checking for dependencies.
Some variables weren't being set due to reordering the code. Now fixed.
hdf5 has been added to the install and I accidentally added a comma when adding it to the install option. Fixed now.
|
I just fixed a bug introduced when the merge with main occurred. Please pull the latest commit! |
|
I have identified the source of the Does this work in bash if you make a similar change? |
|
Yes, it does appear to work still in bash. |
|
By the way, I'm a bit confused that you're not executing the script with bash. The shebang in setup.sh should ensure the script is executed with bash regardless of the shell it was run in... |
Bash version on Mac is very old due to licensing argument for arm64 (if I remember correctly). I believe the bash version dates back to 2007 and is missing some key updates to |
|
Hi @WilfS , we are reviving some efforts into looking into Mac support, and other things in this PR. Thanks again for submitting this! A quick question -- have you tried building xerces-c without ICU? As far as I can tell, ICU is a unicode support library and should be very rarely used by xerces-c under geant4, as GDML files do not container unicode characters. |
|
Hi @JamesJieranShen, I've not specifically looked into that, no. All I know is that when I tried to install xerces-c it wouldn't work without ICU. There may be some options that allow the build without ICU, in which case I'd happily remove the ICU install. I can look into it this week. |
This thread suggests that the transcoder can be removed when using |
|
Nice, thanks. I'll try that cmake option. The main thing I need feedback on regarding the mac install is how to deal with ROOT. The desired "v6-28-00-patches" branch won't build on recent macos versions. I've got around this issue myself by cloning and building the "latest-stable" branch of ROOT, but I imagine that wouldn't be ideal in general—for example minuit is not included in the latest ROOT versions. |
|
|
Thanks for looking into this! Guess ChatGPT is not always correct (shocker). I do know that it is possible to build using the make-configure paradigm ( |
per this ROOT 6.32+ the cmake option for minuit2 was removed as root can no longer be built without minuit2. Since this option was turned on by default in 6.28 as well, we can probably safely remove this flag from the buildscript altogether. As for version advancement -- RAT's coupling with ROOT is very minimal, so I would imagine changes from 6.28->6.34 would not effect anything. Obviously, one should test this. In the mean time, perhaps we can provide separate flags for several versions of the script. If all is well As a general rule of thumb, though, we shouldn't build against |
Older versions of ROOT do not build on the latest macOS versions. Our solution is to checkout v6-34-00-patches for the mac installation. v6-28-00-patches is still used for non-mac installs.
|
I believe this PR is good to go. Might be a good idea to merge in now so that people can use their macs at the June workshop if they want. |
|
Ah wait looks like the ICU dependency wasn't resolved. Thought I had done that. Let me try and fix that... |
ICU is used as a transcoder by xerces-c, but is not always present by default on machines. The build now tells xerces-c to use iconv rather than ICU, which is more likely to be pre-installed.
Accidentally changed the geant4 version in the last commit. This changes it back to the version used before.
|
I've removed the ICU dependency but of course another issue has sprung up. It looks like in the latest macos update Apple Clang was updated to use LLVM 19. This clashes with the zlib version used in geant, so the version of geant we're checking out can't be installed on the latest mac update without installing and pointing to LLVM 16 during the installation. The installation seems to work with the latest geant version (v11.3.2), but I don't know how different that is to the version currently being used (v11.1.2) |
|
Hi Wilf, I don't know if there was asked before, but is there anyway that xercesc can be installed via homebrew / some other package manager instead? I think ideally we would like to restrict the scope of ratpac-setup to dependencies of ratpac2, and not extend it to dependencies of dependencies if possible. Thanks! |
|
Hi @JamesJieranShen. xercesc can be installed via homebrew. I'm not sure about other package managers. I mainly included it in the install because it seemed so common for it not to be present when installing ratpac on computing clusters, where installing via package manager isn't possible due to permissions. However I agree that restricting the scope of ratpac-setup is desirable, so I'm happy to remove the xercesc install if that's what is desired. I think some warning of the xercesc dependency and guidance around it should be added to the readme, at least. |
Thanks! I definitely think then that having the xercesc instruction in the readme/as a warning would be a good compromise. However I think you're right in that this package is special in the sense that it's rarely installed, perhaps we can keep your current implementation but make it such that you have to use ./setup.sh --only xercesc and that it won't be installed by default along with other things. Would that (plus including this behavior in the readme) sound like a reasonable thing to you? |
|
Yes, that sounds like a good idea. I'll update things... |
The number of dependencies in ratpac-setup should be minimised. Xerces-C is normally present on most machines, so it's been removed from the main installation with the option of installing with a specific command if needed. The readme has been updated with instructions.
…tup into WilfS/devForMacAndXerces
|
Regarding the geant4 issue on the latest macos, only the latest version of geant (11.3.2) is installable, but it is incompatible with ratpac and the ratpac install will fail. I think the only solution is to use the older xcode tools. I'll look into how feasible that is to automate in a script like this. |
|
Turns out, not very feasible. It seems running the software on MacOS 15.4 isn't possible unless we update ratpac to use the latest geant version, or maybe if we patch the old geant, either of which would probably be a big job. I can try and look into how difficult it would be. Let me know if anyone has any insight on this. |
|
I am currently playing with the latest version of Geant4 and RATPAC on Linux. It seems that most of the changes are straightforward and only a few warnings are popping up. It does seem to compile and I can run it. I will try this install on the Mac, and see if I can additionally open a PR with requires changes in a smart way, so that it will load the correct code depending on the G4 version. |
|
That would be terrific, Marc. Thanks! |
This is quite a big pull request! But the changes are relatively basic.
Fixed mac install:
There was an option for installation on macs, but the installation failed on macOS, mainly because macOS uses .dylib libraries rather than .so libraries. Added an option to install on arm64 architecture too, which most macs are nowadays. Had to sed some changes to ratpac's CMakeLists.txt, which isn't ideal, but can't be fixed permanently without changes to ratpac itself.
Added more packages:
Xerces-C (needed by Geant4) and its dependency ICU are rarely pre-installed on users' machines, so I've added them to the installation.
Improved error handling:
It was difficult to work out where the install had gone wrong sometimes. I've made it so the install stops as soon as it runs into an error, and it reports back the line where it failed.
Improved bash formatting:
Used shellcheck to reformat setup.sh with recommended bash standard. Mainly adding quotes to variable to prevent expansion.