Opened 6 years ago

Closed 5 years ago

#15586 closed enhancement (fixed)

Update Valgrind to 3.10.0

Reported by: jpflori Owned by:
Priority: minor Milestone: sage-6.4
Component: packages: optional Keywords: spkg valgrind
Cc: vbraun, iandrus Merged in:
Authors: Jean-Pierre Flori, Volker Braun Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: e150b39 (Commits) Commit: e150b39b41e1cd48f8785e3f7d778004234b262f
Dependencies: Stopgaps:

Change History (32)

comment:1 Changed 6 years ago by jpflori

  • Branch set to u/jpflori/ticket/15585
  • Keywords spkg valgrind added
  • Status changed from new to needs_review

Sole non-trivial modification was to remove a patch now upstream. Whatsoever, we should still take care of #14097.

comment:2 Changed 6 years ago by jpflori

  • Branch changed from u/jpflori/ticket/15585 to u/jpflori/ticket/15586
  • Commit set to 9d5a3bfe7da7a8c526fdafa0f0d62533dc110cae

New commits:

9d5a3bfUpdate Valgrind to 3.9.0.

comment:3 Changed 6 years ago by git

  • Commit changed from 9d5a3bfe7da7a8c526fdafa0f0d62533dc110cae to 556c5f499eb9d56927fa4d577a69652dbd984fe9

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

556c5f4Update Valgrind to 3.9.0.

comment:4 Changed 6 years ago by jpflori

  • Cc vbraun iandrus added

comment:5 Changed 6 years ago by vbraun

tarball is go-r

vbraun@boxen:~$ ll /home/jpflori/upstream/valgrind-3.9.0.tar.bz2
-rw------- 1 jpflori jpflori 10003156 2013-12-25 08:42 /home/jpflori/upstream/valgrind-3.9.0.tar.bz2

comment:6 Changed 6 years ago by jpflori

Fixed.

comment:7 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:9 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:10 Changed 5 years ago by jdemeyer

I dislike

+if [ "$UNAME" = "Linux" ]; then
+ echo "Good - Valgrind works on Linux"
+ if [ "`uname -p`" = "ia64" ]; then
+ echo >&2 "But it does not work on Itanium"
+ exit 1
+ fi
+else
+ if [ "$UNAME" = "Darwin" ] && [ -z "`uname -p | grep PPC`" ] && [ `uname -r | grep -Eo '^[0-9]+'` -ge 9 ];
+ then
+ echo "Good - Valgrind works on x86 and AMD64 Darwin 9.x or 10.x"
+ else
+ echo >&2 "Sorry, Valgrind only works on x86,AMD64,PPC32,PPC64,ARM Linux"
+ echo >&2 "and x86,AMD64 on Darwin 9.x/10.x"\
+ "(Mac OS X 10.5.x or later)"
+ exit 1
+ fi
+fi

If it doesn't work, fine, just let the build fail (one would hope that upstream gives a reasonable error message in this case). Too often, these kind of checks prevent building on systems which are perfectly fine.

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

comment:11 Changed 5 years ago by jdemeyer

Also, the suppressions are not patches so I see no reason to put them in the patches directory.

comment:12 Changed 5 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

comment:13 follow-up: Changed 5 years ago by jpflori

The good/bad stuff is historical.

As far as non patch stuff in the patches directory, I had the feeling some other packages did that.

And my first concern here is that I tested the package a couple of days ago and:

  • it does not build at first, stupid glibc version restriction as usual with valgrind...
  • and it does not work properly, or at least did not for me: at least Sage now launches (not as reported in #14097 which I could reproduce when the ticket was active), but I cannot exit it...

comment:14 Changed 5 years ago by jpflori

(I tested it on ppc64 and x86_64.)

comment:15 Changed 5 years ago by vbraun

  • Authors changed from Jean-Pierre Flori to Jean-Pierre Flori, Volker Braun
  • Description modified (diff)
  • Summary changed from Update Valgrind to 3.9.0 to Update Valgrind to 3.10.0

comment:16 Changed 5 years ago by vbraun

  • Branch changed from u/jpflori/ticket/15586 to u/vbraun/valgrind_3_10

comment:17 Changed 5 years ago by vbraun

  • Commit changed from 556c5f499eb9d56927fa4d577a69652dbd984fe9 to bed4a70febbd3b8585e6d307cb895a14cd984bd6

I'm moving the suppression files to $SAGE_EXTCODE/valgrind/sage.supp, that way we can even use the system valgrind (which is also more likely to work with your version of glibc).


New commits:

7b1989dUpdate Valgrind to 3.9.0.
bed4a70Update Valgrind to 3.10.0

comment:18 Changed 5 years ago by git

  • Commit changed from bed4a70febbd3b8585e6d307cb895a14cd984bd6 to 2fb8eb335366a8985ae6b2fab4d3c6ae55187469

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

2fb8eb3Move suppression files to extcode

comment:19 Changed 5 years ago by git

  • Commit changed from 2fb8eb335366a8985ae6b2fab4d3c6ae55187469 to 8d962d29374cf5a14cfc6e715ea0f93d7458b371

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

8d962d2Add PyAlloc suppressions

comment:20 Changed 5 years ago by vbraun

  • Status changed from needs_work to needs_review

Works and is actually useful (doesn't spew spurious pyalloc warnings all the time)

comment:21 in reply to: ↑ 13 Changed 5 years ago by jdemeyer

Replying to jpflori:

The good/bad stuff is historical.

Perhaps, but I still think it should be removed.

comment:22 Changed 5 years ago by git

  • Commit changed from 8d962d29374cf5a14cfc6e715ea0f93d7458b371 to e150b39b41e1cd48f8785e3f7d778004234b262f

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

e150b39Remove unnecessary checks

comment:23 Changed 5 years ago by vbraun

Fixed.

comment:24 Changed 5 years ago by jpflori

Volker: One naive question: is the sage -valgind stuff actually working for you? last time I tested (see above), everything was actually broken except for Sage starting.)

comment:25 Changed 5 years ago by vbraun

Yes, works. Fixed that, too :-)

comment:26 Changed 5 years ago by jpflori

It seems there is an issue with the new location of the supp files and where the sage-valgrind script looks for.

comment:27 Changed 5 years ago by vbraun

Whats the issue? You need to run "make" to install it to SAGE_EXTCODE.

comment:28 Changed 5 years ago by jpflori

  • Status changed from needs_review to positive_review

Oh my bad.... Then everything is OK.

comment:29 Changed 5 years ago by jpflori

I'm still getting thousands of errors when starting up. Is that expected ?

comment:30 Changed 5 years ago by vbraun

Did you recompile python, if not then the python suppression file is not installed. The patch doesn't update the python version so it is not recompiled automatically.

comment:31 Changed 5 years ago by jpflori

Nope I did not.

comment:32 Changed 5 years ago by vbraun

  • Branch changed from u/vbraun/valgrind_3_10 to e150b39b41e1cd48f8785e3f7d778004234b262f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.