Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added recipe for scotch #1427

Merged
merged 32 commits into from
Oct 16, 2016
Merged

Added recipe for scotch #1427

merged 32 commits into from
Oct 16, 2016

Conversation

basnijholt
Copy link
Contributor

No description provided.

@conda-forge-linter
Copy link

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 (recipes/scotch) and found some lint.

Here's what I've got...

For recipes/scotch:

  • The recipe must have some tests.
  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

@conda-forge-linter
Copy link

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 (recipes/scotch) and found some lint.

Here's what I've got...

For recipes/scotch:

  • The recipe must have some tests.

@conda-forge-linter
Copy link

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 (recipes/scotch) and found it was in an excellent condition.

@jakirkham
Copy link
Member

Could you please restore the LICENSE file? It seems to be overwritten by something else.

sed -i "s@CLIBFLAGS\t=@CLIBFLAGS\t= -fPIC@g" Makefile.inc
sed -i 's#-l$(SCOTCHLIB)errexit#-l$(SCOTCHLIB)errexit -lm#g' esmumps/Makefile
fi
make esmumps | tee make.log 2>&1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make check or similar?

sed -i '' 's/-O3/-O3 -fPIC/g' Makefile.inc
else
cp Make.inc/Makefile.inc.x86-64_pc_linux2 Makefile.inc
sed -i "s@CFLAGS\t\t=@CFLAGS\t= -I${PREFIX}/include@" Makefile.inc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to do this for the library search path too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why or how?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would be to pick up zlib.

How would be to add -L${PREFIX}/lib. Though I really am hoping they have a configure file we overlooked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I looked at the source briefly. They don't have a configure. 😖

My recommendation would be to copy these files into the recipe and modify them as you see fit. We did this with SuiteSparse and had much success. Keeping this sed stuff is more likely to cause you hurt later on.

source:
fn: scotch_{{ version }}.tar.gz
md5: {{ md5 }}
url: http://gforge.inria.fr/frs/download.php/file/34618/scotch_6.0.4.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scotch_6.0.4.tar.gz -> scotch_{{ version }}.tar.gz. That should make it easier to update. 😉

@basnijholt basnijholt mentioned this pull request Aug 31, 2016
4 tasks
- test -f "${PREFIX}/lib/libscotcherr.a"
- test -f "${PREFIX}/lib/libscotcherrexit.a"
- test -f "${PREFIX}/lib/libscotchmetis.a"
- test -f "${PREFIX}/lib/libesmumps.a"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakirkham I can't find a better way to test these files? What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check for some dylibs/sos? Also some tests for installed headers would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no dylibs/sos as far as I can see. I did include the header file tests now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not seeing them either now that I'm looking. Is there an option to build dynamic libraries?

CAT = cat
CCS = gcc
CCP = mpicc
CCD = gcc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use clang?

Copy link
Contributor Author

@basnijholt basnijholt Sep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to clang

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I do it in the Linux Makefile.inc too?

version: {{ version }}

source:
fn: scotch_6.0.4.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to {{ name }}-{{ version }}.tar.gz.

Copy link
Contributor Author

@basnijholt basnijholt Sep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now jinja templated

source:
fn: scotch_6.0.4.tar.gz
md5: {{ md5 }}
url: http://gforge.inria.fr/frs/download.php/file/34618/scotch_6.0.4.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please template the version with jinja here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it, but because the number in the url changes for every file I initially didn't.

Copy link
Contributor Author

@basnijholt basnijholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed what @jakirkham suggested.

version: {{ version }}

source:
fn: scotch_6.0.4.tar.gz
Copy link
Contributor Author

@basnijholt basnijholt Sep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now jinja templated

CAT = cat
CCS = gcc
CCP = mpicc
CCD = gcc
Copy link
Contributor Author

@basnijholt basnijholt Sep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to clang

@basnijholt
Copy link
Contributor Author

@jakirkham I've changed what you suggested, two things left:

Can we check for some dylibs/sos?

I don't see that there are any dylibs or sos files generated

Why not use clang?

I have changed it for the MacOS makefile, but should I do it for Linux too?

source:
fn: {{ name }}-{{ version }}.tar.gz
md5: {{ md5 }}
url: http://gforge.inria.fr/frs/download.php/file/34618/{{ name }}-{{ version }}.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could try the link below. The number on it doesn't seem to change with each release. So it works better with templating.

https://gforge.inria.fr/frs/download.php/latestfile/298/{{ name }}_{{ version }}.tar.gz

@jakirkham
Copy link
Member

Seems to be picking up the system zlib. Could we fix it to pick up our zlib?

- zlib 1.2.* # [unix]

test:
commands:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a bunch of executables that are installed. Can we please run --help or --version or similar to check that they do run without crashing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a few.

@basnijholt
Copy link
Contributor Author

I have changed everything @jakirkham suggested, except that I don't know how to generate dynamic libraries, the -fPIC flag is already there at least.

@basnijholt
Copy link
Contributor Author

@jakirkham I've checked there is a Makefile.inc for Linux, not for OSX. However, it's not compiling correctly (d87384b), so I reverted the change.

@basnijholt basnijholt mentioned this pull request Oct 10, 2016
5 tasks
@basnijholt
Copy link
Contributor Author

I have spend a lot of time to get the dynamic lib compilation to work, but it's not working.

Also by looking for info about the Makefile.inc.x86-64_pc_linux2.shlib online, doesn't give me the confidence of being sure that it works at all.

@jakirkham
Copy link
Member

No worries. I think we are now at a healthy place to merge. While there is still room for growth, I'm more than happy to see that continue at the feedstock. Thanks very much for your hard work, @basnijholt .

@jakirkham jakirkham merged commit 6867020 into conda-forge:master Oct 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants