#31034 closed enhancement (fixed)

Fix sage -package download --allow-upstream, add option --on-error=warn, add filtering to sage -package upload

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.3
Component: distribution Keywords:
Cc: vbraun, gh-kliem, gh-tobiasdiez, dimpase, jipilab, slelievre, slabbe Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem, Volker Braun
Report Upstream: N/A Work issues:
Branch: 274db61 (Commits, GitHub, GitLab) Commit: 274db614ef204d682e15cebdf76c0df648c7c7e5
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

After the fix from this ticket, we should be able to automate syncing upstream packages to the Sage server by using

./sage -package download --allow-upstream --on-error=warn :all:

followed by

./sage -package upload :all:

... if someone is able to set up the necessary credentials for the workflow.

We also add a mechanism to exclude non-distributable tarballs: If a tarball name includes the pattern do-not-distribute, sage -package upload now skips it and logs a message. We change the name of the tarball of scipoptsuite accordingly - this is our only non-distributable tarball.

Change History (41)

comment:1 Changed 12 months ago by mkoeppe

  • Cc slelievre added
  • Description modified (diff)

comment:2 Changed 12 months ago by dimpase

it might be more reasonable to do this with a special GH action. Then it can be run on e.g. Volker's repo by him, e.g. while making a beta.

comment:3 Changed 11 months ago by vbraun

I tried to put it into my release management script but it doesn't actually try to download from the upstream_url:

$ cat build/pkgs/pynormaliz/checksums.ini 
tarball=PyNormaliz-VERSION.tar.gz
sha1=c01c7a734deeb09e1dd236fb53575b152b99657b
md5=6ff9ccc61592190fdb88fe08a50e062c
cksum=3111232777
upstream_url=https://pypi.io/packages/source/p/pynormaliz/PyNormaliz-VERSION.tar.gz

$ ./sage -package download --allow-upstream pynormaliz
[...]
http://sagepad.org/spkg/upstream/pynormaliz/PyNormaliz-2.13.tar.gz
[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
ERROR [transfer|run:135]: [Errno 404] Not Found: '//sagepad.org/spkg/upstream/pynormaliz/PyNormaliz-2.13.tar.gz'
Traceback (most recent call last):
  File "/home/release/Sage/build/bin/sage-package", line 42, in <module>
    run()
  File "/home/release/Sage/build/bin/../sage_bootstrap/cmdline.py", line 339, in run
    app.download_cls(args.package_name, args.allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 177, in download_cls
    pc.apply(self.download)
  File "/home/release/Sage/build/bin/../sage_bootstrap/expand_class.py", line 71, in apply
    function(package_name, *args, **kwds)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 167, in download
    package.tarball.download(allow_upstream=allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/tarball.py", line 177, in download
    raise FileNotMirroredError('tarball does not exist on mirror network')
sage_bootstrap.tarball.FileNotMirroredError: tarball does not exist on mirror network

Is there anything that I'm doing wrong?

comment:4 Changed 11 months ago by dimpase

./configure must be run with --enable-download-from-upstream-url

comment:5 Changed 11 months ago by vbraun

Is the --allow-upstream actually doing anything or is only the configure switch relevant?

comment:6 Changed 11 months ago by mkoeppe

./sage -package does NOT respect the configure switch, it only handles --allow-upstream... which, however, was indeed broken

comment:7 Changed 11 months ago by mkoeppe

  • Branch set to u/mkoeppe/gh_actions_workflow_to_sync_upstream_packages_to_the_sage_server

comment:8 Changed 11 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 0e18239948db03964ce5a81fbba86e74084a86ce

New commits:

0e18239sage --package download: Fix --allow-upstream

comment:9 Changed 11 months ago by mkoeppe

  • Status changed from new to needs_review

comment:10 Changed 11 months ago by mkoeppe

  • Cc slabbe added
  • Description modified (diff)
  • Summary changed from GH Actions workflow to sync upstream packages to the Sage server to Fix sage -package download --allow-upstream

comment:11 Changed 11 months ago by gh-kliem

Why did you update pynormaliz to 7.77?

comment:12 Changed 11 months ago by mkoeppe

Sorry...

comment:13 Changed 11 months ago by git

  • Commit changed from 0e18239948db03964ce5a81fbba86e74084a86ce to 34335ca0765c0b68cfc36bec81bc4b626e810fd2

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

34335casage --package download: Fix --allow-upstream

comment:14 Changed 11 months ago by gh-kliem

This ticket looks like a left over from somewhere else.

I mean, why was download_with_args defined but never used?

comment:15 Changed 11 months ago by mkoeppe

Probably I messed it up in a merge.

comment:16 Changed 11 months ago by gh-kliem

  • Status changed from needs_review to positive_review

LGTM.

comment:17 follow-up: Changed 11 months ago by gh-kliem

Btw, pynormaliz still isn't on the mirrors, this is how I tested it. It was convenient for this purpose, but isn't otherwise.

comment:18 Changed 11 months ago by mkoeppe

  • Reviewers set to Jonathan Kliem

Thank you!

comment:19 in reply to: ↑ 17 Changed 11 months ago by slelievre

Replying to gh-kliem:

Btw, pynormaliz still isn't on the mirrors, this is how I tested it. It was convenient for this purpose, but isn't otherwise.

Fixed now. Thanks Volker!

comment:20 Changed 11 months ago by vbraun

To run this unattended we also need to ignore tarballs that we are not allowed to distribute for licensing reasons:

$ echo sage -package download --allow-upstream :all:
[...]
http://sagepad.org/spkg/upstream/scipoptsuite/scipoptsuite-5.0.1.tgz
[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
ERROR [transfer|run:135]: [Errno 404] Not Found: '//sagepad.org/spkg/upstream/scipoptsuite/scipoptsuite-5.0.1.tgz'
Traceback (most recent call last):
  File "/home/release/Sage/build/bin/sage-package", line 42, in <module>
    run()
  File "/home/release/Sage/build/bin/../sage_bootstrap/cmdline.py", line 339, in run
    app.download_cls(args.package_name, args.allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 177, in download_cls
    pc.apply(download_with_args)
  File "/home/release/Sage/build/bin/../sage_bootstrap/expand_class.py", line 71, in apply
    function(package_name, *args, **kwds)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 176, in download_with_args
    return self.download(package, allow_upstream=allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 167, in download
    package.tarball.download(allow_upstream=allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/tarball.py", line 177, in download
    raise FileNotMirroredError('tarball does not exist on mirror network')
sage_bootstrap.tarball.FileNotMirroredError: tarball does not exist on mirror network

comment:21 Changed 11 months ago by vbraun

  • Status changed from positive_review to needs_work

comment:22 Changed 11 months ago by gh-kliem

I think this ticket is fine. It fixes

./sage -package download --allow-upstream :all:

as claimed.

./sage -package upload :all:

needs work yet, which is a seperate issue, I would say.

Of course, both things can be taken care of in this ticket. But one might as well open a seperate ticket.

comment:23 Changed 11 months ago by mkoeppe

  • Summary changed from Fix sage -package download --allow-upstream to Fix sage -package download --allow-upstream, add filtering to sage -package upload

comment:24 Changed 11 months ago by mkoeppe

needs_work is just shorthand for "Could you please prepare the mechanism to exclude non-distributable tarballs as you suggested on the ticket description". I'm working on it.

comment:25 Changed 11 months ago by git

  • Commit changed from 34335ca0765c0b68cfc36bec81bc4b626e810fd2 to fefaec192f7af080f0778044867c2ad7943498e3

Branch pushed to git repo; I updated commit sha1. New commits:

a45caedbuild/pkgs/scipoptsuite: Rename tarball to include tag '-do-not-distribute'
1bad6b4sage -package upload: Add more logging
fefaec1sage -package upload: Skip non-distributable tarballs

comment:26 Changed 11 months ago by mkoeppe

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Here's a version with this new feature

comment:27 follow-up: Changed 11 months ago by gh-kliem

There is at least an info section missing here https://doc.sagemath.org/html/en/developer/packaging.html, so that people are warned when adding new packages.

However, I would prefer it the other way around, even though it is annoying: Have a small sentence in SPKG.rst that we may redistribute. This would be more fool-proof.

comment:28 in reply to: ↑ 27 Changed 11 months ago by mkoeppe

Replying to gh-kliem:

There is at least an info section missing here https://doc.sagemath.org/html/en/developer/packaging.html, so that people are warned when adding new packages.

Good idea.

However, I would prefer it the other way around, even though it is annoying: Have a small sentence in SPKG.rst that we may redistribute. This would be more fool-proof.

I disagree. We make free software here; the non-distributable case is an exception for rare cases.

It does not have to be foolproof. We have the review process that so far seems to have successfully stopped people from adding proprietary packages.

If you are interested in adding structured license info to build/pkgs/... see #21571.

comment:29 Changed 11 months ago by gh-kliem

Fine.

comment:30 Changed 11 months ago by git

  • Commit changed from fefaec192f7af080f0778044867c2ad7943498e3 to 6ee9a337e91a782637cac18acb9ad712c7accaac

Branch pushed to git repo; I updated commit sha1. New commits:

6ee9a33src/doc/en/developer/packaging.rst: Document procedure for non-distributable tarballs

comment:31 Changed 11 months ago by gh-kliem

Ok, seems fine.

Have you tested that this actually works and skips scipoptsuite?

comment:32 Changed 11 months ago by gh-kliem

  • Status changed from needs_review to positive_review

Never mind, I tested it with a dummy file and it works fine.

comment:33 Changed 11 months ago by mkoeppe

Thanks!

comment:34 Changed 11 months ago by vbraun

  • Status changed from positive_review to needs_work

Downloading still fails tho:

$ sage -package download --allow-upstream :all:
[...]
http://sagepad.org/spkg/upstream/scipoptsuite/scipoptsuite-5.0.1-do-not-distribute.tgz
[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
ERROR [transfer|run:135]: [Errno 404] Not Found: '//sagepad.org/spkg/upstream/scipoptsuite/scipoptsuite-5.0.1-do-not-distribute.tgz'
Traceback (most recent call last):
  File "/home/release/Sage/build/bin/sage-package", line 42, in <module>
    run()
  File "/home/release/Sage/build/bin/../sage_bootstrap/cmdline.py", line 339, in run
    app.download_cls(args.package_name, args.allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 177, in download_cls
    pc.apply(download_with_args)
  File "/home/release/Sage/build/bin/../sage_bootstrap/expand_class.py", line 71, in apply
    function(package_name, *args, **kwds)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 176, in download_with_args
    return self.download(package, allow_upstream=allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/app.py", line 167, in download
    package.tarball.download(allow_upstream=allow_upstream)
  File "/home/release/Sage/build/bin/../sage_bootstrap/tarball.py", line 180, in download
    raise FileNotMirroredError('tarball does not exist on mirror network')
sage_bootstrap.tarball.FileNotMirroredError: tarball does not exist on mirror network

comment:35 Changed 11 months ago by git

  • Commit changed from 6ee9a337e91a782637cac18acb9ad712c7accaac to 274db614ef204d682e15cebdf76c0df648c7c7e5

Branch pushed to git repo; I updated commit sha1. New commits:

274db61sage --package download: Add option --on-error=warn

comment:36 Changed 11 months ago by mkoeppe

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from Fix sage -package download --allow-upstream, add filtering to sage -package upload to Fix sage -package download --allow-upstream, add option --on-error=warn, add filtering to sage -package upload

comment:37 Changed 11 months ago by mkoeppe

Here you go

comment:38 Changed 11 months ago by vbraun

  • Status changed from needs_review to positive_review

comment:39 Changed 11 months ago by mkoeppe

Thanks.

comment:40 Changed 11 months ago by mkoeppe

  • Reviewers changed from Jonathan Kliem to Jonathan Kliem, Volker Braun

comment:41 Changed 11 months ago by vbraun

  • Branch changed from u/mkoeppe/gh_actions_workflow_to_sync_upstream_packages_to_the_sage_server to 274db614ef204d682e15cebdf76c0df648c7c7e5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.