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

Update dasgoclient to use revXX as version suffix, fix for finding correct exec #8200

Merged
merged 4 commits into from Nov 26, 2022

Conversation

smuzaffar
Copy link
Contributor

@smuzaffar smuzaffar commented Nov 25, 2022

  • Use dasgoclient_tag.revXX as dasgoclient package directory to make sure we get the latest build of it.
  • Drop extra hash from the install path
  • Use CMSDIST_FILE_REVISION feature of cmsdist to copy the latest script back to cms_path/common

@smuzaffar
Copy link
Contributor Author

FYI @vkuznet

@cmsbuild
Copy link
Contributor

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

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

Pull request #8200 was updated.

@vkuznet
Copy link
Contributor

vkuznet commented Nov 25, 2022

@smuzaffar , could you please elaborate an use-case why do we need such changes now? My understanding was that once new tag version of dasgoclient appears we only change it and make one build. Why do we need now a date?

@smuzaffar
Copy link
Contributor Author

@vkuznet , currently we are using dasgoclient tag as version so we get cms/dasgoclient/<dasgoclient_tag>-<hash1> [a] . Now if we have to change the contents on package itself (e.g. in this case I had to update the dasgoclient wrapper ) then the newly build package will be <dasgoclient_tag>-<hash2> and there is no simple way to find which one is newer.

[a]

> ls /cvmfs/cms.cern.ch/el8_amd64_gcc11/cms/dasgoclient
v02.04.48-f313cb6ef87ad3c001647c10c2b6837e 
v02.04.49-ccf3472cbe7cb8eb7e8ab0f565628681

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Nov 25, 2022

if you do not like the date a version then we can add a suffix e.g. <dasgoclient_tag>.r00 ( just like we do for crab https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_13_0_X/master/crab-dev.spec )

@vkuznet
Copy link
Contributor

vkuznet commented Nov 25, 2022

@smuzaffar , thanks for explanation. In this case I rather prefer CRAB approach but I suggest to improve it further. Since the suffix is CMS specific part, it should be in a form, -cms-01 or -cmsbuild01. Doing this way we can increment suffix, and it will be distinguished from naming convention used by tag of the package. Do you mind to switch to suffix approach instead of a date?

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Nov 25, 2022

@vkuznet , sure I can switch to suffix approach. I can not use -cmsXX or anything which starts with -c as this will not help with sorting the old version which can also have -[0-9a-f]. I suggest to use -revXX

@vkuznet
Copy link
Contributor

vkuznet commented Nov 25, 2022

ok, let's settle on -revXX suffix then.

@cmsbuild
Copy link
Contributor

Pull request #8200 was updated.

@smuzaffar smuzaffar changed the title Update dasgoclientto use date as version. fix for finding correct exec Update dasgoclient to use revXX as version suffix, fix for finding correct exec Nov 25, 2022
@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dcbabe/29262/summary.html
COMMIT: 776f836
CMSSW: CMSSW_13_0_X_2022-11-25-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8200/29262/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation warning when building: See details on the summary page.

@cmsbuild
Copy link
Contributor

Pull request #8200 was updated.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dcbabe/29269/summary.html
COMMIT: d6d0faa
CMSSW: CMSSW_13_0_X_2022-11-25-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8200/29269/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dcbabe/29269/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dcbabe/29269/git-merge-result

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3417239
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3417205
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

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

3 participants