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

[WIP] Allow to specify channels in a recipe #3656

Closed
wants to merge 5 commits into from
Closed

[WIP] Allow to specify channels in a recipe #3656

wants to merge 5 commits into from

Conversation

mmathar
Copy link

@mmathar mmathar commented Aug 7, 2019

Adds support for channels to meta.yaml recipes as discussed in #532 . This is achieved by adding the channels specified in the meta.yaml file to those given by the -c argument.

Example usage:

package:
  name: test
  version: "0.1"

requirements:
  channels:
    - anaconda
    - cdat-forge
  host:
    - cdat-forge::pylint
    - python=3.6

This will (in this case) install pylint from cdat-forge and all other dependencies (including python) from anaconda. This is effectively the same as calling
conda build -c anaconda -c cdat-forge .
on the same meta.yaml minus the channels section.

I noticed some failures in unrelated (?) tests locally. I'll take another look at this when the tests for this PR complete.

@msarahan
Copy link
Contributor

msarahan commented Aug 7, 2019

Thanks @mmathar!

We have generally avoided this because it limits the generality of recipes. Someone may want to use their own pylint in your example, but your recipe will preclude it (without recipe modification, anyway). I'm kind of inclined to say that channels might be allowable with exact specs, but at the same time, that's really the wrong direction for recipes to head. Overspecifying things is painful and fragile. I'm not turning down this PR, but we should have a discussion with the other conda-build maintainers. Let's start here in the comments, but if we need higher bandwidth, we can schedule a call/video thing.

@mmathar
Copy link
Author

mmathar commented Aug 8, 2019

I'm not sure specifying a channels section in a recipe limits generality of recipes. As it is implemented in this PR any channel passed by -c will be preferred over the channels in the recipe (assuming no per-package channel specification). @msarahan

On an unrelated note, it occurred to me it may be more transparent to only use the recipe's channels if the user does not specify any. Otherwise, some user specified channels and some recipe channels might be used which might lead to confusion.

Someone may want to use their own pylint in your example, but your recipe will preclude it (without recipe modification, anyway).

This may be wrong but isn't this the case already? Adding cdat-forge::pylint to a recipe basically forces the user to call conda-build with -c cdat-forge or the build will fail. As I understand the problem seems to be the channel specification on a per-package basis (which is already a thing)?

@msarahan
Copy link
Contributor

msarahan commented Aug 8, 2019

As it is implemented in this PR any channel passed by -c will be preferred over the channels in the recipe (assuming no per-package channel specification)

You're sort of right. There's two things here: the channel list, which affects specs that have no channel (this is where you're right), and specs with channels as part of them (this is where you're wrong - channels in meta.yaml will not override the channel in the spec.)

As implemented, the recipe-specified channels are the lowest priority. That's going to be good in some ways and bad in others, especially when strict channel priority is in use. I think you probably need the ability to both prepend and append channels. Perhaps:

requirements:
  channels:
    prepend:
      - foo
    append:
      - bar

Adding cdat-forge::pylint to a recipe basically forces the user to call conda-build with -c cdat-forge or the build will fail

This is because you need some dependencies from cdat-forge. If you had dependencies that could otherwise be satisfied, cdat-forge::pylint would be enough to use that channel for that package (but no dependencies)

@mmathar
Copy link
Author

mmathar commented Aug 8, 2019

You're sort of right. There's two things here: the channel list, which affects specs that have no channel (this is where you're right), and specs with channels as part of them (this is where you're wrong - channels in meta.yaml will not override the channel in the spec.)

Just to make sure I understand this correctly: the channel list is passed by -c and specs with channels as part of them are e.g. channel::package? So the channels in the spec are meant to extend the channel list? I've thought of it more as the channel in the spec selecting a channel from the channel list. I'm a bit confused now.

I think you probably need the ability to both prepend and append channels.

Prepend and append to the channel list or do you mean something different?

This is because you need some dependencies from cdat-forge. If you had dependencies that could otherwise be satisfied, cdat-forge::pylint would be enough to use that channel for that package (but no dependencies)

I'm not sure I understand this correctly. An example for illustration:

requirements:
  build:
    - intel::intel-openmp

intel-openmp is the only requirement and according to conda search intel-openmp --info does not have any dependencies. Is it expected that conda build works? Because it does not. Only when -c intel is added the package is found.

@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this pull request to remain open please:

  1. Rebase and verify the changes still work
  2. Leave a comment with the current status

NOTE: If this pull request was closed prematurely, please leave a comment.

Thanks!

@github-actions github-actions bot added the stale [bot] marked as stale due to inactivity label Mar 16, 2022
@github-actions
Copy link

Hi again!

This pull request has been closed since it has not had recent activity.

NOTE: If this pull request was closed prematurely, please leave a comment.

Thanks!

@github-actions github-actions bot added the stale::closed [bot] closed after being marked as stale label Apr 15, 2022
@github-actions github-actions bot closed this Apr 15, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Apr 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity stale::closed [bot] closed after being marked as stale stale [bot] marked as stale due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants