Skip to content
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

XRootD update to 5.6.0 #5075

Merged
merged 5 commits into from Jul 10, 2023
Merged

XRootD update to 5.6.0 #5075

merged 5 commits into from Jul 10, 2023

Conversation

amadio
Copy link
Contributor

@amadio amadio commented Jul 7, 2023

Notes:

  • Option ENABLE_CRYPTO has been removed. It is always enabled/required.
  • Options ENABLE_PERL and VOMSXRD_SUBMODULE do not exist since a while.
  • CMake now uses the new FindPython.cmake module, so need to update PYTHON_EXECUTABLE to Python_EXECUTABLE.
  • Python bindings build system has been completely rewritten, now follows PEP517.

This still likely won't solve the problem with uuid, but should be worth picking up before updating that. Please check build options here, there may be things like erasure coding support which you may want to enable (dependencies requires are listed in the same file).

Notes:
- Option ENABLE_CRYPTO has been removed. It is always enabled/required.
- Options ENABLE_PERL and VOMSXRD_SUBMODULE do not exist since a while.
- CMake now uses the new FindPython.cmake module, so need to update
  PYTHON_EXECUTABLE to Python_EXECUTABLE.
- Python bindings build system has been completely rewritten, now
  follows PEP517.
- Using CMAKE_CXX_FLAGS_RELWITHDEBINFO overwrites the default -O2 -g,
  so it means you may get a build without optimizations. It's better
  to use just CMAKE_CXX_FLAGS to add -Wno-error to the default flags.
TimoWilken
TimoWilken previously approved these changes Jul 7, 2023
Copy link
Contributor

@TimoWilken TimoWilken left a comment

Choose a reason for hiding this comment

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

Hi @amadio, thanks, looks good to me in general except for one thing: setting CMAKE_CXX_FLAGS_RELWITHDEBINFO is intentional I think, since we already set the CXXFLAGS environment variable here, and this change to -DCMAKE_CXX_FLAGS would wipe that out. That also contains -O2, so we get an optimised build already!

Approving so that the CI tests can run.

This should avoid undeclared runtime dependencies on libintl/gettext.
On Mac, we use our UUID anyway since we depend on AliEn-Runtime, which depends
on UUID, and UUID sets PKG_CONFIG_PATH so we find it even with UUID_ROOT
unset.

This is counter-intuitive, so stop fiddling with UUID_ROOT manually.
@TimoWilken
Copy link
Contributor

...and cherry-picked the UUID commits from #5066.

@adriansev
Copy link
Contributor

@TimoWilken i would also disable XRDCLHTTP
(-DENABLE_XRDCLHTTP=Off) as it depends on davix, we never use it, and on macos there were quite a few problems when the system installed davix was picked up.

Also, just to build only what we use i would also go with ENABLE_KRB5=Off ENABLE_FUSE=Off ENABLE_VOMS=Off
What do you think?

@TimoWilken
Copy link
Contributor

@adriansev Fine by me if it won't break anything at runtime. ENABLE_KRB5=OFF we have already. For -DENABLE_XRDCLHTTP=OFF, is there overlap with -DXRDCL_ONLY=ON (already present)?

@adriansev
Copy link
Contributor

@adriansev Fine by me if it won't break anything at runtime. ENABLE_KRB5=OFF we have already. For -DENABLE_XRDCLHTTP=OFF, is there overlap with -DXRDCL_ONLY=ON (already present)?

@TimoWilken no, XRDCLHTTP is the davix based client component for using http endpoints .. we are using only curl for this, and we never have davix installed as dependency (and when davix is installed as system macos lib then there will be problems)

@TimoWilken
Copy link
Contributor

Alright. Can you push here or should I @adriansev?

TimoWilken
TimoWilken previously approved these changes Jul 7, 2023
Copy link
Contributor

@TimoWilken TimoWilken left a comment

Choose a reason for hiding this comment

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

Never mind, done, and approving for CI.

CXXFLAGS is set in defaults-o2.sh. We only want to add it it, not replace it.
@adriansev
Copy link
Contributor

Alright. Can you push here or should I @adriansev?

could you please do it @TimoWilken ? I'm currently in holyday and it would take some time to do the procedure

@amadio
Copy link
Contributor Author

amadio commented Jul 7, 2023

Hi @amadio, thanks, looks good to me in general except for one thing: setting CMAKE_CXX_FLAGS_RELWITHDEBINFO is intentional I think, since we already set the CXXFLAGS environment variable here, and this change to -DCMAKE_CXX_FLAGS would wipe that out. That also contains -O2, so we get an optimised build already!

Ah, thank you for the clarification. I see you already added a change to revert my change, so I will leave things as they are.

@adriansev
Copy link
Contributor

@TimoWilken the only test that failed is in QC with the test showing:

Test project /sw/BUILD/7f59d0310fc1455e1af500fe0900c426d727c2d2/QualityControl
      Start  1: o2-qc-test-core
 1/31 Test  #1: o2-qc-test-core .........................***Timeout  30.07 sec
2023-07-07 17:36:04.320631  !  �[1;33mWarning - Aggregator name "012345678901234567890" is longer than 16, it might cause name clashes in the DPL workflow�[0m
2023-07-07 17:36:04.339009  !  �[1;33mWarning - No remote machine was specified for a multinode QC setup. This is fine if running with AliECS, but it will fail in standalone mode.�[0m
2023-07-07 17:36:04.339056  !  �[1;33mWarning - No remote machine was specified for a multinode QC setup. This is fine if running with AliECS, but it will fail in standalone mode.�[0m
2023-07-07 17:36:04.339122  !  �[1;33mWarning - too long detector code for a task data origin: XXXXXXXXX, trying to survive with: XXX�[0m
2023-07-07 17:36:04.339148  !  �[1;33mWarning - too long detector code for a task data origin: XXXXXXXXX, trying to survive with: XXX�[0m
2023-07-07 17:36:04.339164  !  �[1;33mWarning - too long detector code for a task data origin: XXXXXXXXX, trying to survive with: XXX�[0m
2023-07-07 17:36:04.339179  !  �[1;33mWarning - too long detector code for a task data origin: XXXXXXXXX, trying to survive with: XXX�[0m
2023-07-07 17:36:04.339198  !  �[1;33mWarning - PP Task name "SkeletonPostProcessing" is longer than 16, it might cause name clashes in the DPL workflow�[0m
2023-07-07 17:36:04.339445     Extracting configuration of a new aggregator MyAggregatorA
2023-07-07 17:36:04.339455        Found a source : MyAggregatorB
2023-07-07 17:36:04.339460           - MyAggregatorB/newQuality
2023-07-07 17:36:04.339473        Found a source : MyAggregatorC
2023-07-07 17:36:04.339481           - MyAggregatorC/newQuality
2023-07-07 17:36:04.339486           - MyAggregatorC/another
2023-07-07 17:36:04.339495     Extracting configuration of a new aggregator MyAggregatorD
2023-07-07 17:36:04.339503        Found a source : MyAggregatorA
2023-07-07 17:36:04.339507           - MyAggregatorA/newQuality
2023-07-07 17:36:04.339512        Found a source : MyAggregatorC
2023-07-07 17:36:04.339517           - MyAggregatorC/newQuality
2023-07-07 17:36:04.339522           - MyAggregatorC/another
2023-07-07 17:36:04.339528     Extracting configuration of a new aggregator MyAggregatorC
2023-07-07 17:36:04.339535        Found a source : dataSizeCheck
2023-07-07 17:36:04.339542           (no QOs specified, we take all)
2023-07-07 17:36:04.339549        Found a source : someNumbersCheck
2023-07-07 17:36:04.339555           (no QOs specified, we take all)
2023-07-07 17:36:04.339584     Extracting configuration of a new aggregator MyAggregatorB
2023-07-07 17:36:04.339591        Found a source : dataSizeCheck1
2023-07-07 17:36:04.339596           (no QOs specified, we take all)
2023-07-07 17:36:04.339601        Found a source : dataSizeCheck2
2023-07-07 17:36:04.339606           - dataSizeCheck2/someNumbersTask/example
2023-07-07 17:36:04.339955 !!! �[1;31mError - Could not find the DPL InfoLogger.�[0m
2023-07-07 17:36:04.340039 !!! �[1;31mError - exception caught and swallowed in templateILDiscardFile : Dynamic exception type: o2::framework::RuntimeErrorRef�[0m
2023-07-07 17:36:04.340105  !  �[1;33mWarning - Could not get updated config tree in TaskRunner::init() - `qcConfiguration` could not be retrieved�[0m
2023-07-07 17:36:04.340122  !  �[1;33mWarning - No URL provided for Bookkeeping. Nothing will be stored nor retrieved.�[0m
2023-07-07 17:36:04.340138     Loading library libO2QcSkeleton

is this a pass or it requires a test restart?

@adriansev
Copy link
Contributor

Good morning @TimoWilken ! the tests are green 😃

@amadio
Copy link
Contributor Author

amadio commented Jul 10, 2023

I'm going to release 5.6.1 today or tomorrow with a fix for the uuid problem in case you'd like to wait.

@TimoWilken
Copy link
Contributor

@adriansev @amadio Thank you!

I'll merge this PR later this morning because it works. Then we can update to v5.6.1 once it's out, so no rush!

@TimoWilken TimoWilken merged commit fcd5e82 into alisw:master Jul 10, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants