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

removing useless files (old phase2 pixel clusters ) #19360

Merged
merged 1 commit into from Jun 20, 2017

Conversation

boudoul
Copy link
Contributor

@boudoul boudoul commented Jun 19, 2017

Those files were originally developed for the pixel phase2 clusters but are eventually not used and for sure obsolete , i'm removing to clean and prevent any uaage of them by mistake .

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @boudoul (boudoul) for master.

It involves the following packages:

RecoLocalTracker/Phase2ITPixelClusterizer

@perrotta, @cmsbuild, @slava77, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @gpetruc, @ebrondol, @threus this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@boudoul
Copy link
Contributor Author

boudoul commented Jun 19, 2017

adding @atricomi , @delaere

@slava77
Copy link
Contributor

slava77 commented Jun 19, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 19, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20741/console Started: 2017/06/19 18:17

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1803204
  • DQMHistoTests: Total failures: 44266
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1758772
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@slava77
Copy link
Contributor

slava77 commented Jun 20, 2017

+1

for #19360 e3d6358

  • removal of obsolete code RecoLocalTracker/Phase2ITPixelClusterizer
  • jenkins tests pass

@kpedro88
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit d14e638 into cms-sw:master Jun 20, 2017
@davidlange6
Copy link
Contributor

hi @boudoul - looks like there are problems from this PR:

File "/cvmfs/cms-ib.cern.ch/nweek-02477/slc7_amd64_gcc630/cms/cmssw/CMSSW_9_2_X_2017-06-21-1100/python/RecoLocalTracker/Configuration/RecoLocalTracker_cff.py", line 21, in
from RecoLocalTracker.Phase2ITPixelClusterizer.Phase2ITPixelClusterizer_cfi import *
ImportError: No module named Phase2ITPixelClusterizer.Phase2ITPixelClusterizer_cfi

@boudoul
Copy link
Contributor Author

boudoul commented Jun 21, 2017

whoopsie , will look

@slava77
Copy link
Contributor

slava77 commented Jun 21, 2017 via email

@Dr15Jones
Copy link
Contributor

This broke the IB because the deleted files are being imported:
https://github.com/cms-sw/cmssw/blob/master/RecoLocalTracker/Configuration/python/RecoLocalTracker_cff.py#L21

@slava77
Copy link
Contributor

slava77 commented Jun 21, 2017 via email

@Dr15Jones
Copy link
Contributor

@smuzaffar you would know better about how the build system is meant to handle removed python files.

@davidlange6
Copy link
Contributor

davidlange6 commented Jun 21, 2017 via email

@smuzaffar
Copy link
Contributor

smuzaffar commented Jun 22, 2017

@davidlange6 , @Dr15Jones and @slava77 : build system only takes care of deleted python files but there is no protection for python files when either python directory or full package is deleted. Last time I added protection for edm plugins deleted in scram dev area (either due to BuildFile.xml changes or removal of package) via poisoned edm cache.

I will add some protection for deleted python dirs too.

smuzaffar added a commit to cms-sw/cmsdist that referenced this pull request Jun 22, 2017
new tag generates python/package/__init__.py for all the packages (even if there is no src/package/python directory). This fixes the issue we have seen due to PR cms-sw/cmssw#19360 where RecoLocalTracker/Phase2ITPixelClusterizer/python was deleted and we only noticed the problem when full release was built.

New build rules also create python/package/__init__.py for deleted packages
@slava77
Copy link
Contributor

slava77 commented Jun 22, 2017 via email

@smuzaffar
Copy link
Contributor

I already have a new cmssw-config tag (being tested here #19360) which should fix this. After successful testing in DEVEL IBs (for a week), I will include it in normal IBs.

@boudoul
Copy link
Contributor Author

boudoul commented Jun 22, 2017

thank you everybody and sorry for the inconvenince with the broken IBs ... I should have myself caught this while preparing the PR ..

cmsbuild pushed a commit to cms-sw/cmsdist that referenced this pull request Jun 28, 2017
new tag generates python/package/__init__.py for all the packages (even if there is no src/package/python directory). This fixes the issue we have seen due to PR cms-sw/cmssw#19360 where RecoLocalTracker/Phase2ITPixelClusterizer/python was deleted and we only noticed the problem when full release was built.

New build rules also create python/package/__init__.py for deleted packages
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

7 participants