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

move pybind11 to python3 only #6996

Merged
merged 4 commits into from Jun 16, 2021

Conversation

davidlange6
Copy link
Contributor

This PR takes advantage of pybind11 being used as a header only package having only a loose connection to python version. The pybind11 toolfile should be renamed. This will be done as a follow-on PR as it also involves cmssw changes

…will be done as a follow-on PR as it also involves cmssw changes
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

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

@cmsbuild, @smuzaffar, @mrodozov, @iarspider can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@davidlange6
Copy link
Contributor Author

please test

@mrodozov
Copy link
Contributor

mrodozov commented Jun 8, 2021

the test is failing, I think it's scram (or something scram doesn't read as expected)

13:09:38 Name : py2-pybind11
13:09:38 Traceback (most recent call last):
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/bin/scram.py", line 114, in <module>
13:09:38     main()
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/bin/scram.py", line 109, in main
13:09:38     if not execcommand(args, opts):
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/bin/scram.py", line 103, in execcommand
13:09:38     return eval('scram_commands.cmd_%s' % cmds[0])(args, opts)
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/SCRAM/Core/CMD.py", line 43, in cmd_setup
13:09:38     return process(args)
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/SCRAM/Core/Commands/setup.py", line 60, in process
13:09:38     toolmanager.setupalltools()
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/SCRAM/BuildSystem/ToolManager.py", line 64, in setupalltools
13:09:38     self.coresetup(toolfile, dump)
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/SCRAM/BuildSystem/ToolManager.py", line 86, in coresetup
13:09:38     self._update_json(tooljson, dump)
13:09:38   File "/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/share/lcg/SCRAMV1/V3_00_23/SCRAM/BuildSystem/ToolManager.py", line 113, in _update_json
13:09:38     printmsg("Version : %s" % self.xml.contents['TOOLVERSION'])
13:09:38 KeyError: 'TOOLVERSION'
13:09:38 + ERR=1

@davidlange6
Copy link
Contributor Author

davidlange6 commented Jun 8, 2021 via email

@@ -1,5 +1,5 @@
### RPM external py2-pybind11-toolfile 1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidlange6 , rename the spec to py3-pybind11.spec and also update the name in the line above

@smuzaffar
Copy link
Contributor

@davidlange6 , as cmsRun still uses py2, so we will still need the py2-pybind11

@davidlange6
Copy link
Contributor Author

davidlange6 commented Jun 8, 2021 via email

@davidlange6
Copy link
Contributor Author

please test

(I'm building cmssw locally as well - but it seems better now)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

Pull request #6996 was updated.

@smuzaffar
Copy link
Contributor

@davidlange6 , did you see my comment https://github.com/cms-sw/cmsdist/pull/6996/files#r647386284 ? the toolfile name and package name should match. if package is called py3-pybind11 then toolfile spec should be py3-pybind11-toolfile otherwise the logic https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_12_0_X/master/scram-tools-post.file#L4-L7 fails

@davidlange6
Copy link
Contributor Author

ok, no - I didn't. - I'll abort and find another approach.

@davidlange6
Copy link
Contributor Author

please abort

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

Pull request #6996 was updated.

@davidlange6
Copy link
Contributor Author

this will be tested with cmssw changes in #34026..

%install

mkdir -p %{i}/etc/scram.d
cat << \EOF_TOOLFILE >%{i}/etc/scram.d/py2-pybind11.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidlange6 , please also update the xml file name. It should be py3-pybind11.xml

@davidlange6
Copy link
Contributor Author

davidlange6 commented Jun 8, 2021 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

Pull request #6996 was updated.

@smuzaffar
Copy link
Contributor

please test

@davidlange6
Copy link
Contributor Author

davidlange6 commented Jun 8, 2021 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fb9d93/15767/summary.html
COMMIT: d75534d
CMSSW: CMSSW_12_0_X_2021-06-08-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/6996/15767/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: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2862520
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2862497
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Jun 16, 2021

tested with cms-sw/cmssw#34026

@qliphy
Copy link
Contributor

qliphy commented Jun 16, 2021

+1

@qliphy
Copy link
Contributor

qliphy commented Jun 16, 2021

merge

@ggovi
Copy link
Contributor

ggovi commented Jun 18, 2021

+1

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