[DataFormats/PatCandidates] Rework statics in header file. by vgvassilev · Pull Request #30499 · cms-sw/cmssw

vgvassilev · 2020-07-02T13:16:40Z

Static variables have internal linkage (http://eel.is/c++draft/basic.link#3.1) and if used in a header file cause ODR violation as every traslation unit will get a different definition of the same entity.

This should address a bug discovered with the CXXMODULE build.

cc: @davidlange6

cmsbuild · 2020-07-02T13:17:06Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-07-02T13:18:01Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-07-02T13:26:45Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30499/16688

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/PatCandidates/interface/PATObject.h modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935, [RFC] Test clang-tidy --checks modernize-use-auto #29936

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30499/16688/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30499/16688/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2020-07-02T13:32:43Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-07-02T13:37:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30499/16692

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/PatCandidates/interface/PATObject.h modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935, [RFC] Test clang-tidy --checks modernize-use-auto #29936

cmsbuild · 2020-07-02T13:37:47Z

A new Pull Request was created by @vgvassilev (Vassil Vassilev) for master.

It involves the following packages:

DataFormats/PatCandidates

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@gouskos, @hatakeyamak, @rovere, @cbernet, @gpetruc, @peruzzim this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

slava77 · 2020-07-02T14:20:37Z

@cmsbuild please test

cmsbuild · 2020-07-02T14:20:55Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-07-01-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/7608/console Started: 2020/07/02 16:28

cmsbuild · 2020-07-02T14:59:52Z

-1

Tested at: 96d5d1b

CMSSW: CMSSW_11_2_X_2020-07-01-2300
SCRAM_ARCH: slc7_amd64_gcc820

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Modified photon selection for heavy-ion miniAOD #30236 @mandrenguyen: Modified photon selection for heavy-ion miniAOD
Phase2-hgx250 Comparison of results from flat file and from insitu checks #30423 @bsunanda: Phase2-hgx250 Comparison of results from flat file and from insitu checks
Added missing include to fix the ROOT IBs build errors #30496 @cms-sw: Added missing include to fix the ROOT IBs build errors
added esConsumes to modules in EventFilter/RPCRawToDigi #30459 @JamminJones: added esConsumes to modules in EventFilter/RPCRawToDigi
DT TPG for Phase-2 #30344 @dtp2-tpg-am: DT TPG for Phase-2
TBLayer computeCrossings doesn't allow for some displaced starting points #30127 @aminnj: TBLayer computeCrossings doesn't allow for some displaced starting points
[Pixel 3D Digitizer] Fix wrong channel assignation #30446 @duartej: [Pixel 3D Digitizer] Fix wrong channel assignation
Reduce boost dependency in DQM/TrackingMonitor #30476 @camolezi: Reduce boost dependency in DQM/TrackingMonitor
added esConsumes to modules in EventFilter/SiStripRawToDigi #30475 @JamminJones: added esConsumes to modules in EventFilter/SiStripRawToDigi
added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474 @JamminJones: added esConsumes to modules on EventFilter/SiPixelRawToDigi

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7608/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7608/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7608/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

Build:

I found compilation error when building:

In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/DataFormats/PatCandidates/interface/Lepton.h:22,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/DataFormats/PatCandidates/interface/Muon.h:28,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/PhysicsTools/PatAlgos/test/private/PATUserDataTestModule.cc:33:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/DataFormats/PatCandidates/interface/PATObject.h: In instantiation of 'const string& pat::PATObject::userDataObjectType(const string&) const [with ObjectType = reco::Muon; std::__cxx11::string = std::__cxx11::basic_string]':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/PhysicsTools/PatAlgos/test/private/PATUserDataTestModule.cc:144:77:   required from here
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-07-01-2300/src/DataFormats/PatCandidates/interface/PATObject.h:332:54: error: returning reference to temporary [-Werror=return-local-addr]
       return (data != nullptr ? data->typeName() : "");
                                                      ^
cc1plus: some warnings being treated as errors
gmake: *** [tmp/slc7_amd64_gcc820/src/PhysicsTools/PatAlgos/test/PhysicsToolsPatAlgos_testAnalyzers/private/PATUserDataTestModule.cc.o] Error 1
>> Building  edm plugin tmp/slc7_amd64_gcc820/src/PhysicsTools/PatAlgos/test/PhysicsToolsPatAlgos_testAnalyzers/libPhysicsToolsPatAlgos_testAnalyzers.so

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.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Modified photon selection for heavy-ion miniAOD #30236 @mandrenguyen: Modified photon selection for heavy-ion miniAOD
Phase2-hgx250 Comparison of results from flat file and from insitu checks #30423 @bsunanda: Phase2-hgx250 Comparison of results from flat file and from insitu checks
Added missing include to fix the ROOT IBs build errors #30496 @cms-sw: Added missing include to fix the ROOT IBs build errors
added esConsumes to modules in EventFilter/RPCRawToDigi #30459 @JamminJones: added esConsumes to modules in EventFilter/RPCRawToDigi
DT TPG for Phase-2 #30344 @dtp2-tpg-am: DT TPG for Phase-2
TBLayer computeCrossings doesn't allow for some displaced starting points #30127 @aminnj: TBLayer computeCrossings doesn't allow for some displaced starting points
[Pixel 3D Digitizer] Fix wrong channel assignation #30446 @duartej: [Pixel 3D Digitizer] Fix wrong channel assignation
Reduce boost dependency in DQM/TrackingMonitor #30476 @camolezi: Reduce boost dependency in DQM/TrackingMonitor
added esConsumes to modules in EventFilter/SiStripRawToDigi #30475 @JamminJones: added esConsumes to modules in EventFilter/SiStripRawToDigi
added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474 @JamminJones: added esConsumes to modules on EventFilter/SiPixelRawToDigi

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7608/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7608/git-merge-result

cmsbuild · 2020-07-02T14:59:55Z

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

cmsbuild · 2020-07-02T15:46:13Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-07-02T15:56:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30499/16701

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/PatCandidates/interface/PATObject.h modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935, [RFC] Test clang-tidy --checks modernize-use-auto #29936

cmsbuild · 2020-07-02T15:57:22Z

Pull request #30499 was updated. @perrotta, @cmsbuild, @santocch, @slava77 can you please check and sign again.

slava77 · 2020-07-02T21:18:25Z

@cmsbuild please test

cmsbuild · 2020-07-02T21:18:47Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-07-02-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/7628/console Started: 2020/07/02 23:19

cmsbuild · 2020-07-02T23:04:52Z

+1
Tested at: 63edfe1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7628/summary.html
CMSSW: CMSSW_11_2_X_2020-07-02-1100
SCRAM_ARCH: slc7_amd64_gcc820

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Added missing include to fix the ROOT IBs build errors #30496 @cms-sw: Added missing include to fix the ROOT IBs build errors

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7628/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7628/git-merge-result

cmsbuild · 2020-07-02T23:04:55Z

Comparison job queued.

cmsbuild · 2020-07-03T01:04:26Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-49e10f/7628/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2784120
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2784064
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
DQMHistoSizes: changed ( 10224.0 ): -0.004 KiB MessageLogger/Warnings
Checked 154 log files, 17 edm output root files, 37 DQM output files

santocch · 2020-07-03T13:54:02Z

+1

slava77 · 2020-07-03T14:19:12Z

+1

for #30499 63edfe1

technical, in line with the PR description

cmsbuild · 2020-07-03T14:19:34Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-07-04T08:08:26Z

+1

cmsbuild added this to the CMSSW_11_2_X milestone Jul 2, 2020

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jul 2, 2020

vgvassilev force-pushed the internal_linkage_patcand branch from 9f63a5b to c20cf9b Compare July 2, 2020 13:17

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 2, 2020

vgvassilev force-pushed the internal_linkage_patcand branch from c20cf9b to 96d5d1b Compare July 2, 2020 13:32

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 2, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 2, 2020

cmsbuild added tests-started and removed tests-pending labels Jul 2, 2020

cmsbuild removed comparison-pending tests-started labels Jul 2, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 2, 2020

cmsbuild added tests-started and removed tests-pending labels Jul 2, 2020

cmsbuild added tests-approved and removed tests-started labels Jul 2, 2020

cmsbuild added comparison-available and removed comparison-pending labels Jul 3, 2020

cmsbuild added analysis-approved and removed analysis-pending labels Jul 3, 2020

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jul 3, 2020

silviodonato changed the title ~~Rework statics in header file.~~ [DataFormats/PatCandidates] Rework statics in header file. Jul 4, 2020

cmsbuild added orp-approved and removed orp-pending labels Jul 4, 2020

cmsbuild merged commit e5731bb into cms-sw:master Jul 4, 2020

Conversation

vgvassilev commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

slava77 commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

slava77 commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 2, 2020

Uh oh!

cmsbuild commented Jul 3, 2020

Uh oh!

santocch commented Jul 3, 2020

Uh oh!

slava77 commented Jul 3, 2020

Uh oh!

cmsbuild commented Jul 3, 2020

Uh oh!

silviodonato commented Jul 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cmsbuild commented Jul 2, 2020 •

edited

Loading

cmsbuild commented Jul 2, 2020 •

edited

Loading