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

fix make download #29896

Closed
dimpase opened this issue Jun 18, 2020 · 24 comments
Closed

fix make download #29896

dimpase opened this issue Jun 18, 2020 · 24 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jun 18, 2020

This is broken, as reported on
https://groups.google.com/d/msg/sage-devel/KA8tI5GsFX4/1KIV2iuIBwAJ

Depends on #30865

CC: @saliola @mkoeppe @orlitzky @slel @vbraun

Component: distribution

Keywords: download

Branch/Commit: u/mkoeppe/fix_make_download @ 039fed7

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29896

@dimpase dimpase added this to the sage-9.2 milestone Jun 18, 2020
@jhpalmieri
Copy link
Member

comment:2

One option:

diff --git a/Makefile b/Makefile
index 822d1d316f..74863704b1 100644
--- a/Makefile
+++ b/Makefile
@@ -61,9 +61,9 @@ buildbot-python3:
        $(MAKE)
 
 # Preemptively download all standard upstream source tarballs.
-download:
+download: build/make/Makefile
        export SAGE_ROOT=$$(pwd) && \
-       export PATH=$$SAGE_ROOT/src/bin:$$PATH && \
+       export PATH=$$SAGE_ROOT/build/bin:$$SAGE_ROOT/src/bin:$$PATH && \
        ./src/bin/sage-download-upstream
 
 dist: build/make/Makefile

Then (after #29316) users will be warned that they have to run ./configure first. For me, if I've run ./configure, this modified version of make download works.

@jhpalmieri
Copy link
Member

comment:3

Although there is another issue: make download says that it downloads the standard upstream source tarballs, but it actually downloads tarballs for standard, optional, and experimental (after querying) packages. At least one of the experimental database packages is pretty big, so maybe we don't want to do this. With #20104, we could do this:

diff --git a/src/bin/sage-download-upstream b/src/bin/sage-download-upstream
index c5d9afe31e..226806ab7d 100755
--- a/src/bin/sage-download-upstream
+++ b/src/bin/sage-download-upstream
@@ -1,8 +1,8 @@
 #!/usr/bin/env bash
 
-for pkg in $SAGE_ROOT/build/pkgs/*
+for pkg in `sage --package list :standard:`
 do
-    if [ -d $pkg ]; then
+    if [ -d $SAGE_ROOT/build/pkgs/$pkg ]; then
         sage-spkg -d `basename $pkg`
     fi
 done

@jhpalmieri
Copy link
Member

comment:5

I could also imagine trying to use the results from ./configure to decide which packages to download, which to skip because they are already available from the system. But if someone wants to download once and then distribute to colleagues on a thumb drive, it's a good idea to have all of the standard packages.

@jhpalmieri
Copy link
Member

comment:7

Should this be using the same logic as make dist to determine what to download?

@orlitzky
Copy link
Contributor

comment:8

Replying to @jhpalmieri:

Should this be using the same logic as make dist to determine what to download?

That's my instinct. We probably don't need both make download-for-sdist and make download in that case.

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2020

Dependencies: #30846

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2020

Branch: u/mkoeppe/fix_make_download

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2020

comment:12

The branch "fixes" make download (and also cleans up src/bin/ a bit more, on top of #30846 and similar earlier tickets).

I don't use this myself, so I don't know what is the best way to proceed regarding "standard" vs. "all" vs. "configured" packages. Perhaps they should be 3 different make targets.


New commits:

044fcc0Move sage-list-packages from src/bin (sagelib) to build/bin (sage_bootstrap)
896544fMove sage-download-upstream from src/bin (sagelib) to build/bin (sage_bootstrap)

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2020

Commit: 896544f

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2020

Author: John Palmieri, Matthias Koeppe

@slel
Copy link
Member

slel commented Nov 18, 2020

comment:13

In case that is relevant to the present ticket: our release manager
says: "If there were a command-line switch to download all standard

  • optional + experimental packages then I could test that..." (i.e.
    test they all download properly, to avoid missing upstream tarballs
    on our download mirrors after package upgrade tickets get merged).

@slel
Copy link
Member

slel commented Nov 18, 2020

Changed keywords from none to download

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 18, 2020

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

e09d7daMove sage-download-upstream from src/bin (sagelib) to build/bin (sage_bootstrap)
ca87916Makefile: Update documentation of 'make download'
039fed7src/doc/en/installation/source.rst: Remove outdated documentation on old-style packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 18, 2020

Changed commit from 896544f to 039fed7

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2020

comment:17

No longer depends on #30846

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2020

Changed dependencies from #30846 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

comment:18

(pushed to wrong ticket)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

Changed commit from 039fed7 to e263510

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

Changed commit from e263510 to 039fed7

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 19, 2020

comment:20

Actually best to go through #30865, sage -package download :all:

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 19, 2020

Changed author from John Palmieri, Matthias Koeppe to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 19, 2020

Dependencies: #30865

@mkoeppe mkoeppe removed this from the sage-9.3 milestone Nov 19, 2020
@dimpase
Copy link
Member Author

dimpase commented Nov 21, 2020

Reviewer: Dima Pasechnik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants