-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Moar Psycopg2 #1162
Moar Psycopg2 #1162
Conversation
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 recipes/libpq:
|
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 ( |
This appears to be blocked on conda-build. I am resolving the issue, and will tag a new release soon. |
All good here (sans win) when the new conda-build is released. |
This reverts commit 88b4a2a.
Conda-build 1.21.8 tagged. Available on Conda-Forge as of #1168. Let's see what happens... |
Bad news with 1.21.8 creating a logger at https://github.com/conda/conda-build/blob/master/conda_build/__init__.py#L26 - we have some major issues with excessive output from conda, and I need to figure out better ways to have the logger for conda-build, but not interfere with logging decisions over in conda-land. |
Should be all green here @ocefpaf, since circleCI was the only holdup after 1.21.8 - please take one more look and merge at your convenience. |
Awesome @msarahan! Thanks!! I feel bad for all the trouble you went to finish this one. |
build: | ||
number: 0 | ||
skip: True # [win] | ||
# features: |
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.
If you are going to try Windows right after feedstocking we can leave these here. No need to remove them now.
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.
Yes, that was my hope. I don't know how quickly I'll get to it - still fighting postgresql build on win.
This seems to still not work for me with the same error as the original: Creating a new environment:
Trying to connect with
|
Yep, sorry, this appears to be disabled upstream in the postgresql feedstock: https://github.com/conda-forge/postgresql-feedstock/blob/master/recipe/build.sh#L6-L7 We'll look into it. |
Can you explain that a bit more? He doesn't appear to have installed the conda-forge postgresql? If I have a system postgresql with proper ssl support (like the Postgres.App), should psycopg2 work? |
The conda-forge psycopg2 depends on libpq - which is a rebundle of our postgresql build's libpq and pg-config. If you manually delete those, and your system postgresql gets found, then it might work. |
Right. Is there any discussion of that code comment above? I might be able to look in to this sometime this week. Might it be worth not listing libpq as a dependency and letting users take care of this? Or better yet to register an external dependency? Something like |
Right now, listing libpq is the best we can do. We need to cater to the lowest common denominator. Something like update-alternatives would be great, but that's a conda development project. Many people internally are interested in such a thing (also discussed as the concept of "provides"), but I don't know where it is on the road map. |
@jiffyclub, this should be fully functional as of conda-forge/libpq-feedstock#1 |
Thank you @msarahan, confirmed this on my end! |
Excellent. Glad it works. I'm also working on defaults, but compilers are not being agreeable. It'll also be up when I can get the kinks worked out. |
for LIBRARY in `find ${SP_DIR}/${PKG_NAME} -name "*.so"`; | ||
do | ||
install_name_tool -change libssl.1.0.0.dylib @rpath/libssl.1.0.0.dylib $LIBRARY | ||
install_name_tool -change libcrypto.1.0.0.dylib @rpath/libcrypto.1.0.0.dylib $LIBRARY |
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.
If Continuum would build openssl with conda-build this wouldn't be needed (c.f. ContinuumIO/anaconda-issues#498).
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.
I'm assuming these files are part of a different package (otherwise conda build would just do this itself after the build is complete). Assuming that is correct, shouldn't you break the hard links here? It doesn't matter for conda-forge, but if someone tries to build this recipe on their own machine, this will write into the package cache. Sorry if I'm missing something here.
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.
It doesn't matter for conda-forge, but if someone tries to build this recipe on their own machine, this will write into the package cache.
👍 Absolutely correct. Should be a simple copy shuffle to break the hard links.
Could you please raise the issue on the feedstock though as this is already merged or better yet propose the change?
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.
Are they actually from a separate package? Based on the loop, it looks like they are part of this package, but if that's the case, why are these lines needed?
Continuation of #291 - this adds the "libpq" package which is intended to be a repackaging of a minimal subset of postgresql - whatever psycopg2 needs at runtime.