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.

Sign up for GitHub

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

Jump to bottom

Change pip downloading sources to pypi instead of via pip itself #175

Merged
merged 1 commit into from
Apr 27, 2021
Merged

Change pip downloading sources to pypi instead of via pip itself #175

merged 1 commit into from
Apr 27, 2021

Conversation

davidlange6
Copy link
Contributor

33-xx version of #174

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlange6 (David Lange) for branch V00-33-XX.

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

mrodozov
mrodozov reviewed Apr 21, 2021
View reviewed changes
cmsBuild
for file in data["releases"][pack.split('=')[2]]:
if file["packagetype"] == "sdist":
url=file["url"]
if url is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if url is not None:
if url:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems is not None is the more standard formulation.. "Comparisons to singletons like None should always be done with is or is not, never the equality operators."

@davidlange6
Copy link
Contributor Author

hi @mrodozov @smuzaffar -aside from the small fix suggested above, any other comments on this?

@mrodozov
Copy link
Contributor

it says draft so I thought you still have work on it

@davidlange6
Copy link
Contributor Author

davidlange6 commented Apr 27, 2021 via email

@smuzaffar
Copy link
Contributor

@davidlange6 , I am also testing it locally by forcing re-build of all pip based packages and re-download of sources . I will get back once testing is done

@davidlange6
Copy link
Contributor Author

davidlange6 commented Apr 27, 2021 via email

@smuzaffar
Copy link
Contributor

you need to change the url checksum. I did it by changing cmsBuild itself [a] but you can also do it by adding a dummy parameter to pip source url in cmsdist/build-with-pip.file e.g.

%define source0 pip://%{pip_name}/%{realversion}?pip_options=%{PipDownloadOptions}&foo=bar&pip=%{pip}&output=/%{pkgsource}

[a]

@@ -790,6 +791,9 @@ def download(source, dest, options, pkg=None):
     noCmssdtCache = True if 'no-cmssdt-cache=1' in source else False
     source = fixUrl(source)
     checksum = getUrlChecksum(source)
+    print("SOURCE: %s" % source)
+    if source.startswith('pip://'):
+      checksum = getUrlChecksum(source+"/foo")

     # Syntactic sugar to allow the following urls for tag collector:

@smuzaffar
Copy link
Contributor

+externals

changes looks good. These are the packages [a] for which the new code was hit. The issue with boost-histogram==0.13 is due to wrong version in cmsdist. It should have been 0.13.0.

[a]

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next V00-33-XX IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@davidlange6 davidlange6 changed the title draft of downloading sources from pypi instead of pip Change pip downloading sources to pypi instead of via pip itself Apr 27, 2021
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f3442/14610/summary.html
COMMIT: 19640d2
CMSSW: CMSSW_12_0_X_2021-04-26-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/pkgtools/175/14610/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2877605
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2877576
  • 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

@smuzaffar smuzaffar merged commit 6088047 into cms-sw:V00-33-XX Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrodozov mrodozov mrodozov left review comments

Assignees
No one assigned
Labels
externals-approved fully-signed orp-pending tests-approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@davidlange6 @cmsbuild @mrodozov @smuzaffar