patch for setting install_name during install #23

minrk · 2017-03-07T03:13:46Z

I've submitted this patch upstream. Not sure yet if it will be accepted.

closes #22

conda-forge-linter · 2017-03-07T03:13:50Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

basnijholt · 2017-03-09T12:14:28Z

LGTM 👍

@jakirkham @grlee77 what do you think?

grlee77 · 2017-03-10T00:37:34Z

I am not familiar enough with Makefile syntax to judge if this is correct, so I would defer to those with more expertise. Two quick questions:

1.) In short, this patch only modifies behavior on OS X and causes the dynamic libraries to use e.g. *.4.5.dylib rather than the more specific *.4.5.4.dylib to be used as the install_name?

2.) Do we know that SuiteSparse has ABI compatibility between patch versions?

minrk · 2017-03-10T04:10:53Z

In short, this patch only modifies behavior on OS X and causes the dynamic libraries to use e.g. *.4.5.dylib rather than the more specific *.4.5.4.dylib to be used as the install_name?

According to the variables in the Makefiles, it would be *.4.dylib (major only), but otherwise yes.

Do we know that SuiteSparse has ABI compatibility between patch versions?

We know that suitesparse is setting the compatibility version in its dylibs, so at least the libraries themselves claim compatibility at the major version level. I can't speak for the project in general, but a cursory test of a conda package compiled against 4.5.3 updated manually to load 4.5.4 does work.

grlee77 · 2017-11-17T19:40:59Z

sorry this PR has gone neglected. I think now the build number should be bumped to 202 since the OpenBLAS update bumped it to 201.

@minrk: Is this otherwise good to go? if interested, please add yourself as a maintainer as well

conda-forge-linter · 2017-11-19T19:08:13Z

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-linter · 2017-11-19T19:09:05Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jakirkham · 2017-11-22T15:39:59Z

According to the ABI report for SuiteSparse it appears that at least 4.5 has been stable across patch release. Before that it was static libraries that are fairly old, which makes it hard to say much about its compatibility during that period.

jakirkham · 2018-04-17T22:27:46Z

Should we add a file test to ensure that the library name is as expected?

minrk · 2018-04-18T07:31:42Z

Good idea, I'll add a few calls to otool -L that verify the install name is right.

Aside: it would be cool if conda-build test environments could verify compatibility by running the test env with multiple versions of the built package.

In general, a useful pattern for verifying C libs that their linking config is set up correctly:

conda-build the lib
conda-build a test package that links the lib
install and test the test package against the oldest and newest versions of the lib that its dependency matrix would allow

minrk · 2018-06-27T10:17:12Z

Sorry for taking so long with the tests. The tests are here now, so we can start getting a suitesparse with correct linking on mac.

isuruf

Can you add compilers and remove openblas pinning?

isuruf · 2018-07-18T22:57:54Z

recipe/meta.yaml

@@ -10,10 +10,13 @@ source:
  fn: SuiteSparse-{{ version }}.tar.gz
  url: http://faculty.cse.tamu.edu/davis/SuiteSparse/SuiteSparse-{{ version }}.tar.gz
  sha256: de5fb496bdc029e55955e05d918a1862a177805fbbd5b957e8b5ce6632f6c77e
+  patches:
+    - install_name.patch

 build:
  skip: true  # [win]
  number: 200


Can you remove this line?

isuruf · 2018-07-18T22:58:38Z

recipe/meta.yaml


 build:
  skip: true  # [win]
  number: 200
+  number: 201
  features:
    - blas_{{ variant }}


Can you add

run_export: - {{ pin_subpackage("suitesparse") }}

here?

conda-forge-linter · 2018-07-19T05:56:13Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

The build section contained an unexpected subsection name. run_export is not a valid subsection name.

conda-forge-linter · 2018-07-19T08:39:49Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

minrk · 2018-07-19T08:40:32Z

This is now also a full conda-build 3 update per @isuruf's request.

patch submitted upstream

add minrk as maintainer

master has been updated since this PR was opened

minrk · 2018-08-16T13:26:06Z

Rebased again

conda-forge-linter · 2019-08-28T07:52:43Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

license_file entry is missing, but is expected.

conda-forge-linter · 2019-08-28T07:54:52Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

all review addressed

minrk · 2019-08-28T09:12:11Z

I think this is good to go now (and has been for a year or so). Anything else blocking the merge?

SylvainCorlay · 2019-08-28T10:20:51Z

👍

recipe/conda_build_config.yaml

now redundant with conda-forge-pinning

minrk · 2019-09-19T11:52:36Z

Conflicts resolved again, build number bumped again.

minrk · 2019-09-19T14:03:39Z

All builds have succeeded. This should be ready to go again. Not sure why osx shows as still queued, since it's succeeded a while ago.

- move pinning to conda_build_config.yaml pin_run_as_build - add host - cb3 compiler spec

minrk mentioned this pull request Nov 16, 2017

Update openblas pin conda-forge/petsc-feedstock#20

Closed

minrk force-pushed the install_name branch from 083dfb3 to fec65a7 Compare November 19, 2017 19:09

martinandersen mentioned this pull request Apr 17, 2018

Update to CVXOPT 1.2.0 conda-forge/cvxopt-feedstock#17

Merged

minrk mentioned this pull request Apr 17, 2018

investigate setting install name #22

Closed

jakirkham mentioned this pull request May 11, 2018

Pinning suitesparse conda-forge/conda-forge-pinning-feedstock#59

Closed

minrk force-pushed the install_name branch 2 times, most recently from 33e96a6 to e987def Compare June 26, 2018 20:30

isuruf previously requested changes Jul 18, 2018

View reviewed changes

minrk force-pushed the install_name branch from 670c772 to 922314a Compare July 19, 2018 08:39

minrk added 6 commits August 16, 2018 15:23

patch for setting install_name during install

58fd44c

patch submitted upstream

build 201

ba0407a

add minrk as maintainer

add run_test for mac install name

6429ba0

fix expected install_name

2bd4dd8

conda-build 3

10c72e6

bump build number again

8d6423a

master has been updated since this PR was opened

minrk force-pushed the install_name branch from 7116f4f to 8d6423a Compare August 16, 2018 13:25

Merge branch 'master' into install_name

2f7c29d

package license file

f1d52cf

isuruf reviewed Aug 28, 2019

View reviewed changes

recipe/conda_build_config.yaml Outdated Show resolved Hide resolved

minrk added 2 commits September 2, 2019 12:33

remove conda_build_config.yaml

592078b

now redundant with conda-forge-pinning

Merge branch 'master' into install_name

cf0545b

minrk requested review from basnijholt, grlee77 and jakirkham as code owners September 19, 2019 11:51

bump build number again

f474dfb

grlee77 merged commit d7cd426 into conda-forge:master Sep 20, 2019

minrk deleted the install_name branch September 24, 2019 08:33

This was referenced Oct 3, 2019

Missing run_exports statement #45

Closed

Package license file #39

Closed

minrk referenced this pull request in conda-forge/petsc-feedstock Apr 4, 2022

conda-build 3

251b3dc

- move pinning to conda_build_config.yaml pin_run_as_build - add host - cb3 compiler spec

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch for setting install_name during install #23

patch for setting install_name during install #23

minrk commented Mar 7, 2017

conda-forge-linter commented Mar 7, 2017

basnijholt commented Mar 9, 2017 •

edited

Loading

grlee77 commented Mar 10, 2017

minrk commented Mar 10, 2017

grlee77 commented Nov 17, 2017

conda-forge-linter commented Nov 19, 2017

conda-forge-linter commented Nov 19, 2017

jakirkham commented Nov 22, 2017 •

edited

Loading

jakirkham commented Apr 17, 2018

minrk commented Apr 18, 2018

minrk commented Jun 27, 2018

isuruf left a comment

isuruf Jul 18, 2018

isuruf Jul 18, 2018

conda-forge-linter commented Jul 19, 2018

conda-forge-linter commented Jul 19, 2018

minrk commented Jul 19, 2018

minrk commented Aug 16, 2018

conda-forge-linter commented Aug 28, 2019

conda-forge-linter commented Aug 28, 2019

minrk commented Aug 28, 2019

SylvainCorlay commented Aug 28, 2019

minrk commented Sep 19, 2019

minrk commented Sep 19, 2019

patch for setting install_name during install #23

patch for setting install_name during install #23

Conversation

minrk commented Mar 7, 2017

conda-forge-linter commented Mar 7, 2017

basnijholt commented Mar 9, 2017 • edited Loading

grlee77 commented Mar 10, 2017

minrk commented Mar 10, 2017

grlee77 commented Nov 17, 2017

conda-forge-linter commented Nov 19, 2017

conda-forge-linter commented Nov 19, 2017

jakirkham commented Nov 22, 2017 • edited Loading

jakirkham commented Apr 17, 2018

minrk commented Apr 18, 2018

minrk commented Jun 27, 2018

isuruf left a comment

Choose a reason for hiding this comment

isuruf Jul 18, 2018

Choose a reason for hiding this comment

isuruf Jul 18, 2018

Choose a reason for hiding this comment

conda-forge-linter commented Jul 19, 2018

conda-forge-linter commented Jul 19, 2018

minrk commented Jul 19, 2018

minrk commented Aug 16, 2018

conda-forge-linter commented Aug 28, 2019

conda-forge-linter commented Aug 28, 2019

minrk commented Aug 28, 2019

SylvainCorlay commented Aug 28, 2019

minrk commented Sep 19, 2019

minrk commented Sep 19, 2019

basnijholt commented Mar 9, 2017 •

edited

Loading

jakirkham commented Nov 22, 2017 •

edited

Loading