Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#26351 closed enhancement (fixed)

build/pkgs/*/checksums.ini: Add upstream_url field

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.1
Component: build Keywords:
Cc: Erik Bray, Dima Pasechnik, Volker Braun, John Palmieri, Travis Scrimshaw Merged in:
Authors: Matthias Koeppe Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 537210d (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

This is to make testing tickets that include SPKG upgrades easier for developers and patchbots.

To add the upstream_url field, just edit checksums.ini or use the new option --url of sage -package create:

$ ./sage -package create --help
usage: sage --package create [-h] [--version VERSION] [--tarball TARBALL]
                             [--type TYPE] [--url URL]
                             [package_name]

[...]
  --tarball TARBALL  Tarball filename pattern, e.g. Foo-VERSION.tar.bz2
  --type TYPE        Package type
  --url URL          Download URL pattern, e.g. http://example.org/Foo-
                     VERSION.tar.bz2

The default behavior of sage -i is not changed: Packages are only downloaded from the Sage mirrors.

To allow downloading from the upstream URL, use sage -i --allow-upstream SPKG.

Example (for qepcad) at #28388.

Change History (30)

comment:1 Changed 4 years ago by Erik Bray

That's a good idea. It should still be synced to our mirrors primarily, but having a canonical "original" URL (even if it's just an attachment to a trac ticket) would provide the primary source for a tarball independent of the mirrors.

comment:2 Changed 4 years ago by Erik Bray

I'll take a stab at this. What I'm thinking is sage-download-file could try all mirrors first, but failing that try the upstream-tarball-url as a last resort. This way, if that field is filled in, the download should still work even if the tarball is not on the mirrors yet (this will be great for people manually testing new SPKGs too, since it means they won't have to manually download the tarball into upstream/).

The upstream-tarball-url should also be optional, so that we don't have to go adding it for every package. But it should be encouraged to add it when adding new SPGKs or updating existing ones. Additionally, sage --package fix-checksum could test the URL to make sure it's still valid, and sage-download-file could grow a --no-mirrors option to skip the mirrors entirely if there is an upstream-tarball-url (useful if nothing else for testing).

comment:3 Changed 4 years ago by Matthias Köppe

Sounds good, thank you!

comment:4 Changed 4 years ago by Matthias Köppe

Any news on this?

comment:5 Changed 3 years ago by Matthias Köppe

Cc: Dima Pasechnik added
Milestone: sage-8.4sage-9.1

comment:6 Changed 3 years ago by Matthias Köppe

Branch: u/mkoeppe/build_pkgs___checksums_ini__add_upstream_tarball_url_field

comment:7 Changed 3 years ago by Matthias Köppe

Authors: Matthias Koeppe
Cc: Volker Braun added
Commit: 537210da655c72cc8f64ad72d664334e07b662c5
Description: modified (diff)
Status: newneeds_review
Summary: build/pkgs/*/checksums.ini: Add upstream-tarball-url fieldbuild/pkgs/*/checksums.ini: Add upstream_url field

New commits:

cb6f4f3build/tox.ini: Add more python versions
f421bd7build/test: Set maxDiff=None to improve debugging of sage_bootstrap
c02af0dbuild/sage_bootstrap: Handle upstream_url field in checksum.ini, add options to 'sage -package'
5b4a9e8build/sage_bootstrap/download: Add option --allow-upstream
537210dbuild/bin/sage-spkg: Add option -o (--allow-upstream)

comment:8 Changed 3 years ago by Matthias Köppe

Description: modified (diff)

comment:9 Changed 3 years ago by Matthias Köppe

Description: modified (diff)

comment:10 Changed 3 years ago by Matthias Köppe

Cc: John Palmieri Travis Scrimshaw added

comment:11 Changed 3 years ago by Matthias Köppe

Anyone interested in this ticket?

comment:12 Changed 3 years ago by Travis Scrimshaw

Everything looks fine to me from what I understand, but I don't feel qualified enough to set this to a positive review.

comment:13 Changed 3 years ago by John Palmieri

What are the lines maxDiff = None for? And how are we supposed to test that this works?

Edit: I mean, I see the commit message "build/test: Set maxDiff=None to improve debugging of sage_bootstrap", but can you expand on this?

Last edited 3 years ago by John Palmieri (previous) (diff)

comment:14 in reply to:  13 Changed 3 years ago by Dima Pasechnik

comment:15 in reply to:  13 Changed 3 years ago by Matthias Köppe

Replying to jhpalmieri:

What are the lines maxDiff = None for? And how are we supposed to test that this works?

Edit: I mean, I see the commit message "build/test: Set maxDiff=None to improve debugging of sage_bootstrap", but can you expand on this?

Without this change, failed test assertions did not show enough useful information. Now, of course, nothing fails, so these lines have no effect on the operation of sage_bootstrap.

If you want to test that this ticket works, may I suggest picking a package that needs upgrading anyway, and use the new upstream_url mechanism to do the upgrade.

comment:16 Changed 3 years ago by Erik Bray

I have a package to upgrade, so I'll try doing it with this feature.

comment:17 Changed 3 years ago by Erik Bray

Sort of unrelated, but why does tox.ini have so many old Python 3.x versions? The minimal version I've been able to get Sage working on is 3.6.

comment:18 Changed 3 years ago by Erik Bray

Can you please also apply the following patch:

diff --git a/build/sage_bootstrap/cmdline.py b/build/sage_bootstrap/cmdline.py
index abb617dd65..3fa9de4104 100644
--- a/build/sage_bootstrap/cmdline.py
+++ b/build/sage_bootstrap/cmdline.py
@@ -219,14 +219,14 @@ def make_parser():
     parser_fix_checksum.add_argument(
         'package_name', nargs='?', default=None, type=str,
         help='Package name. Default: fix all packages.')
-    
+
     parser_create = subparsers.add_parser(
         'create', epilog=epilog_create,
         formatter_class=argparse.RawDescriptionHelpFormatter,
         help='Create or overwrite package.')
     parser_create.add_argument(
-        'package_name', nargs='?', default=None, type=str,
-        help='Package name. Default: fix all packages.')
+        'package_name', default=None, type=str,
+        help='Package name.')
     parser_create.add_argument(
         '--version', type=str, default=None, help='Package version')
     parser_create.add_argument(

running sage -package create doesn't work without a package name argument, so it shouldn't be optional (it looks like it was just incorrectly copied from the fix-checksum command).

comment:19 in reply to:  17 Changed 3 years ago by Erik Bray

Replying to embray:

Sort of unrelated, but why does tox.ini have so many old Python 3.x versions? The minimal version I've been able to get Sage working on is 3.6.

Nevermind, I misunderstood that these are the tests for the bootstrap scripts.

comment:20 Changed 3 years ago by Erik Bray

Reviewers: Erik Bray

If you want to fix the issue above as part of this ticket please do; it wasn't clear to me whether or not this bug predates this ticket (it probably does). Whether you decide to or not you can set this to positive_review afterwards.

comment:21 Changed 3 years ago by Matthias Köppe

Status: needs_reviewpositive_review

Thanks for reviewing. Separate ticket please for the patch.

comment:22 in reply to:  21 Changed 3 years ago by Matthias Köppe

Replying to mkoeppe:

Separate ticket please for the patch.

That's #29286

comment:23 Changed 3 years ago by Volker Braun

Status: positive_reviewneeds_work

Merge conflict

comment:24 Changed 3 years ago by Volker Braun

Status: needs_workneeds_info

Actually: There is no merge to abort (MERGE_HEAD missing).

comment:25 Changed 3 years ago by Volker Braun

Status: needs_infopositive_review

comment:26 Changed 3 years ago by Volker Braun

Branch: u/mkoeppe/build_pkgs___checksums_ini__add_upstream_tarball_url_field537210da655c72cc8f64ad72d664334e07b662c5
Resolution: fixed
Status: positive_reviewclosed

comment:27 Changed 3 years ago by Dima Pasechnik

Commit: 537210da655c72cc8f64ad72d664334e07b662c5

I am trying to test #29444, and upstream_url does not seem to work (either by running make, or with ./sage -i) What am I doing wrong?

comment:28 Changed 3 years ago by John Palmieri

I think you're supposed to do make SAGE_SPKG="sage-spkg -o" ...

comment:29 Changed 3 years ago by Dima Pasechnik

Thanks. Is there any reason for this extra verbosity? After all, the checksums are supposed to identify the tarball well enough, so as long as they are not tampered with, downloading from upstream_url is safe.

comment:30 Changed 3 years ago by Matthias Köppe

#29392 proposes a nicer user interface to this feature. I didn't make it the default because it's a potentially controversial feature.

Note: See TracTickets for help on using tickets.