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

ParameterSet/python/Config.py does not allow full release imports #46937

Closed
smuzaffar opened this issue Dec 13, 2024 · 7 comments · Fixed by #46955
Closed

ParameterSet/python/Config.py does not allow full release imports #46937

smuzaffar opened this issue Dec 13, 2024 · 7 comments · Fixed by #46955

Comments

@smuzaffar
Copy link
Contributor

@makortel , cms-sw/cmssw-config@91e7bf6 change was using the realpath of imported python package. This was done so that edm class check script can import FWCore.Reflection.ClassesDefXmlUtils during the build phase of patch release. This change broke TestIntegrationParameterSet unit test. Here is what python directory looks like in patch release

  • If FWCore is not part of patch release then we create symlink cmssw-patch/python/FWCore pointing to full release
cmssw-patch/python/FWCore  -- symlink to --> cmssw/python/FWCore
  • if FWCore is part of patch release then we create cmssw-patch/python/FWCore directory and create symlink for Integration
cmssw-patch/python/FWCore/Integration  -- symlink to --> cmssw/python/FWCore/Integration

After the cmssw-config update for python imports, in patch releases , imported python module path starts with

  • CMSSW_FULL_RELEASE_BASE: If package is not part of patch release
  • CMSSW_RELEASE_BASE: If package is part of patch release
  • CMSSW_BASE: In devel area, if patch was locally checkout

The reason unit test https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc12/CMSSW_15_0_X_2024-12-13-1100/unitTestLogs/FWCore/Integration#/3356-3356 start failing was that https://github.com/cms-sw/cmssw/blob/master/FWCore/ParameterSet/python/Config.py#L55 only allows CMSSW_RELEASE_BASE and CMSSW_BASE. For now I have reverted the cmssw-config change. But do you think we should update https://github.com/cms-sw/cmssw/blob/master/FWCore/ParameterSet/python/Config.py#L55 and also allow CMSSW_FULL_RELEASE_BASE ?

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 13, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @smuzaffar.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor Author

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@smuzaffar smuzaffar changed the title ParameterSet/python/Config.py doe snot allow full release imports ParameterSet/python/Config.py does not allow full release imports Dec 13, 2024
@makortel
Copy link
Contributor

But do you think we should update https://github.com/cms-sw/cmssw/blob/master/FWCore/ParameterSet/python/Config.py#L55 and also allow CMSSW_FULL_RELEASE_BASE ?

I think CMSSW_FULL_RELEASE_BASE can be added there. It also seems to me the checkImportPermission() is not used for anything besides those tests.

@makortel
Copy link
Contributor

+core

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants