Opened 6 years ago

Closed 6 years ago

#15580 closed defect (fixed)

Integrate prereq in the new build system

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-6.1
Component: distribution Keywords:
Cc: ohanar, vbraun Merged in:
Authors: R. Andrew Ohana, Jeroen Demeyer Reviewers: R. Andrew Ohana, Volker Braun
Report Upstream: N/A Work issues:
Branch: u/jdemeyer/ticket/15580 (Commits) Commit: 2e27abb8a34a779d5c7cf755450bc273abea443f
Dependencies: #15596 Stopgaps:

Description (last modified by jdemeyer)

After downloading Sage 6.0 either from git or from the .tar.gz sdist tarball, the prereq tarball prereq-1.2.tar.gz is missing. This is a huge problem, because many checks are skipped because of this.

Integrating this was probably overlooked in the sage-git build system. I think the best solution is not have prereq as a tarball at all and simply integrate the contents of the former prereq tarball in the git repo.

Attachments (1)

configure-6.1.beta2.tar.gz (86.3 KB) - added by jdemeyer 6 years ago.

Download all attachments as: .zip

Change History (111)

comment:1 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from prereq tarball is missing to Integrate prereq in the new build system

comment:2 follow-up: Changed 6 years ago by ohanar

Actually prereq-1.2.tar.gz should work if you drop it into the upstream directory, but since it was different, it didn't receive any of the extra treatment the other packages got.

comment:3 in reply to: ↑ 2 Changed 6 years ago by jdemeyer

Replying to ohanar:

Actually prereq-1.2.tar.gz should work if you drop it into the upstream directory

OK, I see that. But it's not distributed by default.

comment:4 Changed 6 years ago by jdemeyer

If somebody can merge the prereq tarball contents in the git repo, I can easily do the rest.

comment:5 Changed 6 years ago by ohanar

  • Branch set to u/ohanar/prereq
  • Commit set to 0881c74dbd67f5cb8ee7733e8f4a9627f823a8f9

I've exported prereq-1.2's history to the root directory with no cleanup (e.g. there is still the hgignore).


New commits:

0881c74Merge in prereq-1.2
cb5e072Trac #14406: remove sqrtl() and SAGE_FORTRAN/SAGE_FORTRAN_LIB checks
e98db82trac 13385: Remove check for openssl.
e63837bTrac #13329: Add -lcrypto and -ldl (needed for -lssl), support SAGE_LOCAL
48b54aaTrac #13329: small reviewer fixes
ac675c6make dpgk-architecture check error out, its at least in Ubuntu 8.04
73f867dadded openssl and dpkg-architecture check
6b9c3e9Trac #12112: Support --disable-compiler-checks option
ea08e9bInitialization of repository

comment:6 Changed 6 years ago by jdemeyer

  • Authors set to R. Andrew Ohana, Jeroen Demeyer

I have a patch ready, but I need to rebuild Sage before it can be uploaded...

comment:7 Changed 6 years ago by jdemeyer

  • Branch changed from u/ohanar/prereq to u/jdemeyer/ticket/15580
  • Created changed from 12/24/13 09:40:37 to 12/24/13 09:40:37
  • Modified changed from 12/28/13 17:39:43 to 12/28/13 17:39:43

comment:8 Changed 6 years ago by jdemeyer

  • Branch changed from u/jdemeyer/ticket/15580 to u/ohanar/prereq

Not yet ready for review, requires more work.

comment:9 Changed 6 years ago by jdemeyer

  • Branch changed from u/ohanar/prereq to u/jdemeyer/ticket/15580
  • Modified changed from 12/28/13 21:49:58 to 12/28/13 21:49:58

comment:10 Changed 6 years ago by jdemeyer

  • Status changed from new to needs_review

comment:11 Changed 6 years ago by git

  • Commit changed from 0881c74dbd67f5cb8ee7733e8f4a9627f823a8f9 to 143e5e4eacab65525b99f71650cac8d4d893d0c3

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

143e5e4Don't use cp -p in sage-clone-source

comment:12 Changed 6 years ago by git

  • Commit changed from 143e5e4eacab65525b99f71650cac8d4d893d0c3 to 68ac77f261a2e36ea899bea3ce76971642c1a0d1

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

68ac77fMerge branch 'u/jdemeyer/ticket/15596' of git://trac.sagemath.org/sage into ticket/15580
c7c0106sage-sdist: copy upstream tarballs using sage-spkg
321a9adMerge remote-tracking branch 'trac/u/ohanar/pillow' into t/15596/sdist_fails_with_capital_p_illow
f005905pillow: depends on setuptools
adc90cePillow -> pillow
8987381remove PIL (it has been replaced by Pillow)
2c055bdPillow: fix patch to work against 2.2.2
6690d97doc: remove now unused environment variables
3778f42Pillow: re-resolve #9864
6df0adbreplace PIL with Pillow

comment:13 Changed 6 years ago by jdemeyer

  • Dependencies set to #15596
  • Modified changed from 12/29/13 10:37:59 to 12/29/13 10:37:59

comment:14 follow-up: Changed 6 years ago by ohanar

  • Branch changed from u/jdemeyer/ticket/15580 to u/ohanar/prereq
  • Commit changed from 68ac77f261a2e36ea899bea3ce76971642c1a0d1 to 43b696f7f53f21e9a08f8a01a3fe0480e1c3a448
  • Reviewers set to R. Andrew Ohana

Looks fine to me, so long as someone can review my reviewer commit. (It is just restoring a small regression of sage-sdist.)


New commits:

43b696fsage-sdist: don't require sage to be built

comment:15 follow-up: Changed 6 years ago by ohanar

One comment I have is that having a toplevel configure script might confuse those not familiar with sage's odd build system. Maybe we should move it to be under the build directory?

comment:16 in reply to: ↑ 15 ; follow-up: Changed 6 years ago by jdemeyer

Replying to ohanar:

One comment I have is that having a toplevel configure script might confuse those not familiar with sage's odd build system. Maybe we should move it to be under the build directory?

My hope is to converge to a more standard build system, so having ./configure at the top-level makes the most sense. And even if it confuses people, doing ./configure && make like the usual packages still works.

comment:17 in reply to: ↑ 14 ; follow-up: Changed 6 years ago by jdemeyer

Replying to ohanar:

Looks fine to me, so long as someone can review my reviewer commit. (It is just restoring a small regression of sage-sdist.)

I don't think there is a regression to be fixed there. It's true that python setup.py is run indeed because of my patch, but why is that a problem? I never liked the [ -z "$$SAGE_INSTALL_FETCH_ONLY" ] in build/deps and I want to remove that. The only reason I didn't do that on this ticket is to avoid potential merge conflicts. And reusing the $(INST)/sage build rule to copy the sources makes both build/deps and sage-sdist harder to understand for no good reason.

So my proposal is still to merge my patch as-is, but of course I'm listening to suggestions...

comment:18 in reply to: ↑ 17 ; follow-ups: Changed 6 years ago by ohanar

Replying to jdemeyer:

I don't think there is a regression to be fixed there. It's true that python setup.py is run indeed because of my patch, but why is that a problem?

It requires that all of the sage libraries dependencies to already be built (otherwise it will bail). Before this branch, you could clone a sage repository, and without running make, you could run (without error) sage -sdist.

This was not true back in mercurial land, but then again, sage-sdist had to be a lot more complicated since there were a number of repositories to manage.

I think that the best solution to these issues are to include the package type (and dependencies) in the metadata of each individual package. Right now you have to do this psuedo-build to pull just the standard packages (since the only thing that records them as standard is that they are a dependency of sage in build/deps).

I would propose my additional commit for now (or some alternative to restore sage -sdist for freshly cloned repos), and open a new ticket to introduce the extra metadata.

comment:19 in reply to: ↑ 16 Changed 6 years ago by ohanar

Replying to jdemeyer:

My hope is to converge to a more standard build system, so having ./configure at the top-level makes the most sense. And even if it confuses people, doing ./configure && make like the usual packages still works.

Ok, sounds good.

comment:20 in reply to: ↑ 18 Changed 6 years ago by jdemeyer

Replying to ohanar:

and open a new ticket to introduce the extra metadata.

On the other hand, I don't like the idea of storing the same information (which packages are "standard") twice.

comment:21 in reply to: ↑ 18 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Replying to ohanar:

Replying to jdemeyer:

I don't think there is a regression to be fixed there. It's true that python setup.py is run indeed because of my patch, but why is that a problem?

It requires that all of the sage libraries dependencies to already be built (otherwise it will bail). Before this branch, you could clone a sage repository, and without running make, you could run (without error) sage -sdist.

OK, I get it. I'm not convinced that the extra complication is justified for this use-case, but I'll fix it (but hopefully in a simpler way than your patch).

comment:22 Changed 6 years ago by jdemeyer

I think this is sufficient

  • src/bin/sage-clone-source

    commit 0255a2a53e9aebaf54c0fab335ef0202a6b77ae8
    Author: Jeroen Demeyer <jdemeyer@cage.ugent.be>
    Date:   Mon Dec 30 12:47:06 2013 +0100
    
        Allow sage --sdist without building Sage
    
    diff --git a/src/bin/sage-clone-source b/src/bin/sage-clone-source
    index aabc452..090466b 100755
    a b SRC="$1" 
    2020DST="$2"
    2121
    2222rm -rf "$DST"
    23 mkdir "$DST"
     23mkdir -p "$DST"
    2424
    2525git clone "$SRC" "$DST"
    2626
  • src/bin/sage-sdist

    diff --git a/src/bin/sage-sdist b/src/bin/sage-sdist
    index 3a554b6..1aee0cc 100755
    a b sage-clone-source "$SAGE_ROOT" "$TMP_DIR/$TARGET" 
    4646# Download and copy all upstream tarballs (the -B option to make forces
    4747# all targets to be built unconditionally)
    4848cd "$SAGE_ROOT/build"
    49 SAGE_INSTALL_GCC=yes SAGE_SPKG_OPTS="-f -d" SAGE_SPKG_COPY_UPSTREAM="$TMP_DIR/$TARGET/upstream" ./install -B all
     49SAGE_INSTALL_GCC=yes SAGE_SPKG_COPY_UPSTREAM="$TMP_DIR/$TARGET/upstream" \
     50SAGE_INSTALL_FETCH_ONLY=yes SAGE_SPKG_OPTS="-f -d" \
     51./install -B all
    5052
    5153# Create source .tar.gz
    5254cd "$TMP_DIR"

comment:23 Changed 6 years ago by jdemeyer

  • Branch changed from u/ohanar/prereq to u/jdemeyer/ticket/15580
  • Commit changed from 43b696f7f53f21e9a08f8a01a3fe0480e1c3a448 to 0255a2a53e9aebaf54c0fab335ef0202a6b77ae8
  • Status changed from needs_work to needs_review

New commits:

0255a2aAllow sage --sdist without building Sage

comment:24 Changed 6 years ago by ohanar

  • Status changed from needs_review to positive_review

Sure, this is fine.

comment:25 Changed 6 years ago by jdemeyer

Does this imply also a positive review to #15596 by transitivity?

comment:26 follow-up: Changed 6 years ago by vbraun

Fails on mod:

configure.ac:21: error: Autoconf version 2.63 or higher is required
configure.ac:21: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
aclocal: autom4te failed with exit status: 63

comment:27 in reply to: ↑ 26 Changed 6 years ago by jdemeyer

Replying to vbraun:

Fails on mod:

configure.ac:21: error: Autoconf version 2.63 or higher is required
configure.ac:21: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
aclocal: autom4te failed with exit status: 63

Of course it fails, installing from git now requires a recent enough version of autotools (this is true for many projects, it's not the fault of Sage).

If you're bothered by this, you could drop the git builder on mod and only build from the source tarball (which doesn't require autotools).

comment:28 follow-up: Changed 6 years ago by jdemeyer

Why does it say "does not merge cleanly"? It merges cleanly for me...

comment:29 follow-up: Changed 6 years ago by vbraun

Andrew is working on the git plugin this week, so its possibly not accurate at the moment.

Do we really need ac 2.63, which is only about 5 years old? 2.61 doesn't work? Also I don't see the new dependency being documented.

In any case, I think OSX users will be in a world of hurt if we merge this. I take it you are volunteering to walk them through autoconf installation? ;-)

comment:30 in reply to: ↑ 28 ; follow-up: Changed 6 years ago by ohanar

Replying to jdemeyer:

Why does it say "does not merge cleanly"? It merges cleanly for me...

This particular merge confuses a lot of git tools since there is a new root in your branch. I should be rolling out the new plugin later today, but it won't fix this ticket (or this sort of issue). In the long run, these sorts of merge issues should disappear as all the old repositories get merged.

Last edited 6 years ago by ohanar (previous) (diff)

comment:31 follow-up: Changed 6 years ago by vbraun

I discussed this with Andrew and we agreed to just package the autogenerated files into a tarball, and make it so that the "make configure" target automatically updates the tarball / checksum.ini. Objections?

comment:32 in reply to: ↑ 30 Changed 6 years ago by jdemeyer

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:33 in reply to: ↑ 29 Changed 6 years ago by jdemeyer

Replying to vbraun:

Andrew is working on the git plugin this week, so its possibly not accurate at the moment.

Do we really need ac 2.63, which is only about 5 years old? 2.61 doesn't work?

Probably, but I can easily check to be sure.

Also I don't see the new dependency being documented.

Well, building directly from the git repo isn't documented anywhere AFAIK, that would be the natural place to document the extra dependency.

comment:34 in reply to: ↑ 31 ; follow-up: Changed 6 years ago by jdemeyer

Replying to vbraun:

I discussed this with Andrew and we agreed to just package the autogenerated files into a tarball. Objections?

No objections in principle. But this tarball should not be treated like other "upstream" tarballs. I prefer a direct call to sage-download-file, without going through sage-spkg and checksum checking.

I think creating this tarball should be left to sage --sdist, I don't see the point of recreating it every time you run autoconf.

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:35 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

I am working on this.

comment:36 Changed 6 years ago by jdemeyer

You're right, autoconf version 2.60 does work (2.59 does not).

comment:37 in reply to: ↑ 34 ; follow-up: Changed 6 years ago by vbraun

Replying to jdemeyer:

I think creating this tarball should be left to sage --sdist, I don't see the point of recreating it every time you run autoconf.

If we want to treat that tarball like all other tarballs then we need to commit its checksum *before* running sage -sdist. Also, for testing it would be nice to run the new tarball through the buildbot before releasing it. Of course that can be worked around by running sage -sdist twice but then thats hardly ideal.

And while one could treat it completely different from the other tarballs, I don't see why we have to give up checksumming here.

Last edited 6 years ago by vbraun (previous) (diff)

comment:38 in reply to: ↑ 37 Changed 6 years ago by jdemeyer

Replying to vbraun:

I don't see why we have to give up checksumming here.

Because checksumming is done is sage-spkg and this tarball wouldn't sage-spkg. And I would very much be against using sage-spkg for this, since it would be the upside-down world. Normally, configure is run as very first thing in a build and it might for example be used to configure sage-spkg. This currently isn't the case, but we should plan for it.

comment:39 follow-up: Changed 6 years ago by vbraun

Either way you need a script somewhere that a) does not depend on configure output and b) downloads a tarball and run the spkg-install script. If we want to auto-generate the equivalent of a sage-spkg script then we can just give it a different name, no?

comment:40 in reply to: ↑ 39 Changed 6 years ago by jdemeyer

Replying to vbraun:

Either way you need a script somewhere that a) does not depend on configure output and b) downloads a tarball and run the spkg-install script.

I don't understand what you're saying here. We need a script that

  1. does not depend on configure output
  2. downloads a tarball

but that script does not need to

  1. check checksums
  2. run spkg-install

I also really don't want to come up with an overkill solution for this. Personally, I'd be happy to simply say "if you want to build from git, make sure you have autotools installed". There's always the source tarball which doesn't require autotools and once you've built Sage, you could install the autotools package, or simply hope that an outdated configure script is still fine.

comment:41 Changed 6 years ago by vbraun

I also want to make it as easy as possible, this is why I'm suggesting to just treat it as every other tarball instead of a custom script just for that one.

Changed 6 years ago by jdemeyer

comment:42 Changed 6 years ago by jdemeyer

Volker, are you able to put files in the upstream directory on the server? If so, please do that for configure-6.1.beta2.tar.gz

comment:43 Changed 6 years ago by git

  • Commit changed from 0255a2a53e9aebaf54c0fab335ef0202a6b77ae8 to 19d5a15ba83c7861237be766420541bb783ec34f

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

19d5a15Create and use "configure" tarball for auto-generated files

comment:44 Changed 6 years ago by jdemeyer

This is the simplest solution I could come up with which should work...


New commits:

19d5a15Create and use "configure" tarball for auto-generated files

New commits:

19d5a15Create and use "configure" tarball for auto-generated files

comment:45 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:46 follow-up: Changed 6 years ago by vbraun

I don't like the fact that I can't test the updated tarball on the buildbot without bumping the version. This slows down the release process as I have to bump version and re-run the buildbot. For a release I just want to bump the version and publish it after the buildbot finished. Any other change can break things, so requres testing, ie. another day twiddling thumbs.

Version 0, edited 6 years ago by vbraun (next)

comment:47 in reply to: ↑ 46 Changed 6 years ago by jdemeyer

Replying to vbraun:

I don't like the fact that I can't test the updated tarball on the buildbot without bumping the version. This slows down the release process as I have to bump version and re-run the buildbot.

I thought you were using a private git repo for the buildbot? In that case, you could easily do the version bump only in your private repo. Alternatively, point $SAGE_UPSTREAM to a place you control.

My philosophy with the buildbot was to always test the exact source tarball that I would release. If you're testing first, then changing the version number, then sdist, something could go wrong in those last two steps and you would never notice.

And yes, there has been at least one bug which dependended on the length of the Sage version number, so it can happen.

comment:48 follow-ups: Changed 6 years ago by vbraun

I'm not testing the source tarball at all, its much faster to do incremental builds for git commits with occasional full rebuilds.

comment:49 in reply to: ↑ 48 Changed 6 years ago by jdemeyer

Replying to vbraun:

its much faster to do incremental builds for git commits with occasional full rebuilds.

Even then, I still don't understand why this patch prevents you from testing properly.

comment:50 follow-up: Changed 6 years ago by vbraun

It doesn't prevent me from testing, but it makes it slower and involves more steps than it should.

comment:51 in reply to: ↑ 50 Changed 6 years ago by jdemeyer

Replying to vbraun:

It doesn't prevent me from testing, but it makes it slower and involves more steps than it should.

Why slower? I don't understand the problem...

comment:52 follow-up: Changed 6 years ago by vbraun

I want to have a workflow where I merge branches, test, and once it works slap on the new version and release. Bumping the version should not require another test run. Bumping the version just to make the new configure-x.y.z tarball and then resetting to before the version bump is also not desirable.

comment:53 in reply to: ↑ 52 Changed 6 years ago by jdemeyer

Replying to vbraun:

I want to have a workflow where I merge branches, test, and once it works slap on the new version and release. Bumping the version should not require another test run. Bumping the version just to make the new configure-x.y.z tarball and then resetting to before the version bump is also not desirable.

Why not the following workflow:

  1. In volker's private repo, bump the version number and merge various patches
  2. Run sage --sdist to generate the configure tarball
  3. Put the configure tarball in some accessible place
  4. Run the buildbots on Volker's private repo
  5. If tests pass, merge Volker's repo with the official repo.

(Ideally, I would argue there should also be a 4b. Run at least one buildbot on the source tarball instead of the git repo)

What alternative would you propose? You need to generate the configure tarball anyway before you can test it, regardless of any version bumps...

comment:54 follow-up: Changed 6 years ago by vbraun

Since we don't put the version into AC_INIT we don't have to bump the tarball at all with each version. Not doing anything is always the easiest and least error-prone.

Just increment configure-n.tar, configure-(n+1).tar, ... each time you have to re-run autoconf (i.e. make configure / autogen.sh).

comment:55 follow-up: Changed 6 years ago by vbraun

Also, I've been bumping the version in the source and tagging the new version at the same commit. IMHO having version and tag match is the easiest to understand for others. That requires me to bump the version at the end of the beta series.

comment:56 in reply to: ↑ 54 ; follow-up: Changed 6 years ago by jdemeyer

Replying to vbraun:

we don't put the version into AC_INIT

That's "fixed" in #15606, but it's not really important anyway.

Not doing anything is always the easiest and least error-prone.

I disagree. Doing the same thing every time (i.e. always changing the version number) is always the easiest and least error-prone. Special cases are often the cause for errors.

Just increment configure-n.tar, configure-(n+1).tar, ... each time you have to re-run autoconf (i.e. make configure / autogen.sh).

And how are you going to implement this auto-incrementing and keeping track of which number to use... this seems far more complicated than my proposal.

comment:57 in reply to: ↑ 55 Changed 6 years ago by jdemeyer

Replying to vbraun:

Also, I've been bumping the version in the source and tagging the new version at the same commit. IMHO having version and tag match is the easiest to understand for others. That requires me to bump the version at the end of the beta series.

One could argue that the new version "starts" right after the old tag. The first commit after sage-6.1.beta4 belongs to sage-6.1.beta5, so I would find it logical that checking out that commit would give the new version number.

comment:58 in reply to: ↑ 56 Changed 6 years ago by vbraun

On can certainly argue for other ways of labeling things. In the end, doing it this way is one step vs. two separate steps, so its easier for me.

Replying to jdemeyer:

And how are you going to implement this auto-incrementing and keeping track of which number to use..

Easy, there will be a build/pkgs/configure/package-version.txt that contains the integer n. Reading it and incrementing it by one is standard bash $[n+1]

comment:59 Changed 6 years ago by git

  • Commit changed from 19d5a15ba83c7861237be766420541bb783ec34f to 5d7729c51042f87a125e76b3e2819e564d2d562f

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

5d7729cUse build/pkgs/configure/package-version.txt for configure version number

comment:60 Changed 6 years ago by jdemeyer

Do you like this better?

comment:61 follow-up: Changed 6 years ago by vbraun

Looks good. Is there an argument against automatically increasing the version number whenever you re-run autoconf? Skipping configure-$version numbers costs us nothing, but forgetting to increase it will potentially cause major problems.

Also, filling out the other files (e.g. checksums.ini) in the build/pkgs/configure files will make it easier to manage the upstream directory on the sagemath web server in a unified manner.

comment:62 in reply to: ↑ 61 Changed 6 years ago by jdemeyer

Replying to vbraun:

Looks good. Is there an argument against automatically increasing the version number whenever you re-run autoconf? Skipping configure-$version numbers costs us nothing, but forgetting to increase it will potentially cause major problems.

Would you agree with increasing the version number automatically in sage-update-version?

comment:63 follow-up: Changed 6 years ago by vbraun

My main question is whether we are going to hardcode the current version into the autogenerated scripts or not (see #15606). If that is the case then we of course should save the new autogenerated stuff in sage-update-version. If not then there would be no need to tie version to the autogenerated scripts, so we should not.

comment:64 in reply to: ↑ 63 Changed 6 years ago by jdemeyer

Replying to vbraun:

My main question is whether we are going to hardcode the current version into the autogenerated scripts or not (see #15606).

I propose to do so indeed.

comment:65 Changed 6 years ago by vbraun

Ok, fine. Thats imho the right thing to do, keep the version in configure.ac and then generate all other files that contain the version form .in files at configure time.

I'd still prefer to not use the Sage version as the configure tarball version since it'll be potentially confusing to test different configure tarballs on the buildbot if it needs to be debugged.

comment:66 Changed 6 years ago by git

  • Commit changed from 5d7729c51042f87a125e76b3e2819e564d2d562f to e13b216c61977b1d9ae262791bb5bdaf392033cd

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

e13b216Create sage-autogen script to generate configure tarball

comment:67 Changed 6 years ago by jdemeyer

OK, new approach. I now have a script src/bin/sage-autogen which takes care of creating the configure tarball and computing the checksum. It can optionally (with the -i option) increase the version number. This script is never run automatically, it must be explicitly called by the release manager. But then the release manager can freely choose when to do this.

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:68 in reply to: ↑ 48 Changed 6 years ago by jdemeyer

Replying to vbraun:

I'm not testing the source tarball at all

I think that's a mistake. If something breaks sage-sdist, you need to know.

comment:69 follow-up: Changed 6 years ago by vbraun

At the end of the day, the git repo is the authorative download. The source tarball is added as a convenience, but I don't think there is a point in worrying too much about it.

comment:70 Changed 6 years ago by vbraun

Gnu recommends ./bootstrap (http://www.gnu.org/software/automake/faq/autotools-faq.html#What-does-_002e_002fbootstrap-or-_002e_002fautogen_002esh-do_003f} instead of autogen.

Also, the new configure tarball needs to be committed together with the version number change (and should eventually become the version number change) to end up with the correct tag.

I'll implement these changes...

comment:71 Changed 6 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580

comment:72 Changed 6 years ago by vbraun

  • Commit changed from e13b216c61977b1d9ae262791bb5bdaf392033cd to 1c57e53f536db2f505b447ee6f617682372550d3

I agree with everything so far, only my final commits need review.


New commits:

900212erename to bootstrap, run during version bump
56adaa6add a SPKG.txt file
1c57e53also commit changes to confball in sage-update-version

comment:73 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work

In SPKG.txt, They are shippd [sic] with is not a sentence.

comment:74 Changed 6 years ago by git

  • Commit changed from 1c57e53f536db2f505b447ee6f617682372550d3 to cdc7c0a332ae31e61c6bf92a6e09e6dd7c4b8e1a

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

5d17bdafix typos in SPKG.txt
cdc7c0amake saving of the confball optional in bootstrap

comment:75 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Volker: I'm fine with your patch, modulo a few details in my reviewer commit.

comment:76 Changed 6 years ago by jdemeyer

  • Branch changed from u/vbraun/ticket/15580 to u/jdemeyer/ticket/15580
  • Modified changed from 01/12/14 19:45:06 to 01/12/14 19:45:06

comment:77 Changed 6 years ago by jdemeyer

  • Commit changed from cdc7c0a332ae31e61c6bf92a6e09e6dd7c4b8e1a to d0427228339f59811fc41d0ecb861d7e5e64a99a

New commits:

d042722bootstrap configure: small fixes

comment:78 in reply to: ↑ 69 Changed 6 years ago by jdemeyer

Replying to vbraun:

At the end of the day, the git repo is the authorative download. The source tarball is added as a convenience

Given that the Sage home page only has links to the source and binary tarballs, I'd say the opposite is true.

but I don't think there is a point in worrying too much about it.

Perhaps not, but this ticket could certainly break sdists. So if you don't want automated testing, you should at least try building from an sdist generated after this ticket.

comment:79 follow-ups: Changed 6 years ago by vbraun

Breaks incremental builds on mod:

configure:2803: $? = 1
configure:2826: checking for C compiler default output file name
configure:2853: gcc   -I/mnt/SSD1/mod_buildslave/sage_git/build/local/include  -L/mnt/SSD1/mod_buildslave/sage_git/build/local/lib conftest.c  >&5
/mnt/SSD1/mod_buildslave/sage_git/build/local/libexec/gcc/x86_64-unknown-linux-gnu/4.7.3/cc1: error while loading shared libraries: libmpc.so.3: cannot open shared object file: No such file or directory
configure:2856: $? = 1
configure:2894: result: 
configure: failed program was:
| /* confdefs.h.  */
| #define PACKAGE_NAME "Sage"
| #define PACKAGE_TARNAME "sage"
| #define PACKAGE_VERSION "x.y.z"
| #define PACKAGE_STRING "Sage x.y.z"
| #define PACKAGE_BUGREPORT "sage-devel@googlegroups.com"
| #define PACKAGE "sage"
| #define VERSION "x.y.z"
| /* end confdefs.h.  */
| 
| int
| main ()
| {
| 
|   ;
|   return 0;
| }
configure:2901: error: C compiler cannot create executables
See `config.log' for more details

Why does it even know about the $SAGE_LOCAL paths? The part should also set up LD_LIBRARY_PATH...

comment:80 follow-up: Changed 6 years ago by vbraun

Btw the "See `config.log' for more details" is kind of impractical. It would be nice if the config.log would be inserted into the prereq.log file, especially if something goes wrong.

comment:81 Changed 6 years ago by vbraun

Full rebuild works on mod, for the record.

comment:82 in reply to: ↑ 79 Changed 6 years ago by jdemeyer

Replying to vbraun:

Why does it even know about the $SAGE_LOCAL paths? The part should also set up LD_LIBRARY_PATH...

Could it be a bug in your build scripts?

In any case, if the full build works, I wouldn't worry too much...

comment:83 in reply to: ↑ 80 Changed 6 years ago by jdemeyer

Replying to vbraun:

Btw the "See `config.log' for more details" is kind of impractical. It would be nice if the config.log would be inserted into the prereq.log file, especially if something goes wrong.

OK, I'll have a look at that in #15606.

comment:84 in reply to: ↑ 79 Changed 6 years ago by jdemeyer

Replying to vbraun:

Why does it even know about the $SAGE_LOCAL paths? The part should also set up LD_LIBRARY_PATH...

It seems you discovered a very old bug. build/install adds SAGE_LOCAL/bin to the PATH, but doesn't change LD_LIBRARY_PATH.

comment:85 Changed 6 years ago by jdemeyer

The reason that the bug hasn't been seen before is that configure was never re-run after upgrading.

comment:86 Changed 6 years ago by git

  • Commit changed from d0427228339f59811fc41d0ecb861d7e5e64a99a to e03f296685f986f89e1e12974f4c5ead9a9ff440

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

e03f296Source sage-env inside configure

comment:87 Changed 6 years ago by vbraun

Mirror directory layout is SAGE_UPSTREAM/configure/configure-n.tar.gz, for the record

comment:88 Changed 6 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580

comment:89 Changed 6 years ago by vbraun

  • Commit changed from e03f296685f986f89e1e12974f4c5ead9a9ff440 to 3e5dfa5f0623f015faf6a81d96dbf3aaaa7f712b

Still fails on mark/skynet:

make[1]: Entering directory `/home/buildbot/build/sage/mark-1/sage_git/build/build'
make[1]: warning: -jN forced in submake: disabling jobserver mode.
make[1]: Warning: File `Makefile' has modification time 6.4e+02 s in the future
make -j2 base
make[2]: Entering directory `/home/buildbot/build/sage/mark-1/sage_git/build/build'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Warning: File `Makefile' has modification time 6.4e+02 s in the future
/home/buildbot/build/sage/mark-1/sage_git/build/build/pipestatus "./prereq.sh 2>&1" "tee -a /home/buildbot/build/sage/mark-1/sage_git/build/logs/pkgs/prereq.log"
Starting prerequisite check.
Machine: SunOS mark 5.10 Generic_127111-01 sun4u sparc SUNW,Sun-Blade-2500

make[3]: Entering directory `/home/buildbot/build/sage/mark-1/sage_git/build'
make[3]: warning: -jN forced in submake: disabling jobserver mode.
make[3]: Warning: File `Makefile' has modification time 5.5e+02 s in the future
test -f config/missing || make -j2 config/missing || \
		bash -c 'source src/bin/sage-env; sage-download-file $SAGE_UPSTREAM/configure/configure-`cat build/pkgs/configure/package-version.txt`.tar.gz | tar zxf -'
config/missing --run aclocal -I m4
config/missing: aclocal: not found
make[3]: *** [configure] Error 1
make[3]: warning:  Clock skew detected.  Your build may be incomplete.
make[3]: Leaving directory `/home/buildbot/build/sage/mark-1/sage_git/build'
make[2]: *** [/home/buildbot/build/sage/mark-1/sage_git/build/local/var/lib/sage/installed/prereq] Error 2
make[2]: Target `base' not remade because of errors.
make[2]: warning:  Clock skew detected.  Your build may be incomplete.
make[2]: Leaving directory `/home/buildbot/build/sage/mark-1/sage_git/build/build'
make[1]: *** [all] Error 2
make[1]: warning:  Clock skew detected.  Your build may be incomplete.
make[1]: Leaving directory `/home/buildbot/build/sage/mark-1/sage_git/build/build'

New commits:

3e5dfa5fix tarball url

comment:90 Changed 6 years ago by jdemeyer

  • Branch changed from u/vbraun/ticket/15580 to u/jdemeyer/ticket/15580
  • Modified changed from 01/14/14 16:55:40 to 01/14/14 16:55:40

comment:91 Changed 6 years ago by jdemeyer

  • Commit changed from 3e5dfa5f0623f015faf6a81d96dbf3aaaa7f712b to e1df3ce1e4a7004cc9ca0aef6008c9edde6c6e54

Not tested, but I guess this should help.


New commits:

e1df3ceRun "missing" using bash

comment:92 follow-up: Changed 6 years ago by vbraun

How is that supposed to change anything? The `aclocal.m4' is not in the confball, so you need aclocal to generate it. The job of the "missing" script is to mangle the timestamps as if aclocal has run, but you still need the file whose timestamp is to be changed.

comment:93 Changed 6 years ago by vbraun

PS: What running with bash does give you is the proper warning before it errors out:

WARNING: 'aclocal' is missing on your system.
         You should only need it if you modified 'acinclude.m4' or
         'configure.ac' or m4 files included by 'configure.ac'.
         The 'aclocal' program is part of the GNU Automake package:
         <http://www.gnu.org/software/automake>
         It also requires GNU Autoconf, GNU m4 and Perl in order to run:
         <http://www.gnu.org/software/autoconf>
         <http://www.gnu.org/software/m4/>
         <http://www.perl.org/>
make[3]: *** [configure] Error 127

comment:94 Changed 6 years ago by vbraun

PPS: any thoughts on enabling AM_MAINTAINER_MODE?

comment:95 Changed 6 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580

comment:96 Changed 6 years ago by vbraun

  • Commit changed from e1df3ce1e4a7004cc9ca0aef6008c9edde6c6e54 to 9c13193d2fedb3d4d20ddb1aacd44d5c3e333ded

I guess disabling maintainer mode is only interesting when you actually run the automake-generated makefile. But we probably should, then. I'll put it in for now.

In any case, we should only run autotools from the bootstrap script. As long as we don't use the automake output there are no timestamp issues.


New commits:

a38f919allow disabling of maintainer mode
9c13193only run autotools from the bootstrap script

comment:97 Changed 6 years ago by jdemeyer

I disagree with ./sage -f configure since I absolutely don't want configure to act like a spkg. I prefer configure to have as few dependencies as possible. I much prefer to not depend on sage-spkg (and preferably also not on build/install nor sage-env, but that's harder for now).

Also, what was wrong with bootstrapping from the top-level Makefile? Doesn't the missing script work properly? I like the fact that configure gets regenerated automatically when I edit configure.ac.

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:98 in reply to: ↑ 92 Changed 6 years ago by jdemeyer

Replying to vbraun:

How is that supposed to change anything? The `aclocal.m4' is not in the confball, so you need aclocal to generate it.

OK, I forgot to add that file, no big deal.

comment:99 Changed 6 years ago by jdemeyer

Hmm, it seems that missing has changed functionality. Now it doesn't really do anything, except for printing a more verbose error message. Old versions of missing did indeed touch the files.

comment:100 Changed 6 years ago by jdemeyer

  • Branch changed from u/vbraun/ticket/15580 to u/jdemeyer/ticket/15580
  • Modified changed from 01/15/14 08:15:47 to 01/15/14 08:15:47

comment:101 Changed 6 years ago by jdemeyer

  • Commit changed from 9c13193d2fedb3d4d20ddb1aacd44d5c3e333ded to 2e27abb8a34a779d5c7cf755450bc273abea443f

Volker, this patch simply stops using the missing script while keeping the rest of the bootstrap process.


New commits:

2e27abbDon't use "missing"

comment:102 follow-ups: Changed 6 years ago by vbraun

bootstrap shouldn't call make to do its business, usually it would run automake to generate the makefile. But we can also disentangle that later if you prefer.

AFAIK we do guarantee that "sage -f" works even before building Sage, so that you can install replacements for certain optional build-time dependencies (e.g. ccache). Not using that mechanism means that "make download" won't work, for example. Not to mention that it duplicates the sagemath upstream url, doesn't verify the checksum, etc. I don't necessarily care so much about using it, just wanted to make sure that you thought about the potential downsides of downloading the confball manually.

comment:103 Changed 6 years ago by vbraun

PS: I agree that missing changed and should be treated as implementation details for the autotools, i.e., do not call directly.

comment:104 in reply to: ↑ 102 ; follow-up: Changed 6 years ago by jdemeyer

Replying to vbraun:

AFAIK we do guarantee that "sage -f" works even before building Sage, so that you can install replacements for certain optional build-time dependencies (e.g. ccache). Not using that mechanism means that "make download" won't work, for example. Not to mention that it duplicates the sagemath upstream url, doesn't verify the checksum, etc. I don't necessarily care so much about using it, just wanted to make sure that you thought about the potential downsides of downloading the confball manually.

I was thinking of moving towards a more typical "configure + make" model. Then ./sage -f should be guaranteed to work only after ./configure but before make. What we actually want for optional packages would be something like

$ ./configure --enable-ccache --enable-build-gcc --enable-mirror=http://example.com/ --optional-packages=database_gap
$ make

comment:105 in reply to: ↑ 102 Changed 6 years ago by jdemeyer

Replying to vbraun:

bootstrap shouldn't call make to do its business, usually it would run automake to generate the makefile. But we can also disentangle that later if you prefer.

Perhaps you are right. But I would indeed prefer to do that later, especially since #15606 changes the bootstrap process also.

comment:106 in reply to: ↑ 104 ; follow-up: Changed 6 years ago by vbraun

Replying to jdemeyer:

$ ./configure --enable-ccache --enable-build-gcc --enable-mirror=http://example.com/ --optional-packages=database_gap

Sounds good to me, but does not have to prevent sage -f from working (with sane defaults) before configure is run. If you want to change the mirror url then you just have to have your own set of autotools.

comment:107 in reply to: ↑ 106 Changed 6 years ago by jdemeyer

Replying to vbraun:

Sounds good to me, but does not have to prevent sage -f from working (with sane defaults) before configure is run.

I'd say: let's see how things evolve. I will not actively prevent sage -f from working.

comment:108 Changed 6 years ago by vbraun

  • Reviewers changed from R. Andrew Ohana to R. Andrew Ohana, Volker Braun
  • Status changed from needs_review to positive_review

fine with me...

comment:109 Changed 6 years ago by vbraun

Another side effect of treating the confball different:

vbraun@boxen:~/Sage/git$ git clone ...
vbraun@boxen:~/Sage/git$ ./sage -sdist
Sage version 6.1.beta5, release date 2014-01-15
Error: configure tarball '/home/vbraun/Sage/git/upstream/configure-3.tar.gz' does not exist.

comment:110 Changed 6 years ago by vbraun

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