Opened 3 years ago

Closed 13 months ago

Last modified 13 months ago

#26351 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.1
Component: build Keywords:
Cc: embray, dimpase, vbraun, jhpalmieri, tscrim 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 mkoeppe)

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 3 years ago by embray

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 3 years ago by embray

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 3 years ago by mkoeppe

Sounds good, thank you!

comment:4 Changed 2 years ago by mkoeppe

Any news on this?

comment:5 Changed 14 months ago by mkoeppe

  • Cc dimpase added
  • Milestone changed from sage-8.4 to sage-9.1

comment:6 Changed 14 months ago by mkoeppe

  • Branch set to u/mkoeppe/build_pkgs___checksums_ini__add_upstream_tarball_url_field

comment:7 Changed 14 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc vbraun added
  • Commit set to 537210da655c72cc8f64ad72d664334e07b662c5
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from build/pkgs/*/checksums.ini: Add upstream-tarball-url field to build/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 14 months ago by mkoeppe

  • Description modified (diff)

comment:9 Changed 14 months ago by mkoeppe

  • Description modified (diff)

comment:10 Changed 14 months ago by mkoeppe

  • Cc jhpalmieri tscrim added

comment:11 Changed 14 months ago by mkoeppe

Anyone interested in this ticket?

comment:12 Changed 14 months ago by tscrim

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 follow-ups: Changed 14 months ago by 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?

Last edited 14 months ago by jhpalmieri (previous) (diff)

comment:14 in reply to: ↑ 13 Changed 14 months ago by dimpase

comment:15 in reply to: ↑ 13 Changed 14 months ago by mkoeppe

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 14 months ago by embray

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

comment:17 follow-up: Changed 14 months ago by 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.

comment:18 Changed 14 months ago by embray

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 14 months ago by embray

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 14 months ago by embray

  • Reviewers set to 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 follow-up: Changed 14 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Thanks for reviewing. Separate ticket please for the patch.

comment:22 in reply to: ↑ 21 Changed 14 months ago by mkoeppe

Replying to mkoeppe:

Separate ticket please for the patch.

That's #29286

comment:23 Changed 13 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:24 Changed 13 months ago by vbraun

  • Status changed from needs_work to needs_info

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

comment:25 Changed 13 months ago by vbraun

  • Status changed from needs_info to positive_review

comment:26 Changed 13 months ago by vbraun

  • Branch changed from u/mkoeppe/build_pkgs___checksums_ini__add_upstream_tarball_url_field to 537210da655c72cc8f64ad72d664334e07b662c5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:27 Changed 13 months ago by dimpase

  • Commit 537210da655c72cc8f64ad72d664334e07b662c5 deleted

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 13 months ago by jhpalmieri

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

comment:29 Changed 13 months ago by dimpase

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 13 months ago by mkoeppe

#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.