-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Build both static and shared libraries on Unix and Windows platforms #15
Conversation
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 ( |
@conda-forge-admin, please rerender |
Hi! This is the friendly automated conda-forge-webservice. I tried to re-render for you, but it looks like there was nothing to do. |
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 ( |
Thanks for putting this in @wesm! Any interest in adding yourself as a maintainer here? I think you probably care about this package more than some of the current maintainers 😉 |
Does this close #10? Windows is still disabled |
No this doesn't seem to enable #10 |
Oh let me try to enable it then |
To be clear, you'll need to remove the win selector here and then rerender |
@conda-forge-admin, please rerender |
Looks like the windows builds fail with:
|
OK, so Python 3.5 or later I guess. I'll have to look into it later, if someone wants to push changes here please go ahead |
Note, that we are about to drop explicit support for python v3.5 if that helps! |
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 ( Here's what I've got... For recipe:
|
@conda-forge-admin, please rerender |
@conda-forge-admin, please rerender |
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 ( |
@scopatz is this the right windows skip clause?
|
@wesm - what are you trying to skip? That looks like it is going to skip all versions of windows, effectively |
I tried to follow this C library pattern: https://github.com/conda-forge/brotli-feedstock/blob/master/recipe/meta.yaml#L14 I guess that's not right. I'm copying the recipe from arrow-cpp-feedstock now (another C++11-only library) |
@conda-forge-admin, please rerender |
What you have now looks more correct. I am not a deep windows build expert, so you might want to ask on gitter, but if this works and passes, I'll merge it |
👍 |
Looks like the windows build failed 😢
|
Hm. Guess we should try the normal MSVC build |
Appears to be building now. What's this all about?
|
That seems odd, perhaps a conda-build issue. Maybe bring it up on the gitter or conda-build |
recipe/bld.bat
Outdated
nmake | ||
if errorlevel 1 exit 1 | ||
|
||
copy Release\re2.lib %LIBRARY_DIR%\re2_static.lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LIBRARY_DIR -> LIBRARY_LIB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Ideas? |
|
||
nmake | ||
if errorlevel 1 exit 1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not doing an nmake install here, not sure if that is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't do that because it would clobber the DLL import lib re2.lib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh Ok, that makes sense. Is there a way we could turn echo on, so that we can see what is actually being run?
OK, adding echo shows what's wrong:
I will fix and then merge once it's working |
@scopatz can you merge this? Thanks! |
Glad to see this working! |
Checklist
conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)Close #10, #13