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

Add cudatoolkit #6240

Closed
wants to merge 40 commits into from
Closed

Add cudatoolkit #6240

wants to merge 40 commits into from

Conversation

xmnlab
Copy link
Member

@xmnlab xmnlab commented Jul 5, 2018

Added recipe for cudatoolkit for linux.
next step would be to add recipes for win and osx platform

@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/cudatoolkit) and found it was in an excellent condition.

@xmnlab
Copy link
Member Author

xmnlab commented Jul 5, 2018

added @scopatz @andersy005 as recipe-maintainers.

@xmnlab
Copy link
Member Author

xmnlab commented Jul 5, 2018

it seems it is broken because some perl issue. I am taking a look on that.

recipe-maintainers:
- xmnlab
- scopatz
- andersy005
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @xmnlab! @andersy005 are you OK with being a maintainer?

Copy link
Member

Choose a reason for hiding this comment

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

@scopatz, Yes!

@@ -0,0 +1,57 @@
#!/bin/bash

Copy link
Member

@scopatz scopatz Jul 5, 2018

Choose a reason for hiding this comment

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

Can you add a set -ex line here please, so that we can debug this? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

sure! thanks!

@scopatz
Copy link
Member

scopatz commented Jul 5, 2018

Thanks for putting this in @xmnlab!

@scopatz
Copy link
Member

scopatz commented Jul 5, 2018

it seems it is broken because some perl issue. I am taking a look on that.

This is probably because they used /usr/bin/perl in one of their scripts rather than /usr/bin/env perl

@scopatz
Copy link
Member

scopatz commented Jul 5, 2018

Also, you may be able to use a yum_requirements.txt file to install perl into the system. Here is an example feedstock: https://github.com/conda-forge/imagepy-feedstock/blob/master/recipe/yum_requirements.txt

This is an alternative to doing a sed replace on whatever file is running the perl is.

@xmnlab
Copy link
Member Author

xmnlab commented Jul 5, 2018

thanks @scopatz !

I will try yum_requirements approach. normally I use sed to replace some code to work around some issue to build the package .. but in this case it seems that the runfile extract and run perl scripts ... I have no time to apply a sed command before run scrips are called.

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

I have no time to apply a sed command before run scrips are called

Yeah, it would seem that you'd need to use --extract in order to use sed. yum_requirements.txt seems to be working though.

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

Since this seems to be working for Linux, I'd likt o bring it to the attention of @conda-forge/staged-recipes @conda-forge/core for further discussion and review.

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

Ok! This looks great @xmnlab! Did you want to try to tackle Mac and or Windows here in staged-recipes or instead should we try to add them in the feedstock?

@isuruf
Copy link
Member

isuruf commented Jul 6, 2018

Let's keep the discussion at conda-forge/conda-forge.github.io#63 and not merge until it is resolved

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

Let's keep the discussion at conda-forge/conda-forge.github.io#63 and not merge until it is resolved

Hmmm OK. The current discussion there doesn't seem super relate to the contents of this PR, which really just mirrors what is on defaults

@xmnlab
Copy link
Member Author

xmnlab commented Jul 6, 2018

@scopatz so would be better working on here to support osx and win until conda-forge/conda-forge.github.io#63 is resolved?

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

Yes @xmnlab - I think it is better to work here for the time being.

@xmnlab
Copy link
Member Author

xmnlab commented Jul 6, 2018

basically there are 2 runfiles for windows:

how to treat this case?

@scopatz
Copy link
Member

scopatz commented Jul 6, 2018

@xmnlab
Copy link
Member Author

xmnlab commented Jul 6, 2018

thanks @scopatz !

@scopatz
Copy link
Member

scopatz commented Aug 7, 2018

Ahh OK, that is pretty easy to solve, I think you need to add the following dependncies:

build:
  - {{ compiler('cxx') }}   # [osx]
host:
  - ncurses  # [osx]
run:
  - ncurses  # [osx]

@scopatz
Copy link
Member

scopatz commented Aug 7, 2018

made a slight edit to add run dep.

@xmnlab
Copy link
Member Author

xmnlab commented Aug 7, 2018

great! thanks @scopatz ! I am going to add that!

@xmnlab
Copy link
Member Author

xmnlab commented Aug 8, 2018

[win] for some reason there is no "NVIDIA GPU Computing Toolkit" folder [1] .. I will mix installation and extraction approach.

[1] https://ci.appveyor.com/project/conda-forge/staged-recipes/build/1.0.26980#L755

@xmnlab
Copy link
Member Author

xmnlab commented Aug 9, 2018

@scopatz it seems it worked for windows now. maybe we could improve that in the future .. but at least it is working now.

@xmnlab
Copy link
Member Author

xmnlab commented Aug 9, 2018

@scopatz I changed the tests for osx to check the files there.

@karl616
Copy link
Contributor

karl616 commented Aug 10, 2018

Could it have something to do with DYLD_LIBRARY_PATH? Wild guess and I found this in the documentation:

Set up the required environment variables:

export PATH=/Developer/NVIDIA/CUDA-9.2/bin
${PATH:+:${PATH}}
export DYLD_LIBRARY_PATH=/Developer/NVIDIA/CUDA-9.2/lib\
                         ${DYLD_LIBRARY_PATH:+:${DYLD_LIBRARY_PATH}

It is a wild guess, the path above is probably wrong... and I thought conda normally handled things like this on it's own.

Perhaps add an `echo ${DYLD_LIBRARY_PATH} to the build script... Hope someone with more osx experience know better...

@xmnlab
Copy link
Member Author

xmnlab commented Aug 10, 2018

thanks @karl616 for the feedback!

we probably should avoid to change DYLD_LIBRARY_PATH to avoid conflicts with conda. but it seems to be a good point and we should have this in mind. Thanks so much!

@scopatz
Copy link
Member

scopatz commented Aug 10, 2018

This is now passing on all platforms - and is ready for review by @conda-forge/staged-recipes!

@jjhelmus
Copy link
Contributor

jjhelmus commented Aug 21, 2018

This looks to be including both the dynamic and static libraries (at least on linux) which results in a really large package, ~770 MB vs 350 MB for just the dynamic libraries. Is there a reason for including the static libraries? Would it be reasonable to split these into their own package, cudatoolkit-static?

@scopatz
Copy link
Member

scopatz commented Aug 22, 2018

This is a great suggestion @jjhelmus! This package should include only the dynamic libraries.

@hadim
Copy link
Member

hadim commented Nov 4, 2018

Any chance to get this merged?

@hadim
Copy link
Member

hadim commented Nov 4, 2018

Ok, I just realized there is some licensing issue at conda-forge/conda-forge.github.io#63.

@leofang
Copy link
Member

leofang commented Oct 30, 2019

just curious, is this PR superseded by something else or abandoned?

@andersy005
Copy link
Member

@leofang

just curious, is this PR superseded by something else or abandoned?

https://github.com/conda-forge/cudatoolkit-dev-feedstock

@xmnlab
Copy link
Member Author

xmnlab commented Oct 30, 2019

@scopatz should we keep it opened? or should we close it?

@scopatz
Copy link
Member

scopatz commented Oct 30, 2019

It would be nice to have this still, so we don't rely on defaults, but we need permission from Nvidia.

@stale
Copy link

stale bot commented Mar 28, 2020

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on master so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

@stale stale bot added the stale will be closed in 30 days label Mar 28, 2020
@stale
Copy link

stale bot commented Apr 27, 2020

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!

@stale stale bot closed this Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale will be closed in 30 days
Development

Successfully merging this pull request may close these issues.