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 )
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)
Change History (111)
comment:1 Changed 6 years ago by
- Description modified (diff)
- Summary changed from prereq tarball is missing to Integrate prereq in the new build system
comment:2 follow-up: ↓ 3 Changed 6 years ago by
comment:3 in reply to: ↑ 2 Changed 6 years ago by
Replying to ohanar:
Actually
prereq-1.2.tar.gz
should work if you drop it into theupstream
directory
OK, I see that. But it's not distributed by default.
comment:4 Changed 6 years ago by
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
- 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:
0881c74 | Merge in prereq-1.2
|
cb5e072 | Trac #14406: remove sqrtl() and SAGE_FORTRAN/SAGE_FORTRAN_LIB checks
|
e98db82 | trac 13385: Remove check for openssl.
|
e63837b | Trac #13329: Add -lcrypto and -ldl (needed for -lssl), support SAGE_LOCAL
|
48b54aa | Trac #13329: small reviewer fixes
|
ac675c6 | make dpgk-architecture check error out, its at least in Ubuntu 8.04
|
73f867d | added openssl and dpkg-architecture check
|
6b9c3e9 | Trac #12112: Support --disable-compiler-checks option
|
ea08e9b | Initialization of repository
|
comment:6 Changed 6 years ago by
I have a patch ready, but I need to rebuild Sage before it can be uploaded...
comment:7 Changed 6 years ago by
- 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
- 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
- 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
- Status changed from new to needs_review
comment:11 Changed 6 years ago by
- Commit changed from 0881c74dbd67f5cb8ee7733e8f4a9627f823a8f9 to 143e5e4eacab65525b99f71650cac8d4d893d0c3
Branch pushed to git repo; I updated commit sha1. New commits:
143e5e4 | Don't use cp -p in sage-clone-source
|
comment:12 Changed 6 years ago by
- Commit changed from 143e5e4eacab65525b99f71650cac8d4d893d0c3 to 68ac77f261a2e36ea899bea3ce76971642c1a0d1
Branch pushed to git repo; I updated commit sha1. New commits:
68ac77f | Merge branch 'u/jdemeyer/ticket/15596' of git://trac.sagemath.org/sage into ticket/15580
|
c7c0106 | sage-sdist: copy upstream tarballs using sage-spkg
|
321a9ad | Merge remote-tracking branch 'trac/u/ohanar/pillow' into t/15596/sdist_fails_with_capital_p_illow
|
f005905 | pillow: depends on setuptools
|
adc90ce | Pillow -> pillow
|
8987381 | remove PIL (it has been replaced by Pillow)
|
2c055bd | Pillow: fix patch to work against 2.2.2
|
6690d97 | doc: remove now unused environment variables
|
3778f42 | Pillow: re-resolve #9864
|
6df0adb | replace PIL with Pillow
|
comment:13 Changed 6 years ago by
- Dependencies set to #15596
- Modified changed from 12/29/13 10:37:59 to 12/29/13 10:37:59
comment:14 follow-up: ↓ 17 Changed 6 years ago by
- 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:
43b696f | sage-sdist: don't require sage to be built
|
comment:15 follow-up: ↓ 16 Changed 6 years ago by
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: ↓ 19 Changed 6 years ago by
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: ↓ 18 Changed 6 years ago by
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: ↓ 20 ↓ 21 Changed 6 years ago by
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
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
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
- 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
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" 20 20 DST="$2" 21 21 22 22 rm -rf "$DST" 23 mkdir "$DST"23 mkdir -p "$DST" 24 24 25 25 git clone "$SRC" "$DST" 26 26 -
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" 46 46 # Download and copy all upstream tarballs (the -B option to make forces 47 47 # all targets to be built unconditionally) 48 48 cd "$SAGE_ROOT/build" 49 SAGE_INSTALL_GCC=yes SAGE_SPKG_OPTS="-f -d" SAGE_SPKG_COPY_UPSTREAM="$TMP_DIR/$TARGET/upstream" ./install -B all 49 SAGE_INSTALL_GCC=yes SAGE_SPKG_COPY_UPSTREAM="$TMP_DIR/$TARGET/upstream" \ 50 SAGE_INSTALL_FETCH_ONLY=yes SAGE_SPKG_OPTS="-f -d" \ 51 ./install -B all 50 52 51 53 # Create source .tar.gz 52 54 cd "$TMP_DIR"
comment:23 Changed 6 years ago by
- 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:
0255a2a | Allow sage --sdist without building Sage
|
comment:24 Changed 6 years ago by
- Status changed from needs_review to positive_review
Sure, this is fine.
comment:25 Changed 6 years ago by
Does this imply also a positive review to #15596 by transitivity?
comment:26 follow-up: ↓ 27 Changed 6 years ago by
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
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: ↓ 30 Changed 6 years ago by
Why does it say "does not merge cleanly"? It merges cleanly for me...
comment:29 follow-up: ↓ 33 Changed 6 years ago by
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: ↓ 32 Changed 6 years ago by
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 the merge 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.
comment:31 follow-up: ↓ 34 Changed 6 years ago by
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
comment:33 in reply to: ↑ 29 Changed 6 years ago by
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: ↓ 37 Changed 6 years ago by
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
.
comment:35 Changed 6 years ago by
- Status changed from positive_review to needs_work
I am working on this.
comment:36 Changed 6 years ago by
You're right, autoconf
version 2.60 does work (2.59 does not).
comment:37 in reply to: ↑ 34 ; follow-up: ↓ 38 Changed 6 years ago by
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 runautoconf
.
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.
comment:38 in reply to: ↑ 37 Changed 6 years ago by
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: ↓ 40 Changed 6 years ago by
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
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
- does not depend on configure output
- downloads a tarball
but that script does not need to
- check checksums
- 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
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
comment:42 Changed 6 years ago by
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
- Commit changed from 0255a2a53e9aebaf54c0fab335ef0202a6b77ae8 to 19d5a15ba83c7861237be766420541bb783ec34f
Branch pushed to git repo; I updated commit sha1. New commits:
19d5a15 | Create and use "configure" tarball for auto-generated files
|
comment:44 Changed 6 years ago by
comment:45 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:46 follow-up: ↓ 47 Changed 6 years ago by
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 immediately afterward. Any other change can break things, so requres testing, ie. another day twiddling thumbs.
comment:47 in reply to: ↑ 46 Changed 6 years ago by
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: ↓ 49 ↓ 68 Changed 6 years ago by
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
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: ↓ 51 Changed 6 years ago by
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
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: ↓ 53 Changed 6 years ago by
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
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:
- In volker's private repo, bump the version number and merge various patches
- Run
sage --sdist
to generate the configure tarball - Put the configure tarball in some accessible place
- Run the buildbots on Volker's private repo
- 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: ↓ 56 Changed 6 years ago by
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: ↓ 57 Changed 6 years ago by
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: ↓ 58 Changed 6 years ago by
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
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
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
- Commit changed from 19d5a15ba83c7861237be766420541bb783ec34f to 5d7729c51042f87a125e76b3e2819e564d2d562f
Branch pushed to git repo; I updated commit sha1. New commits:
5d7729c | Use build/pkgs/configure/package-version.txt for configure version number
|
comment:60 Changed 6 years ago by
Do you like this better?
comment:61 follow-up: ↓ 62 Changed 6 years ago by
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
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: ↓ 64 Changed 6 years ago by
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
comment:65 Changed 6 years ago by
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
- Commit changed from 5d7729c51042f87a125e76b3e2819e564d2d562f to e13b216c61977b1d9ae262791bb5bdaf392033cd
Branch pushed to git repo; I updated commit sha1. New commits:
e13b216 | Create sage-autogen script to generate configure tarball
|
comment:67 Changed 6 years ago by
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.
comment:68 in reply to: ↑ 48 Changed 6 years ago by
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: ↓ 78 Changed 6 years ago by
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
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
- Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580
comment:72 Changed 6 years ago by
- Commit changed from e13b216c61977b1d9ae262791bb5bdaf392033cd to 1c57e53f536db2f505b447ee6f617682372550d3
comment:73 Changed 6 years ago by
- 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
- Commit changed from 1c57e53f536db2f505b447ee6f617682372550d3 to cdc7c0a332ae31e61c6bf92a6e09e6dd7c4b8e1a
comment:75 Changed 6 years ago by
- 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
- 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
- Commit changed from cdc7c0a332ae31e61c6bf92a6e09e6dd7c4b8e1a to d0427228339f59811fc41d0ecb861d7e5e64a99a
New commits:
d042722 | bootstrap configure: small fixes
|
comment:78 in reply to: ↑ 69 Changed 6 years ago by
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: ↓ 82 ↓ 84 Changed 6 years ago by
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: ↓ 83 Changed 6 years ago by
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
Full rebuild works on mod, for the record.
comment:82 in reply to: ↑ 79 Changed 6 years ago by
Replying to vbraun:
Why does it even know about the
$SAGE_LOCAL
paths? The part should also set upLD_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
comment:84 in reply to: ↑ 79 Changed 6 years ago by
Replying to vbraun:
Why does it even know about the
$SAGE_LOCAL
paths? The part should also set upLD_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
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
- Commit changed from d0427228339f59811fc41d0ecb861d7e5e64a99a to e03f296685f986f89e1e12974f4c5ead9a9ff440
Branch pushed to git repo; I updated commit sha1. New commits:
e03f296 | Source sage-env inside configure
|
comment:87 Changed 6 years ago by
Mirror directory layout is SAGE_UPSTREAM/configure/configure-n.tar.gz
, for the record
comment:88 Changed 6 years ago by
- Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580
comment:89 Changed 6 years ago by
- 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:
3e5dfa5 | fix tarball url
|
comment:90 Changed 6 years ago by
- 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
- Commit changed from 3e5dfa5f0623f015faf6a81d96dbf3aaaa7f712b to e1df3ce1e4a7004cc9ca0aef6008c9edde6c6e54
comment:92 follow-up: ↓ 98 Changed 6 years ago by
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
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
PPS: any thoughts on enabling AM_MAINTAINER_MODE
?
comment:95 Changed 6 years ago by
- Branch changed from u/jdemeyer/ticket/15580 to u/vbraun/ticket/15580
comment:96 Changed 6 years ago by
- 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:
a38f919 | allow disabling of maintainer mode
|
9c13193 | only run autotools from the bootstrap script
|
comment:97 Changed 6 years ago by
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
.
comment:98 in reply to: ↑ 92 Changed 6 years ago by
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
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
- 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
- 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:
2e27abb | Don't use "missing"
|
comment:102 follow-ups: ↓ 104 ↓ 105 Changed 6 years ago by
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
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: ↓ 106 Changed 6 years ago by
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
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: ↓ 107 Changed 6 years ago by
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
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
- 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
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
- Resolution set to fixed
- Status changed from positive_review to closed
Actually
prereq-1.2.tar.gz
should work if you drop it into theupstream
directory, but since it was different, it didn't receive any of the extra treatment the other packages got.