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

missing buildfile dependencies in DataFormats and CondFormats #31005

Closed
wants to merge 5 commits into from

Conversation

davidlange6
Copy link
Contributor

as usual - just grepping for #include...

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31005/17468

  • This PR adds an extra 44KB to repository

@davidlange6
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 31, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlange6 (David Lange) for master.

It involves the following packages:

CondFormats/AlignmentRecord
CondFormats/CSCObjects
CondFormats/CastorObjects
CondFormats/DTObjects
CondFormats/DataRecord
CondFormats/ESObjects
CondFormats/EcalCorrections
CondFormats/EcalObjects
CondFormats/External
CondFormats/GEMObjects
CondFormats/GeometryObjects
CondFormats/JetMETObjects
CondFormats/MFObjects
CondFormats/PPSObjects
CondFormats/SiPixelTransient
DataFormats/Alignment
DataFormats/BTauReco
DataFormats/BeamSpot
DataFormats/CSCDigi
DataFormats/CTPPSDetId
DataFormats/CTPPSDigi
DataFormats/CTPPSReco
DataFormats/CastorReco
DataFormats/DTDigi
DataFormats/EcalDigi
DataFormats/EcalRecHit
DataFormats/EgammaCandidates
DataFormats/EgammaReco
DataFormats/EgammaTrackReco
DataFormats/FP420Cluster
DataFormats/FTLDigi
DataFormats/FTLRecHit
DataFormats/GEMDigi
DataFormats/GEMRecHit
DataFormats/GeometrySurface
DataFormats/GsfTrackReco
DataFormats/HGCDigi
DataFormats/HGCRecHit
DataFormats/HGCalReco
DataFormats/HLTReco
DataFormats/HcalCalibObjects
DataFormats/HcalDetId
DataFormats/HcalDigi
DataFormats/Histograms
DataFormats/JetReco
DataFormats/L1TCalorimeterPhase2
DataFormats/L1TCorrelator
DataFormats/L1TGlobal
DataFormats/L1THGCal
DataFormats/L1TMuon
DataFormats/L1TParticleFlow
DataFormats/L1TrackTrigger
DataFormats/L1Trigger
DataFormats/LTCDigi
DataFormats/Luminosity
DataFormats/METReco
DataFormats/Math
DataFormats/MuonReco
DataFormats/MuonSeed
DataFormats/OnlineMetaData
DataFormats/ParticleFlowCandidate
DataFormats/ParticleFlowReco
DataFormats/ProtonReco
DataFormats/Provenance
DataFormats/RPCDigi
DataFormats/RecoCandidate
DataFormats/SiPixelCluster
DataFormats/SiPixelDetId
DataFormats/SiPixelRawData
DataFormats/SiStripCluster
DataFormats/SiStripCommon
DataFormats/Streamer
DataFormats/TauReco
DataFormats/TestObjects
DataFormats/TrackCandidate
DataFormats/TrackerRecHit2D
DataFormats/TrackingRecHit
DataFormats/TrajectoryState
DataFormats/VertexReco

@andrius-k, @ggovi, @christopheralanwest, @kpedro88, @Martin-Grunewald, @rekovic, @fioriNTU, @tlampen, @pohsun, @santocch, @perrotta, @civanch, @makortel, @cmsbuild, @fwyzard, @smuzaffar, @Dr15Jones, @tocheng, @jfernan2, @mdhildreth, @slava77, @jpata, @benkrikler, @kmaeshima, @emeschi, @mommsen can you please review it and eventually sign? Thanks.
@erikbutz, @rappoccio, @gouskos, @echabert, @felicepantaleo, @jainshilpi, @hatakeyamak, @robervalwalsh, @emilbols, @sviret, @argiro, @jshlee, @Fedespring, @thomreis, @ahinzmann, @lgray, @abbiendi, @varuns23, @seemasharmafnal, @mmarionncern, @smoortga, @sethzenz, @JyothsnaKomaragiri, @makortel, @threus, @jhgoh, @amagitte, @apsallid, @jbsauvan, @cericeci, @dildick, @battibass, @pieterdavid, @forthommel, @vandreev11, @Sam-Harper, @HuguesBrun, @cbernet, @rovere, @VinInn, @cseez, @ferencek, @nhanvtran, @gkasieczka, @tocheng, @deguio, @schoef, @alesaggio, @mmusich, @ptcox, @abdoulline, @fabiocos, @clelange, @kreczko, @jdamgov, @jdolen, @rchatter, @trocino, @riga, @jan-kaspar, @wddgit, @calderona, @mverzett, @sobhatta, @lecriste, @afiqaize, @gpetruc, @mariadalfonso, @amarini, @andrzejnovak, @folguera, @pfs this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

-1

Tested at: f9b0ef8

CMSSW: CMSSW_11_2_X_2020-07-31-1100
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-70fc35/8467/summary.html

I found follow errors while testing this PR

Failed tests: Build HeaderConsistency ClangBuild

  • Build:

I found compilation warning when building: See details on the summary page.

  • Clang:

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2020

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@Dr15Jones
Copy link
Contributor

So it looks like this is causing scram to find circular dependencies?

@davidlange6
Copy link
Contributor Author

davidlange6 commented Aug 1, 2020 via email

DataFormats/ParticleFlowReco/BuildFile.xml Outdated Show resolved Hide resolved
DataFormats/ParticleFlowReco/BuildFile.xml Outdated Show resolved Hide resolved
@Dr15Jones
Copy link
Contributor

The problem is a direct circular dependency between DataFormats/GeometrySurface and DataFormats/GeometryCommonDetAlgo. It looks like GeometrySurface was intended to be the 'lower level' package. Unfortunately it grew the following dependency

[cdj@cmslpc151 src]$ grep GeometryCommonDetAlgo $CMSSW_RELEASE_BASE/src/DataFormats/GeometrySurface/*/*
[cut]DataFormats/GeometrySurface/interface/LocalErrorBaseExtended.h:#include "DataFormats/GeometryCommonDetAlgo/interface/DeepCopyPointer.h"
[cut]/DataFormats/GeometrySurface/interface/LocalErrorExtended.h:#include "DataFormats/GeometryCommonDetAlgo/interface/ErrorMatrixTag.h"

The inclusion of DeepCopyPointer.h is unnecessary while ErrorMatrixTag is used. However, the definition of LocalErrorExtended is only used in two places

[cdj@cmslpc151 src]$ git grep DataFormats/GeometrySurface/interface/LocalErrorExtended.h
DataFormats/GeometryCommonDetAlgo/interface/ErrorFrameTransformer.h:#include "DataFormats/GeometrySurface/interface/LocalErrorExtended.h"
Geometry/CommonTopologies/interface/MuonGeomDet.h:#include "DataFormats/GeometrySurface/interface/LocalErrorExtended.h"

and

[cdj@cmslpc151 src]$ git grep DataFormats/GeometrySurface/interface/LocalErrorBaseExtended.h
DataFormats/GeometrySurface/interface/LocalErrorExtended.h:#include "DataFormats/GeometrySurface/interface/LocalErrorBaseExtended.h"

Therefore LocalErrorBaseExtended.h and LocalErrorExtended.h can easily be moved to DataFormats/GeometryCommonDetAlgo.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31005/17498

  • This PR adds an extra 64KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2020

@Dr15Jones
Copy link
Contributor

@davidlange6 I'm working on a pull request now to fix the geometry dependencies.

@Dr15Jones
Copy link
Contributor

Dr15Jones commented Aug 1, 2020

See #31018

@davidlange6
Copy link
Contributor Author

thanks @Dr15Jones - I see that there is at least one more circular dep to fix too:(

@davidlange6
Copy link
Contributor Author

ok - I close this - there are numerous circular dependencies revealed by this PR

@makortel
Copy link
Contributor

makortel commented Aug 3, 2020

Great - if you want to suggest a patch, I can apply it in this PR.

Could you cherry-pick this one makortel@7868841 ?

I'll make a separate PR of this then.

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