search

cms-externals / geant4

CMS mirror of Geant4 sources. Not endorsed by Geant4 organization itself under any circumstance.
Other
2 stars 6 forks source link

Fixed warnings for LTO build of biglib #70

Closed civanch closed 1 year ago

civanch commented 1 year ago

This PR is needed to fix compilation warnings in Geant4 when biglib is built. These warnings happens in the classes, which are not used in standard CMSSW WFs, so no change of results are expected.

cmsbuild commented 1 year ago

A new Pull Request was created by @civanch (Vladimir Ivantchenko) for branch cms/v10.7.2.

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks. @perrotta, @dpiparo, @rappoccio you are the release manager for this. cms-bot commands are listed here

smuzaffar commented 1 year ago

thanks @civanch. Please note that currently we use G4. sources directly from https://github.com/Geant4/geant4/commits/v10.7.2 ( see https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_13_1_X/master/geant4.spec#L5-L8 ) . So your fixes [a] are not part of our G4 build. You PR is on top of our branch cms/v10.7.2 . So is it ok to include [a] also in cms build?

[a] https://github.com/cms-externals/geant4/commits/cms/v10.7.2

civanch commented 1 year ago

@smuzaffar , I am a bit confuse, should I make the same PR to https://github.com/Geant4/geant4/commits/v10.7.2 and close this one?

smuzaffar commented 1 year ago

@civanch , I mean your change https://github.com/cms-externals/geant4/pull/54 was only tested in special Geant4 IBs and they are not part geant4 we are using in 13.1.X. My question is , should https://github.com/cms-externals/geant4/pull/54 be included in default 13.1.X Geant4 or shoudl we use official G4 v10.7.2 plus changes in this PR. Currently this PR has

smuzaffar commented 1 year ago

should I make the same PR to https://github.com/Geant4/geant4/commits/v10.7.2 and close this one?

No, there is no proint making PR to Geant4/geant4. If #54 is not needed then we can create a new cms/v10.7.2 (based on official G4 v10.7.2 tag) and then you can open this PR based on that. But if #54 is needed then this PR is fine

civanch commented 1 year ago

Both PRs #54 and #70 are pure technical, do not affect results but help to avoid extra warnings for different builds and may improve backtrace if something real happens in production. These fixes are backports from newer G4. Because 13_0 is the 2023 production release with G4 10.7.2 it is better to accumulate such patches.

For 13_1 I still hope we can migrate to G4 11.1.

smuzaffar commented 1 year ago

please test for CMSSW_13_1_X

cmsbuild commented 1 year ago

-1

Failed Tests: UnitTests Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-72d06f/31309/summary.html COMMIT: 14a6d1c151d61c27ab02ef3329d0a7e8920ab75e CMSSW: CMSSW_13_1_X_2023-03-16-0800/el8_amd64_gcc11 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-externals/geant4/70/31309/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test DRNTest had ERRORS

Comparison Summary

Summary: