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

bootstrap-clean must remove src/bin/sage-env-config #30795

Closed
dimpase opened this issue Oct 19, 2020 · 20 comments
Closed

bootstrap-clean must remove src/bin/sage-env-config #30795

dimpase opened this issue Oct 19, 2020 · 20 comments

Comments

@dimpase
Copy link
Member

dimpase commented Oct 19, 2020

Otherwise on partially configured trees this might be a problem.
Here is a reproduction of a real bug I hit with a student today

dima@hilbert /mnt/opt/Sage/sage-clang $ echo Foo > src/bin/sage-env-config
dima@hilbert /mnt/opt/Sage/sage-clang $ make bootstrap-clean 
...
dima@hilbert /mnt/opt/Sage/sage-clang $ ./bootstrap 
rm -rf config configure build/make/Makefile-auto.in
rm -f src/doc/en/installation/*.txt
rm -rf src/doc/en/reference/spkg/*.rst
rm -f src/doc/en/reference/repl/*.txt
src/bin/sage-env-config: line 1: Foo: command not found
must source sage-env-config before sage-env
src/doc/bootstrap:56: installing src/doc/en/installation/arch.txt and src/doc/en/installation/arch-optional.txt
src/doc/bootstrap:56: installing src/doc/en/installation/debian.txt and src/doc/en/installation/debian-optional.txt
src/doc/bootstrap:56: installing src/doc/en/installation/fedora.txt and src/doc/en/installation/fedora-optional.txt
src/doc/bootstrap:56: installing src/doc/en/installation/cygwin.txt and src/doc/en/installation/cygwin-optional.txt
src/doc/bootstrap:56: installing src/doc/en/installation/homebrew.txt and src/doc/en/installation/homebrew-optional.txt
src/doc/bootstrap:65: installing src/doc/en/reference/spkg/*.rst
src/doc/bootstrap:97: installing src/doc/en/reference/repl/options.txt
/mnt/opt/Sage/sage-clang/src/bin/sage-env-config: line 1: Foo: command not found
must source sage-env-config before sage-env
Error setting environment variables by sourcing '/mnt/opt/Sage/sage-clang/src/bin/sage-env';
possibly contact sage-devel (see http://groups.google.com/group/sage-devel).

CC: @vbraun @orlitzky @mkoeppe

Component: build: configure

Branch: f612b22

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

@dimpase dimpase added this to the sage-9.2 milestone Oct 19, 2020
@dimpase
Copy link
Member Author

dimpase commented Oct 19, 2020

Branch: u/dimpase/build/morecleanbootstrap

@dimpase
Copy link
Member Author

dimpase commented Oct 19, 2020

Commit: f976c52

@mkoeppe
Copy link
Member

mkoeppe commented Oct 19, 2020

comment:2

This does not seem to be the right branch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2020

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

f612b22add the removal of sage-env-config

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 19, 2020

Changed commit from f976c52 to f612b22

@dimpase
Copy link
Member Author

dimpase commented Oct 19, 2020

comment:4

should be ok now,
sorry for pushing this branch from a wrong window.

@mkoeppe
Copy link
Member

mkoeppe commented Oct 19, 2020

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Oct 24, 2020

comment:6

Better development experience can't be a blocker

@vbraun
Copy link
Member

vbraun commented Nov 1, 2020

Changed branch from u/dimpase/build/morecleanbootstrap to f612b22

@vbraun
Copy link
Member

vbraun commented Nov 1, 2020

Changed commit from f612b22 to none

@vbraun
Copy link
Member

vbraun commented Nov 1, 2020

comment:10

This breaks make clean && ./sage -sdist, which is what I'm using to create the source tarballs. One could require to first build Sage before making source tarballs, but I think thats a) complicated and b) error-prone as build artifacts going to sneak into source tarballs.

@vbraun vbraun reopened this Nov 1, 2020
@dimpase
Copy link
Member Author

dimpase commented Nov 1, 2020

comment:11

Replying to @vbraun:

This breaks make clean && ./sage -sdist, which is what I'm using to create the source tarballs. One could require to first build Sage before making source tarballs, but I think thats a) complicated and b) error-prone as build artifacts going to sneak into source tarballs.

but sage-env-config is a build artefact!

@dimpase
Copy link
Member Author

dimpase commented Nov 1, 2020

comment:12

Replying to @vbraun:

This breaks make clean && ./sage -sdist, which is what I'm using to create the source tarballs. One could require to first build Sage before making source tarballs,

no, why? All sdist needs is to know SAGE_ROOT and SAGE_SRC - and this info definitely does not need Sage to be built.
I.e.

make clean && SAGE_ROOT=`pwd` SAGE_SRC=`pwd`/src ./sage -sdist

should work for you. If we really must, we can provide such defaults in build/bin/sage-sdist.

Please tell us how we should proceed here.

@vbraun
Copy link
Member

vbraun commented Nov 1, 2020

comment:13

sage -sdist should work without setting environment variables. The path could also be hardcoded in the sage launcher script.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2020

comment:14

Volker, is having SAGE_SRC configurable by environment setting needed for your release management scripts? Otherwise I would suggest that we get rid of this configuration option.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2020

comment:15

The problem reported on the original ticket description is fixed in #30841.

But I think it's time to clean up the -clean targets - see #21566, #21775.

@dimpase
Copy link
Member Author

dimpase commented Jan 18, 2021

comment:17

is this already merged? Can it be closed, or?

@mkoeppe
Copy link
Member

mkoeppe commented Jan 18, 2021

Changed reviewer from Matthias Koeppe to none

@mkoeppe
Copy link
Member

mkoeppe commented Jan 18, 2021

comment:18

I think it can be closed.

@mkoeppe
Copy link
Member

mkoeppe commented Jan 18, 2021

Changed author from Dima Pasechnik to none

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

4 participants