#30795 closed defect (duplicate)

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

Reported by: dimpase Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build: configure Keywords:
Cc: vbraun, mjo, mkoeppe Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: f612b22 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

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

Change History (20)

comment:1 Changed 21 months ago by dimpase

  • Branch set to u/dimpase/build/morecleanbootstrap
  • Cc vbraun mjo mkoeppe added
  • Commit set to f976c52c57a722ed07feeafa5bc82a5064b2b362
  • Status changed from new to needs_review

comment:2 Changed 21 months ago by mkoeppe

  • Priority changed from critical to blocker

This does not seem to be the right branch

comment:3 Changed 21 months ago by git

  • Commit changed from f976c52c57a722ed07feeafa5bc82a5064b2b362 to f612b221e0180d8cdea595dbee54a11a39c041b8

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

f612b22add the removal of sage-env-config

comment:4 Changed 21 months ago by dimpase

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

comment:5 Changed 21 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:6 Changed 21 months ago by vbraun

  • Priority changed from blocker to major

Better development experience can't be a blocker

comment:7 Changed 21 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:8 Changed 20 months ago by mkoeppe

  • Priority changed from major to critical

comment:9 Changed 20 months ago by vbraun

  • Branch changed from u/dimpase/build/morecleanbootstrap to f612b221e0180d8cdea595dbee54a11a39c041b8
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 follow-ups: Changed 20 months ago by vbraun

  • Commit f612b221e0180d8cdea595dbee54a11a39c041b8 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

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.

comment:11 in reply to: ↑ 10 Changed 20 months ago by dimpase

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!

comment:12 in reply to: ↑ 10 Changed 20 months ago by dimpase

  • Status changed from new to needs_review

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.

comment:13 Changed 20 months ago by vbraun

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

comment:14 Changed 20 months ago by mkoeppe

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.

comment:15 Changed 20 months ago by mkoeppe

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.

comment:16 Changed 20 months ago by mkoeppe

  • Priority changed from critical to major

comment:17 Changed 18 months ago by dimpase

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

comment:18 Changed 18 months ago by mkoeppe

  • Authors Dima Pasechnik deleted
  • Milestone changed from sage-9.3 to sage-duplicate/invalid/wontfix
  • Reviewers Matthias Koeppe deleted

I think it can be closed.

comment:19 Changed 18 months ago by dimpase

  • Status changed from needs_review to positive_review

comment:20 Changed 13 months ago by chapoton

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.