libsecret: add recipe #4858
libsecret: add recipe #4858
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 ( |
nehaljwani
force-pushed
the
libpthread
branch
from
38e12fe
to
74d4d31
Compare
recipes/libsecret/build.sh
Outdated
| #!/bin/bash | ||
|
|
||
| # Nasty hack for $PREFIX/bin/intltool-* | ||
| ln -s "$PREFIX/bin/perl" "$PREFIX/bin/perl -w" |
There was a problem hiding this comment.
Wow, yeah that's gross. I guess it's okay if necessary since it's only happening in the build env, though.
There was a problem hiding this comment.
Couldn't think of a shorter way, other than patching intltool itself.
recipes/libsecret/meta.yaml
Outdated
| - pkg-config | ||
| - sed # [osx] | ||
| - perl 5.22.2.1 | ||
| - glib 2.55.* |
There was a problem hiding this comment.
conda-forge currently uses glib 2.53.
| --disable-manpages \ | ||
| --with-libgcrypt-prefix="$PREFIX" | ||
|
|
||
| # Requires python-dbus and some other stuff |
There was a problem hiding this comment.
How much "other stuff"? It'd be useful to have at least some basic check that things are working, either here or in the tests.
There was a problem hiding this comment.
Added a patch to disable some tests.
|
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/libsecret:
|
|
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 ( |
|
Ping 😅 |
|
Toggling for CI. |
recipes/libsecret/meta.yaml
Outdated
| # Uncomment when conda build >= 3 | ||
| # run_exports: | ||
| # https://abi-laboratory.pro/tracker/timeline/libsecret/ | ||
| # - libsecret >=0.18.5,<0.19 |
There was a problem hiding this comment.
Raising for pinning in PR ( conda-forge/conda-forge-pinning-feedstock#88 ). Please feel free to add more thoughts there.
|
This probably needs some cleanup given how we now handle pinnings and use |
|
Feedstock updated. |
recipes/libsecret/meta.yaml
Outdated
| - python 2.7 | ||
| host: | ||
| - glib | ||
| - libgcrypt |
There was a problem hiding this comment.
These don't have run_exports yet. Could we have a run requirement with them listed?
There was a problem hiding this comment.
Aren't we relying on the pin_run_as_build thingy?
and
libgcrypt has run_exports https://github.com/conda-forge/libgcrypt-feedstock/blob/master/recipe/meta.yaml#L16-L17
There was a problem hiding this comment.
If you're using pin_run_as_build, I think you still need it listed in run; example.
| skip: True # [win] | ||
| run_exports: | ||
| # https://abi-laboratory.pro/tracker/timeline/libsecret/ | ||
| - {{ pin_subpackage('libsecret', max_pin='x.x') }} |
There was a problem hiding this comment.
For now, can we address in issue ( conda-forge/conda-forge-pinning-feedstock#88 ).
There was a problem hiding this comment.
These lines should now be dropped.
There was a problem hiding this comment.
I am confused now. Why should this line be removed?
There was a problem hiding this comment.
Because it will be added to global pinnings with PR ( conda-forge/conda-forge-pinning-feedstock#89 ).
There was a problem hiding this comment.
@jakirkham: is it not generally preferable to use run_exports anyway though? conda-forge/conda-forge-pinning-feedstock#89 will encourage installation of libsecret 0.18 but not actually enforce the versioning as a dependency in the way that run_exports will. And I thought the goal was to eventually move to run_exports in general rather than the centralized pin_run_as_build (which is more error-prone, e.g.).
There was a problem hiding this comment.
I was confused because I saw the following line in conda-forge-pinning-feedstock:
# TODO: remove these when run_exports are added to the packages.
That being said, (I didn't know about this a week before now) I do like the idea of having pin_run_as_build so that one doesn't have to rely on the recipe maintainer, but have the pinnings at a central place.
However, there is one downside to it. What if I really want a version > pin_at_central_location and still want the auto-run-dep inject functionality, and the api compatibility might have changed for a future version of the same package? For example, if they say screw semver, let's break things at patch releases now, for a future version?
There was a problem hiding this comment.
I think it is a lot easier to stick to the one most common strategy until we are ready to change. Mixing and matching them will make things confusing and harder to maintain. Right now that is the global pinning file. In the future that may change and we can update this accordingly.
There was a problem hiding this comment.
Whatever happens with conda-forge-pinning, we should not remove the run_exports line. We have run_exports in most of the packages in conda-forge-pinning. Why do you want it dropped?
There was a problem hiding this comment.
Let's centralize this conversation: conda-forge/conda-forge-pinning-feedstock#102
recipes/libsecret/meta.yaml
Outdated
| - pkg-config | ||
| - sed # [osx] | ||
| # intltool hard pins perl, can't rely on cbc.yaml here | ||
| - perl 5.22.2.1 |
There was a problem hiding this comment.
Can't we just say perl here, and have intltool's requirement restrict it to this version? Otherwise when/if intltool changes the pin then this recipe will break, right?
There was a problem hiding this comment.
This is what happens when I remove the hard pin:
conda.exceptions.UnsatisfiableError: The following specifications were found to be in conflict:
- intltool -> perl-xml-parser -> perl==5.22.2.1
- perl=5.26.0
I don't know where is this pin coming from. It's probably from within CB3.
There was a problem hiding this comment.
Think that is in bioconda. @bgruening, is there some way to relax the perl version constraint of perl-xml-parser?
Side note: Don't think we should block the PR on account of this.
There was a problem hiding this comment.
There was a problem hiding this comment.
Hmm...must have missed that. Maybe we need some additional maintainers over there?
|
Think this is nearly ready. Just a couple things left. |
|
|
|
conda-forge/intltool-feedstock#2 also removed the need for the disgusting |
|
Okay, merged! Sorry for the delay on this one @nehaljwani. :) |
No description provided.