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 conda #656

Merged
merged 1 commit into from
Aug 8, 2016
Merged

Add conda #656

merged 1 commit into from
Aug 8, 2016

Conversation

jakirkham
Copy link
Member

Adds recipes that either I wrote or used conda skeleton pypi to generate and then modified. Having some issue getting the conda recipe to build. The rest seem to be doing ok. Maybe we can break out the working ones and merge them first.

@msarahan, I have added you and myself as maintainers on all of these. Please let me know if that is ok. I don't feel a strong need to be a maintainer here, but I feel my handle should be on them as I wrote them.

@pelson, seemed like you were interested in seeing this happen. This gets the process started at least. If the going gets slow, feel free to take the reigns.

@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/conda, recipes/conda-env, recipes/pycosat, recipes/requests) and found it was in an excellent condition.

@jakirkham jakirkham mentioned this pull request May 23, 2016
@jakirkham
Copy link
Member Author

Broke out the dependencies in this PR ( #657 ) so we can sort them out and get them in first.

url: https://github.com/conda/conda/archive/{{ version }}.tar.gz
sha256: 4ca7ec875de13519aeb3eebc9beca23adf3c41e8852c186deb503e8bc2282c40

build:
Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly, you also need to always_include the conda executable: http://conda.pydata.org/docs/building/meta-yaml.html#force-files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we are going to need to do that with a number of files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I'm wrong. We shouldn't need any as conda does not live in this environment. Though we may run into a nasty situation with the conda symlink. Care to comment on this point, @msarahan.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think bin/activate and bin/deactivate might need to be in the always_include also, but this might only be needed on certain platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

From the conda repository

  # These are present when the new environment is created
  # so we have to exempt them from the list of initial files
  # for conda-build to realize they should be included.
  always_include_files:
    - bin/conda                 [unix]
    - bin/activate              [unix]
    - bin/deactivate            [unix]
    - Scripts/activate.bat      [win]
    - Scripts/deactivate.bat    [win]

Copy link
Member

Choose a reason for hiding this comment

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

activate and deactivate both need to be in always_include (win needs them for bash support). Windows also needs the .bat files. There's a fish script, but I don't know where it goes. Issue here: conda/conda#2611

@pelson
Copy link
Member

pelson commented May 25, 2016

@jakirkham - indeed. Feel free to add me to all of these recipes.

@pelson
Copy link
Member

pelson commented May 25, 2016

I can also provide the working recipes I have once I am back at my desk.

@jakirkham jakirkham changed the title WIP: Add conda WIP: Add conda and conda-env May 25, 2016
@jakirkham
Copy link
Member Author

@mcg1969, conda-build and conda-env are being dealt with here.

@mcg1969
Copy link
Contributor

mcg1969 commented Jun 2, 2016

This is great!

build:
- python
- conda-env
- pycosat
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these are build deps - I don't seem to need these in my own recipe.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, this is required for the solver. I'm pretty sure it should be there. Care to comment on this, @mcg1969?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've never built a conda package for conda, frankly. These are certainly run dependencies but if it builds fine without them I see no reason to include them.

One thing about the conda-env dependency: please look at our dependencies carefully. It's important that conda-env and conda remain in rough version lock-step. For instance, conda 4.1.x requires conda-env >=2.5, and conda 4.0.x requires conda-env <2.5.

We also use ruamel_yaml for conda 4.1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not adding the conda-env version dependencies can cause catastrophic issues if someone tries to downgrade.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, we had this chat before. This hasn't been touch in awhile. Thanks for the version pointers.

@jakirkham
Copy link
Member Author

jakirkham commented Jun 30, 2016

pycosat and requests don't need to be here any more as it cleared. Will remove.

@pelson
Copy link
Member

pelson commented Jun 30, 2016

👍 in principle. I have a few review comments, but nothing show-stopping.

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


build:
number: 0
script: python setup.py install
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need an entry point?

  entry_points:
    - conda-env = conda_env.cli.main:main

The anaconda-recipes has it but not the repositories recipe

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. 😄 Good point. Will add.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, actually does someone want to clarify this discrepancy?

@jjhelmus
Copy link
Contributor

Sorry for the barrage @jakirkham, would love to get this into conda-forge.

@jakirkham jakirkham force-pushed the add_conda branch 2 times, most recently from e78b554 to b6773de Compare June 30, 2016 15:59
@msarahan
Copy link
Member

msarahan commented Jul 5, 2016

I'm proposing to just not die if the files aren't there. They need to be there at the end, and we will clobber them if they are there. Nothing else matters.

I don't think pinning this is breaking anything. I'm confused on how this works, anyway, since conda nominally won't install itself into non-root environments, but some of these scripts and entry points should actually only be present in environments with conda installed.

@jakirkham
Copy link
Member Author

Would commenting those Windows files in always_include_files be ok? It seems they don't appear in any build I've tried.

@msarahan
Copy link
Member

msarahan commented Jul 6, 2016

conda/conda-build#1101 implements the relaxation of always_include_files

@jakirkham
Copy link
Member Author

Restarting AppVeyor now that conda-build version 1.21.5 includes relaxed always_include_files. However, we may still have some issues due to version pinning on some builds. My suggestion would be if the bulk pass we resolve this with a workaround in the feedstock ATM. This is assuming there are no other issues worth addressing here at staged-recipes.

@jakirkham
Copy link
Member Author

jakirkham commented Jul 9, 2016

Just realized we are pinned to very old conda-build versions for all Windows builds. So, AppVeyor is still going to fail until we move them to a newer version.

@pelson pelson mentioned this pull request Jul 10, 2016
9 tasks
This was referenced Jul 16, 2016
@jakirkham
Copy link
Member Author

Broke out our work here on conda-env into this PR ( #1064 ). I think it is probably ready to go with a few more tweaks.

@jakirkham
Copy link
Member Author

jakirkham commented Jul 16, 2016

Is there any chance we could get conda version 4.1.7 and 4.1.8 on PyPI given the issues we have had packaging them from GitHub, @kalefranz?

@jakirkham jakirkham changed the title WIP: Add conda and conda-env WIP: Add conda Jul 16, 2016
@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/conda) and found it was in an excellent condition.

@jakirkham jakirkham force-pushed the add_conda branch 3 times, most recently from 2240d81 to 8e8132a Compare August 4, 2016 20:35
@jakirkham
Copy link
Member Author

So, I have fixed this up. It is now packaging the latest version of conda using a git tag from the repo. Also it uses the aforementioned hack to fix the version as proposed by @kalefranz.

Also it seems to be pulling in setuptools through one of its dependencies (probably ruamel_yaml). As a result, it had to be changed to use setuptools for the build.

During these changes, it was determined this now has a dependency on pyyaml. The dependency was added to the build and run requirements.

Some other changes were made like adding some more paths to the always include list.

@jakirkham
Copy link
Member Author

Thus far this seems to be passing on the CIs now (including AppVeyor).

However, it fails on Windows Python 3.4 64-bit. This is actually not due to any problem with this recipe. Instead this is a consequence of our conda-build pinning in that case. In other words, we need to remove that pinning as proposed in PR ( conda-forge/conda-forge-build-setup-feedstock#22 ) to get this recipe for conda out. Until that is done we are blocked.

@pelson pelson changed the title WIP: Add conda Add conda Aug 5, 2016
@jakirkham
Copy link
Member Author

Closing and reopening to restart CIs.

@jakirkham jakirkham closed this Aug 5, 2016
@jakirkham jakirkham reopened this Aug 5, 2016
@jakirkham
Copy link
Member Author

Now passing and ready to go. 😄

@pelson pelson merged commit 2111740 into conda-forge:master Aug 8, 2016
@jakirkham jakirkham deleted the add_conda branch August 8, 2016 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

7 participants