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

cleanup of py2 packages; make sure to install their dependencies #4482

Merged
merged 6 commits into from Nov 8, 2018

Conversation

smuzaffar
Copy link
Contributor

@smuzaffar smuzaffar commented Nov 6, 2018

Cleanup for py2-* packages. These changes work with pkgtools V00-32-XX branch.

  • match cms package name to pip package name. . in package name should be replaced with - e.g. pip package backports.lzma becomes py2-backports-lzma.spec
  • dropped most of py2-*-toolfile.spec and automatically generate these via cmssw-tool-conf.spec
  • Make sure that dependency of a py2-* package satisfied and fail at build time if missing
  • Simplified py2-* packages to be of the form
### RPM external py2-PipPackageName Version
## IMPORT build-with-pip

<extra build/requires flags>

one day we can auto generate these from a single file. For packages with <extra build/requires flags> can have py2-PipPackageName.file which can be included in the auto-generated package specs (using new ## INCLUDE file-name feature of cmsBuild).

  • make build-with-pip.file generic enough so that we can customize the build from the package spec.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2018

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for branch IB/CMSSW_10_4_X/gcc700.

@cmsbuild, @smuzaffar, @gudrutis, @mrodozov can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@smuzaffar
Copy link
Contributor Author

FYI @davidlange6 and @amaltaro

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31495/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2018

Pull request #4482 was updated.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31496/console

@davidlange6
Copy link
Contributor

hi @smuzaffar - cool this will really help.

We talked some time ago about dependencies. You've removed the few declared dependencies that the py2*spec files had. So this means if package X really does depend at build time on package py2-Y, then it needs to declare that dependency and any others in the full dependency tree of py2-Y. Is that right?

Hopefully we will continue to have o(1) such package (tensorflow) where this is important.

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Nov 6, 2018

@davidlange6 , yes package X needs to declare its dependencies and it will also recursively get all the dependencies from packages its depend on.

I only added the dependencies which are needed. e.g. I removed 47 deps of py2-jupyter.spec but all of them should come from py2-ipywidgets py2-jupyter_console py2-qtconsole (56 in total).

@amaltaro
Copy link

amaltaro commented Nov 6, 2018

Thanks Shahzad. Once this gets merged, I'll pick your build-with-pip changes and adapt according to the comp_gcc630 needs (aka PYTHONPATH only, since we don't have the same py2/py3 distinction as in cmssw).

The question I had is about the pkgtools tag. Do you see any reason why we shouldn't start using the 00-32 tag ASAP in comp_gcc630?

@smuzaffar
Copy link
Contributor Author

@amaltaro , there is no reason why you should not be able to move to V00-32. It is back word compatible.
In order to test it I would suggest that just take an existing tag of cmsdist/comp_gcc630 and try to build comp or any other package (which is already available in comp repo) using V00-32/cmsBuild. If it re-builds any package then we have a issue and we should fix it.

@amaltaro
Copy link

amaltaro commented Nov 6, 2018

Ok, I built comp with the pkgtools V00-32-XX and it actually didn't build anything, since all the sources were already available in the repo.
I'll circulate an email tomorrow to Lina and the service responsibles asking to upgrade pkgtools to this version. Is this pkgtools branch required to work with the new build-with-pip changes? Or there are other important features in 00-32 that aren't available in 00-30?

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2018

-1

Tested at: f633865

  • Build:

I found compilation error when building:

py2-networkx 2.2
py2-nose 1.3.7
py2-pymongo 3.7.2
py2-six 1.11.0
+ exit 1
error: Bad exit status from /build/cmsbld/jenkins/workspace/ib-any-integration/testBuildDir/tmp/rpm-tmp.zRnok2 (%build)


RPM build errors:
Bad exit status from /build/cmsbld/jenkins/workspace/ib-any-integration/testBuildDir/tmp/rpm-tmp.zRnok2 (%build)



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

@smuzaffar
Copy link
Contributor Author

@amaltaro, V00-32 works with both (old and new) build-with-pip.

@smuzaffar
Copy link
Contributor Author

hold
I have a better option for checking the dependencies. looks like with this current change ever package download its dependencies sources which is not good as base level packages sources are downloaded multiple times. As we have a relatively newer pip in cmsdist, so I can make use of pip show and pip check to make sure if package dependencies are resolved.

I will update this PR later with the changes for build-with-pip

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2018

Pull request has been put on hold by @smuzaffar
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31530/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

Pull request #4482 was updated.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

Pull request #4482 was updated.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-4482/7.3_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3013827
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3013629
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 134 log files, 14 edm output root files, 32 DQM output files

@smuzaffar
Copy link
Contributor Author

+externals

@smuzaffar smuzaffar merged commit 995b143 into IB/CMSSW_10_4_X/gcc700 Nov 8, 2018
### RPM external py2-pyOpenSSL 18.0.0
## IMPORT build-with-pip

Requires: py2-cryptography
Copy link

Choose a reason for hiding this comment

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

you missed a dependency on py2-six here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smuzaffar smuzaffar deleted the py2-cleanup branch November 16, 2018 08:55
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

4 participants