Opened 6 years ago

Closed 6 years ago

#17588 closed defect (fixed)

Remove brain damaged installation procedure for pari on OS X

Reported by: fbissey Owned by:
Priority: major Milestone: sage-6.5
Component: packages: standard Keywords:
Cc: kcrisman, jhpalmieri, was Merged in:
Authors: François Bissey Reviewers: Karl-Dieter Crisman, John Palmieri
Report Upstream: N/A Work issues:
Branch: 0fa866c (Commits) Commit: 0fa866c5fa60de81c981163c3e8697ef158bf119
Dependencies: Stopgaps:

Description

The current spkg-install script for pari includes the following specific darwing instruction

if [ "$UNAME" = "Darwin" ]; then
        # The following is a hack to get the library installed correctly
        # on Mac OS X:
        echo "Applying Mac OS X hack -- deleting and reinstalling libpari.a ..."
        rm -f Odarwin-ppc/*lib*
        $MAKE install-lib-sta
        if [ $? -ne 0 ]; then
            echo >&2 "Error reinstalling PARI's static libraries (OS X trick)"
            exit 1
        fi

        cd "`config/objdir`" && cp -f libpari* "$SAGE_LOCAL/lib"
        if [ $? -ne 0 ]; then
            echo >&2 "Error copying PARI's libraries (OS X trick)"
            exit 1
        fi
        cd "$CUR"
    fi

This section completely destroy the proper setting of darwin's install_name for no good reason. It needs to be removed. See conversation in #16017 and #16044 about this.

Change History (15)

comment:1 follow-up: Changed 6 years ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/pari_osx
  • Cc kcrisman added
  • Commit set to 53e6d2e4e2b20d24a154a8c8a33e3db10d3345ab
  • Status changed from new to needs_review

Ready for review if you could Karl-Dieter...


New commits:

53e6d2eRemove counter productive installation procedure for darwin (OS X) it currently destroys the proper setting of OS X install_name.

comment:2 Changed 6 years ago by git

  • Commit changed from 53e6d2e4e2b20d24a154a8c8a33e3db10d3345ab to 62039fb0c4647f55d7eb63fd90f77c525589652d

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

62039fbForgot to bump the revision level

comment:3 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by kcrisman

Ready for review if you could Karl-Dieter...

Haha! I might get a chance to do it on a couple older machines (10.4 and 10.7) eventually, but you should have reviewers testing it on 10.9 and 10.10, methinks.

comment:4 in reply to: ↑ 3 Changed 6 years ago by fbissey

Replying to kcrisman:

Ready for review if you could Karl-Dieter...

Haha! I might get a chance to do it on a couple older machines (10.4 and 10.7) eventually, but you should have reviewers testing it on 10.9 and 10.10, methinks.

I got 10.10 covered before committing.

comment:5 Changed 6 years ago by kcrisman

This passes all long doctests on 10.7 (other than the PowmInsecureWarning: Not using mpz_powm_sec. and a Maxima timeout, but those are unrelated and happen to me anyway).

Maybe someone should test on 10.9, but otherwise I wouldn't hold it up for me testing on any older platform.

comment:6 Changed 6 years ago by kcrisman

  • Cc jhpalmieri was added
  • Reviewers set to Karl-Dieter Crisman

John or William, can you quick check this doesn't break anything? I don't have access to the right version of OS X and François is of course the author here. Should be a very quick finish of review.

comment:7 Changed 6 years ago by fbissey

pari has been updated in rc0 I need to rebase this.

comment:8 Changed 6 years ago by fbissey

  • Branch changed from u/fbissey/pari_osx to u/fbissey/trac17588-pari-osx
  • Commit changed from 62039fb0c4647f55d7eb63fd90f77c525589652d to 0fa866c5fa60de81c981163c3e8697ef158bf119

New branch pushed.


New commits:

0fa866cTrac #17588: Remove darwin specific install instruction that are counterproductive.

comment:9 Changed 6 years ago by fbissey

No new code just made sure it would merge ok.

comment:10 Changed 6 years ago by kcrisman

Just need someone to check this independently of you on 10.9 or 10.10 (preferred), then.

comment:11 Changed 6 years ago by jhpalmieri

It certainly builds on OS X 10.10 with sage -f pari. I need to do make distclean; make, though, to ensure that it's really working. This is in progress. (If the installation procedure is being changed, then just doing sage -f pari might leave parts of the old installation lying around, so it feels safer to delete everything and start over.)

comment:12 Changed 6 years ago by fbissey

It shouldn't as the file set installed is the same. Just the content of the dylib headers are different. But it is better to rebuild all pari dependencies to make sure it has been taken into account and rebuilding will do that for you.

comment:13 Changed 6 years ago by jhpalmieri

This works for me on OS X 10.10.

comment:14 Changed 6 years ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, John Palmieri
  • Status changed from needs_review to positive_review

All is well, then?

comment:15 Changed 6 years ago by vbraun

  • Branch changed from u/fbissey/trac17588-pari-osx to 0fa866c5fa60de81c981163c3e8697ef158bf119
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.