Opened 5 years ago
Closed 4 years ago
#23700 closed enhancement (fixed)
update gc to version 7.6
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | packages: standard | Keywords: | |
Cc: | fbissey, embray | Merged in: | |
Authors: | Dima Pasechnik, Erik Bray | Reviewers: | Erik Bray, Dima Pasechnik, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | cd63b82 (Commits, GitHub, GitLab) | Commit: | cd63b82e0d58ecd4961a22e084c6d43af667c7bc |
Dependencies: | Stopgaps: |
Description (last modified by )
An update to gc is needed to support arm64/aarch64, see #23687, as well as for FreeBSD support, see #22679. It also appears to be good to be in sync with Linux boxes that ship make with guile extension (the latter depends on gc, and causes interesting errors, see #24575).
The source is here: https://github.com/ivmai/bdwgc/releases/download/v7.6.4/gc-7.6.4.tar.gz
On some platforms this new version also needs libatomic_ops, see #23701
Change History (58)
comment:1 Changed 5 years ago by
- Dependencies set to #23701
- Description modified (diff)
comment:2 Changed 5 years ago by
- Commit changed from 37cbca100a8be09b868a9280550de5a039600cac to 277ddd77e391269febfa10a66c62f34f475b4181
comment:3 Changed 5 years ago by
- Cc fbissey embray added
- Status changed from new to needs_review
Erik, could you please check whether this works on Cygwin without your patch?
(one need #23701 first, and re-running ./configure
to register the new package)
comment:4 Changed 5 years ago by
Ah, yes. I remember this. I'll give it a go.
comment:5 Changed 5 years ago by
I think this branch needs to be updated on the latest version of #23701 since it's using 7.4.6 and this branch is using 7.4.4.
comment:6 Changed 5 years ago by
- Commit changed from 277ddd77e391269febfa10a66c62f34f475b4181 to e53f78480379ce726091927c870c0869cb115b40
comment:7 Changed 5 years ago by
You're right - done now.
comment:8 Changed 5 years ago by
Thanks--now it's working. Or at least, it's building. Can't say anything about tests right now.
comment:9 Changed 5 years ago by
on FreeBSD this needs the patch fixing https://github.com/ivmai/bdwgc/issues/132
comment:10 Changed 5 years ago by
- Commit changed from e53f78480379ce726091927c870c0869cb115b40 to 931652aecee2bd1d775ccbd8fcc18aa5d1afb0ee
comment:11 Changed 5 years ago by
- Status changed from needs_review to needs_work
On Linux tests pass, but I can trigger a nasty crash interactively:
sage: from sage.libs.ecl import * sage: from sage.interfaces.maxima_lib import <TAB>
Debian experimental uses gc 7.4.4, so this would be one to test for this regression, too.
comment:12 Changed 5 years ago by
With GC included in ECL (tagged version 7.5.0 in ECL source, although I haven't checked how canonical this is) I do get the crash in comment 11.
comment:13 Changed 5 years ago by
- Commit changed from 931652aecee2bd1d775ccbd8fcc18aa5d1afb0ee to e365c32caf4e82cc9f6a43f066d8314313756248
comment:14 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:15 Changed 5 years ago by
- Status changed from needs_review to needs_work
Turns out this is still not building properly on Cygwin. It fails to produce a shared lib at link time:
/bin/sh ./libtool --tag=CC --mode=link gcc -fexceptions -Wall -Wextra -O2 -g -fno-strict-aliasing -version-info 1:3:0 -no-undefined -L/home/embray/src/sagemath/sage/local/lib -Wl,-rpath,/home/embray/src/sagemath/sage/local/lib -o libgc.la -rpath /home/embray/src/sagemath/sage/local/lib allchblk.lo alloc.lo blacklst.lo checksums.lo dbg_mlc.lo dyn_load.lo finalize.lo gc_dlopen.lo gcj_mlc.lo headers.lo mach_dep.lo malloc.lo mallocx.lo mark.lo mark_rts.lo misc.lo new_hblk.lo obj_map.lo os_dep.lo pcr_interface.lo ptr_chck.lo real_malloc.lo reclaim.lo specific.lo stubborn.lo thread_local_alloc.lo typd_mlc.lo win32_threads.lo fnlz_mlc.lo -L/home/embray/src/sagemath/sage/local/lib -latomic_ops mv -f cord/.deps/libcord_la-cordxtra.Tpo cord/.deps/libcord_la-cordxtra.Plo *** Warning: linker path does not have real file for library -latomic_ops. *** I have the capability to make that library automatically link in when *** you link to this library. But I can only do this if you have a *** shared version of the library, which you do not appear to have *** because I did check the linker path looking for a file starting *** with libatomic_ops and none of the candidates passed a file format test *** using a file magic. Last file checked: /home/embray/src/sagemath/sage/local/lib/libatomic_ops.a *** The inter-library dependencies that have been dropped here will be *** automatically added whenever a program is linked with this library *** or is declared to -dlopen it. *** Since this library must not contain undefined symbols, *** because either the platform does not support them or *** it was explicitly requested with -no-undefined, *** libtool will only create a static version of it. libtool: link: ar cru .libs/libgc.a allchblk.o alloc.o blacklst.o checksums.o dbg_mlc.o dyn_load.o finalize.o gc_dlopen.o gcj_mlc.o headers.o mach_dep.o malloc.o mallocx.o mark.o mark_rts.o misc.o new_hblk.o obj_map.o os_dep.o pcr_interface.o ptr_chck.o real_malloc.o reclaim.o specific.o stubborn.o thread_local_alloc.o typd_mlc.o win32_threads.o fnlz_mlc.o libtool: link: ranlib .libs/libgc.a libtool: link: ( cd ".libs" && rm -f "libgc.la" && ln -s "../libgc.la" "libgc.la" ) /bin/sh ./libtool --tag=CC --mode=link gcc -fexceptions -Wall -Wextra -O2 -g -fno-strict-aliasing -version-info 1:3:0 -no-undefined -L/home/embray/src/sagemath/sage/local/lib -Wl,-rpath,/home/embray/src/sagemath/sage/local/lib -o libcord.la -rpath /home/embray/src/sagemath/sage/local/lib cord/libcord_la-cordbscs.lo cord/libcord_la-cordprnt.lo cord/libcord_la-cordxtra.lo ./libgc.la *** Warning: This system cannot link to static lib archive ./libgc.la. *** I have the capability to make that library automatically link in when *** you link to this library. But I can only do this if you have a *** shared version of the library, which you do not appear to have. *** Warning: linker path does not have real file for library -latomic_ops. *** I have the capability to make that library automatically link in when *** you link to this library. But I can only do this if you have a *** shared version of the library, which you do not appear to have *** because I did check the linker path looking for a file starting *** with libatomic_ops and none of the candidates passed a file format test *** using a file magic. Last file checked: /home/embray/src/sagemath/sage/local/lib/libatomic_ops.a *** The inter-library dependencies that have been dropped here will be *** automatically added whenever a program is linked with this library *** or is declared to -dlopen it. *** Since this library must not contain undefined symbols, *** because either the platform does not support them or *** it was explicitly requested with -no-undefined, *** libtool will only create a static version of it. libtool: link: ar cru .libs/libcord.a cord/libcord_la-cordbscs.o cord/libcord_la-cordprnt.o cord/libcord_la-cordxtra.o libtool: link: ranlib .libs/libcord.a libtool: link: ( cd ".libs" && rm -f "libcord.la" && ln -s "../libcord.la" "libcord.la" )
It's probably a minor issue with link flag ordering. I will investigate and provide a solution.
comment:16 Changed 5 years ago by
- Commit changed from e365c32caf4e82cc9f6a43f066d8314313756248 to 32b8eeba01a780943c24e58e8108582babeec8b7
comment:17 Changed 5 years ago by
updated the branch to take into account update on #23701
comment:18 Changed 5 years ago by
- Description modified (diff)
comment:19 Changed 5 years ago by
Ah, I should have warned you, I'm about to propose another needed change for #23701.
comment:20 Changed 5 years ago by
We should go for 7.6.2: https://github.com/ivmai/bdwgc/releases/tag/v7.6.2 In particular, this includes the patches needed by the FreeBSD port. I will update this accordingly.
comment:21 Changed 5 years ago by
- Commit changed from 32b8eeba01a780943c24e58e8108582babeec8b7 to 7db1b1444d1dbb2829f06a78585c8bb40bca5855
comment:22 Changed 5 years ago by
- Description modified (diff)
- Milestone changed from sage-8.1 to sage-8.2
comment:23 Changed 5 years ago by
- Commit changed from 7db1b1444d1dbb2829f06a78585c8bb40bca5855 to b97bf993652a49c9dbc14dd40e2bed2090e70465
Branch pushed to git repo; I updated commit sha1. New commits:
b97bf99 | removing already included patches
|
comment:24 Changed 5 years ago by
- Commit changed from b97bf993652a49c9dbc14dd40e2bed2090e70465 to e790e65e44e0ff74a9b58ee8366348f3f2ca9a5d
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e790e65 | removing already included patches
|
comment:25 Changed 5 years ago by
- Status changed from needs_work to needs_review
OK, another advantage is that all our patches are now upstream!
comment:26 Changed 5 years ago by
I'm trying to get this built on Cygwin--will report back with any findings.
comment:27 Changed 5 years ago by
IIRC I had some trouble with this on Cygwin (specifically with libatomic_ops), but that was a while ago and I got distracted. Going to give it another try.
comment:28 Changed 5 years ago by
Likewise with ticket:23701#comment:15 would you accept a small update to this for Cygwin?
comment:29 Changed 5 years ago by
sure
comment:30 Changed 5 years ago by
- Branch changed from u/dimpase/gcupdate to public/ticket-23700
- Commit changed from e790e65e44e0ff74a9b58ee8366348f3f2ca9a5d to c2eb88c41a904432f8f6e81a6f462ed01f909ee0
- Reviewers set to Erik Bray
Rebased on top of my updates to #23701 and incorporated a minor change for Cygwin.
New commits:
7135b2f | On Cygwin install libatomic_ops as a shared lib only; this is inline with what the official Cygwin package does
|
18813a2 | Convert libatomic_ops to use sage-dist-helpers
|
e57cfc4 | update to 7.6.0
|
897c091 | patch for FreeBSD
|
2d923f2 | backported patch for FreeBSD support
|
822cc06 | removing already included patches
|
f6fd533 | update gc to 7.6.2
|
c2eb88c | For Cygwin only builds shared libraries
|
comment:31 Changed 4 years ago by
- Reviewers changed from Erik Bray to Erik Bray, Dima Pasechnik
- Status changed from needs_review to positive_review
comment:32 Changed 4 years ago by
- Status changed from positive_review to needs_work
Found local metadata for gc-7.6.2 Using cached file /mnt/disk/home/release/Sage/upstream/gc-7.6.2.tar.gz gc-7.6.2 ==================================================== Setting up build directory for gc-7.6.2 Finished extraction Applying patches from ../patches... Applying ../patches/0001-On-Cygwin-use-mprotect-instead-of-mmap-to-set-PROT_N.patch patching file os_dep.c Hunk #1 FAILED at 698. Hunk #2 FAILED at 2404. Hunk #3 FAILED at 2515. 3 out of 3 hunks FAILED -- saving rejects to file os_dep.c.rej Error applying '../patches/0001-On-Cygwin-use-mprotect-instead-of-mmap-to-set-PROT_N.patch' ************************************************************************ Error applying patches ************************************************************************ Please email sage-devel (http://groups.google.com/group/sage-devel) explaining the problem and including the log file /mnt/disk/home/release/Sage/logs/pkgs/gc-7.6.2.log Describe your computer, operating system, etc. ************************************************************************
comment:33 Changed 4 years ago by
facepalm... we should bump it up to 7.6.4 anyway.
comment:34 Changed 4 years ago by
- Description modified (diff)
I believe that the failing patch is already in 7.6.4, see https://github.com/ivmai/bdwgc/commit/be148f4141ab5d6812928a7bceeb2617384e7fe7
comment:35 Changed 4 years ago by
- Branch changed from public/ticket-23700 to u/dimpase/gc764
- Commit changed from c2eb88c41a904432f8f6e81a6f462ed01f909ee0 to 0263addf139baf3f62fda637bb220a7491af2f18
- Status changed from needs_work to needs_review
Last 10 new commits:
871bc6a | 7.6.2 is the current version
|
71e46a4 | On Cygwin install libatomic_ops as a shared lib only; this is inline with what the official Cygwin package does
|
0308045 | Convert libatomic_ops to use sage-dist-helpers
|
8c47a8a | update to 7.6.0
|
6148efd | patch for FreeBSD
|
7496a68 | backported patch for FreeBSD support
|
674a6d6 | removing already included patches
|
48766dd | update gc to 7.6.2
|
cdb00d4 | For Cygwin only builds shared libraries
|
0263add | updated to 7.6.4 and removed patch
|
comment:36 Changed 4 years ago by
- Description modified (diff)
comment:37 Changed 4 years ago by
- Status changed from needs_review to needs_work
build/pkgs/libatomic_ops/spkg-install
: you are wrongly using "BoehmGC".
build/pkgs/libatomic_ops/spkg-install
: instead of a generic variable likeCONFIGURE_FLAGS
, please useLIBATOMIC_OPS_CONFIGURE
to be more consistent with other packages.
build/pkgs/libatomic_ops/spkg-check
: there is no longer need for theif [ -z "$SAGE_LOCAL" ]; then
boilerplate.
comment:38 follow-up: ↓ 40 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:39 Changed 4 years ago by
They were just minor comments, nothing serious.
comment:40 in reply to: ↑ 38 Changed 4 years ago by
- Status changed from needs_review to needs_work
Replying to dimpase:
Jeroen,
libatomic_ops
has nothing to do with this ticket. It was #23701. If you think #23701 needs an improvement please open another ticket.
To be very precise, the commits which are supposedly from #23701 do not actually appear on the branch at #23701. So this ticket needs to be rebased.
comment:41 follow-up: ↓ 42 Changed 4 years ago by
There is also the obvious missing dependency
[gc-7.6.4] checking for atomic_ops... no [gc-7.6.4] checking atomic_ops.h usability... no [gc-7.6.4] checking atomic_ops.h presence... no [gc-7.6.4] checking for atomic_ops.h... no [gc-7.6.4] configure: error: libatomic_ops is required. You can either install it on [gc-7.6.4] your system, or fetch and unpack a recent version into the [gc-7.6.4] source directory and link or rename it to libatomic_ops. [gc-7.6.4] Error configuring BoehmGC.
comment:42 in reply to: ↑ 41 ; follow-up: ↓ 43 Changed 4 years ago by
Replying to jdemeyer:
There is also the obvious missing dependency
[gc-7.6.4] checking for atomic_ops... no [gc-7.6.4] checking atomic_ops.h usability... no [gc-7.6.4] checking atomic_ops.h presence... no [gc-7.6.4] checking for atomic_ops.h... no [gc-7.6.4] configure: error: libatomic_ops is required. You can either install it on [gc-7.6.4] your system, or fetch and unpack a recent version into the [gc-7.6.4] source directory and link or rename it to libatomic_ops. [gc-7.6.4] Error configuring BoehmGC.
good catch, but how do you get this? OS/compiler?
comment:43 in reply to: ↑ 42 Changed 4 years ago by
comment:44 Changed 4 years ago by
- Commit changed from 0263addf139baf3f62fda637bb220a7491af2f18 to 4d6eee33939ae419776f36b4cd08406e8212d663
Branch pushed to git repo; I updated commit sha1. New commits:
4d6eee3 | added deps; addressed reviewer comments: #23700:37
|
comment:45 Changed 4 years ago by
- Status changed from needs_work to needs_review
all fixed, hopefully.
comment:46 Changed 4 years ago by
- Status changed from needs_review to needs_work
- This branch still needs to be rebased to the branch at #23701.
- You should replace
CONFIGURE_FLAGS
->LIBATOMIC_OPS_CONFIGURE
everywhere inbuild/pkgs/libatomic_ops/spkg-install
comment:47 Changed 4 years ago by
- Commit changed from 4d6eee33939ae419776f36b4cd08406e8212d663 to cd63b82e0d58ecd4961a22e084c6d43af667c7bc
Branch pushed to git repo; I updated commit sha1. New commits:
4d5bbd0 | libatomic_ops 7.4.6 added as standard pkg
|
7b0f626 | Merge branch 'u/dimpase/libatomic_ops' of trac.sagemath.org:sage into laup
|
5c5d8f6 | remove meaningless flags
|
54e1da7 | Merge branch 'develop' into libatomic_ops
|
a024f8d | 7.6.2 is the current version
|
7135b2f | On Cygwin install libatomic_ops as a shared lib only; this is inline with what the official Cygwin package does
|
18813a2 | Convert libatomic_ops to use sage-dist-helpers
|
8d05b70 | Merge branch 'atomic762' into gc764
|
cd63b82 | LIBATOMIC_OPS_CONFIGURE throughout
|
comment:48 Changed 4 years ago by
- Status changed from needs_work to needs_review
I've addressed the comment 46.
comment:49 Changed 4 years ago by
The git history is a total mess now... at least messy enough that I'd like to see it merged with the next beta before considering to review it.
comment:50 Changed 4 years ago by
well, yes, I tried to rebase, and it was really doing weird stuff, such as creating LICENSE
file and changing .gitignore
, so I merged instead.
comment:51 Changed 4 years ago by
Perhaps I could try to clean it up?
comment:52 follow-up: ↓ 53 Changed 4 years ago by
- Dependencies #23701 deleted
Hopefully it is clean enough now, as the merge of #23701 has happened.
comment:53 in reply to: ↑ 52 Changed 4 years ago by
comment:54 follow-up: ↓ 55 Changed 4 years ago by
The diff is pretty small now: there are largish patches removed, and the rest it really "admin".
Erik, does the branch work on Cygwin for you now? (You never answered comment 34 above). The commit in the upstream repo seems to do the same job and is authored by you, but it looks differently---hopefully it does the same thing.
comment:55 in reply to: ↑ 54 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:56 Changed 4 years ago by
- Reviewers changed from Erik Bray, Dima Pasechnik to Erik Bray, Dima Pasechnik, Jeroen Demeyer
comment:57 Changed 4 years ago by
LGTM. My offer still stands to clean up the commit history if desired, but otherwise I'll let it be.
comment:58 Changed 4 years ago by
- Branch changed from u/dimpase/gc764 to cd63b82e0d58ecd4961a22e084c6d43af667c7bc
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
libatomic_ops added as standard pkg
Merge branch 'libatomic_ops' into gcupdate
libatomic_ops is a dependency