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

Run sage-upgrade *outside* of the Sage shell #15517

Closed
jdemeyer opened this issue Dec 13, 2013 · 26 comments
Closed

Run sage-upgrade *outside* of the Sage shell #15517

jdemeyer opened this issue Dec 13, 2013 · 26 comments

Comments

@jdemeyer
Copy link

Running make inside a Sage shell doesn't really work well, because sage-env isn't resourced during the build. This breaks upgrading if new packages are installed in the upgrade which require environment variable changes. Most of the old work-arounds in spkg/install removed in #14715 existed precisely for this reason.

Now that nothing can upgrade to Sage 6.0, we should make sure that upgrading from Sage 6.0 doesn't suffer this problem.

Depends on #14715

CC: @vbraun @ohanar

Component: build

Author: Jeroen Demeyer

Branch/Commit: u/vbraun/ticket/15517 @ 1f27435

Reviewer: Volker Braun, R. Andrew Ohana

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

@jdemeyer jdemeyer added this to the sage-6.0 milestone Dec 13, 2013
@jdemeyer
Copy link
Author

Dependencies: #14715

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Branch: u/jdemeyer/ticket/15517

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

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

[bcd9b51](https://github.com/sagemath/sagetrac-mirror/commit/bcd9b51)Create sage-update script, split off from sage-upgrade

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

Commit: bcd9b51

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

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

[6298111](https://github.com/sagemath/sagetrac-mirror/commit/6298111)Source sage-env before "set -e"

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

Changed commit from bcd9b51 to 6298111

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

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

[3709390](https://github.com/sagemath/sagetrac-mirror/commit/3709390)Pass arguments to sage-update

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2013

Changed commit from 6298111 to 3709390

@jdemeyer
Copy link
Author

comment:7

In theory, this needs_review, but I don't manage to test upgrading because I don't know what command line to give to sage -upgrade.

@vbraun
Copy link
Member

vbraun commented Dec 13, 2013

comment:8

There is no need to name a temporary branch, you can just work with commits:

git fetch trac master
git merge --ff-only FETCH_HEAD

@jdemeyer
Copy link
Author

comment:9

The point is that I want to test the actual sage -upgrade command, not emulate it with git commands.

@vbraun
Copy link
Member

vbraun commented Dec 13, 2013

comment:10

No I get that, but I'm saying that the contraption in the sage-update script to generate a random string (as branch name) is entirely unnecessary.

@jdemeyer
Copy link
Author

comment:11

Replying to @vbraun:

No I get that, but I'm saying that the contraption in the sage-update script to generate a random string (as branch name) is entirely unnecessary.

Perhaps, but I guess Andrew wrote that, feel free to simplify it.

@jdemeyer
Copy link
Author

Author: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Dec 15, 2013

Changed branch from u/jdemeyer/ticket/15517 to u/vbraun/ticket/15517

@vbraun
Copy link
Member

vbraun commented Dec 15, 2013

Changed commit from 3709390 to 1f27435

@vbraun
Copy link
Member

vbraun commented Dec 15, 2013

comment:14

I've simplified the script and added the --ff-only.


New commits:

[1f27435](https://github.com/sagemath/sagetrac-mirror/commit/1f27435)Simplify the update script, error out if not fast-forward

@vbraun
Copy link
Member

vbraun commented Dec 15, 2013

Reviewer: Volker Braun

@jdemeyer
Copy link
Author

comment:15

Andrew, can you review this please?

@ohanar
Copy link
Member

ohanar commented Dec 15, 2013

comment:16

Why are you always upgrading to the current development version, unless otherwise specified? IMO this is a user feature, developers will just be using git directly, so I would argue that it should update to the latest released version by default (which is more or less what the old one was doing -- it just checked if you were on either a released tag, or the release branch).

@vbraun
Copy link
Member

vbraun commented Dec 15, 2013

comment:17

There is no release branch yet. Has there been any decision on using (master, release) or (beta, master) or some variation of it?

In any case, all we have to make sure NOW is that we can update 6.0 to a future version that fixes the update script...

@ohanar
Copy link
Member

ohanar commented Dec 15, 2013

comment:18

Replying to @vbraun:

There is no release branch yet. Has there been any decision on using (master, release) or (beta, master) or some variation of it?

What I was imagining (although you are welcome to do whatever you wish) was that there would be a beta branch (or whatever you would want to call it) -- which would be our release candidate (i.e. only bug fix type things would get merged in here, and when the release manager is happy, it would become the next release). Secondly, when I initially wrote this (like 9 months ago), I was imagining only one release branch, however, I could easily imagine more in the future (e.g. 6.0.x, 6.1.x, etc.).

In any case, all we have to make sure NOW is that we can update 6.0 to a future version that fixes the update script...

Sure. One other thing that I just noticed before I can hand off a positive review, is using origin as the remove server name. Why not just specify the actual repository, since we have no idea where origin would point to (if anywhere)? (i.e. $SAGE_REPO_ANONYMOUS)

@vbraun
Copy link
Member

vbraun commented Dec 16, 2013

comment:19

I wanted to use master as the release branch since we've been publicizing it for a while now. Checking out master should give you something that has a high probability of working ;-) Beta versions will go into develop. That also matches the git flow thing so far. There can be more short-lived branches but there is probably no need to publish those.

About origin, I thought about it. But if origin does not point at our repository then you must have changed it yourself after checking out Sage. The update script shouldn't try to outsmart the user.

@vbraun vbraun closed this as completed in 9531df5 Dec 16, 2013
@vbraun vbraun reopened this Dec 16, 2013
@vbraun
Copy link
Member

vbraun commented Dec 16, 2013

comment:22

Sorry wrong button... needs review

@ohanar
Copy link
Member

ohanar commented Dec 16, 2013

Changed reviewer from Volker Braun to Volker Braun, R. Andrew Ohana

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

3 participants