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

Add missing includes (companion commit for cms-sw/cmsdist#7309) #35286

Merged
merged 3 commits into from Sep 21, 2021

Conversation

iarspider
Copy link
Contributor

@iarspider iarspider commented Sep 15, 2021

PR description:

Companion commit to cms-sw/cmsdist#7309, adds #include <algorithm> to several source files to fix build errors, e.g.

>> Compiling  /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalParametersFromDD.cc
>> Compiling  /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalSimParametersFromDD.cc
>> Compiling  /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalSimulationConstants.cc
>> Compiling  /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalTopologyMode.cc
/build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalDDDRecConstants.cc: In member function 'std::vector HcalDDDRecConstants::getEtaBins(const int&) const':
/build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-14-1100/src/Geometry/HcalCommonData/src/HcalDDDRecConstants.cc:83:72: error: no matching function for call to 'find(std::vector::iterator, std::vector::iterator, int)'
   83 |           if (std::find(phiSp.begin(), phiSp.end(), (zside * phi.first)) == phiSp.end()) {
      |                                                                        ^
In file included from /cvmfs/cms-ib.cern.ch/nweek-02698/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/bits/locale_facets.h:48,
                 from /cvmfs/cms-ib.cern.ch/nweek-02698/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/bits/basic_ios.h:37,
                 from /cvmfs/cms-ib.cern.ch/nweek-02698/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/ios:44,

Possibly missing #include
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35286/25289

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @iarspider for master.

It involves the following packages:

  • Geometry/HcalCommonData (geometry)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@bsunanda, @fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor

@iarspider , why work in progress? Also please update the pr title to provide the short description of change

@iarspider
Copy link
Contributor Author

WIP is because I am not sure if this will solve the issue, just a wild guess.

@iarspider iarspider changed the title [WIP] Update HcalDDDRecConstants.cc [WIP] Add missing include in HcalDDDRecConstants.cc Sep 15, 2021
@smuzaffar
Copy link
Contributor

Better to test locally first then

@iarspider iarspider changed the title [WIP] Add missing include in HcalDDDRecConstants.cc Add missing includes (companion commit for cms-sw/cmsdist#7309) Sep 17, 2021
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35286/25363

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

Pull request #35286 was updated. @malbouis, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @rekovic, @yuanchao, @ggovi, @francescobrivio, @cecilecaillol, @tvami can you please check and sign again.

@tvami
Copy link
Contributor

tvami commented Sep 17, 2021

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ccd6d9/18715/summary.html
COMMIT: f0a7089
CMSSW: CMSSW_12_1_X_2021-09-17-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35286/18715/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 3000833
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3000799
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Sep 18, 2021

+1

@tvami
Copy link
Contributor

tvami commented Sep 19, 2021

type bug-fix

@tvami
Copy link
Contributor

tvami commented Sep 19, 2021

@iarspider can you please include a little bit more information in the PR description? For example a pointer to the build error? Could you also delete the "Before submitting your pull requests, make sure you followed this checklist:" part?

@tvami
Copy link
Contributor

tvami commented Sep 19, 2021

+db

@tvami
Copy link
Contributor

tvami commented Sep 19, 2021

+alca

  • technical PR, the build error is gone

@smuzaffar
Copy link
Contributor

@cms-sw/l1-l2 , can you please review this? It is just a technical change

@qliphy
Copy link
Contributor

qliphy commented Sep 21, 2021

merge
technical fix

@cmsbuild cmsbuild merged commit 87fa9df into cms-sw:master Sep 21, 2021
@iarspider iarspider deleted the patch-1 branch September 22, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants