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: Do not merge: A potential recipe for handing VC versions. #10

Closed
wants to merge 1 commit into from
Closed

WIP: Do not merge: A potential recipe for handing VC versions. #10

wants to merge 1 commit into from

Conversation

pelson
Copy link
Member

@pelson pelson commented May 17, 2016

So, here are some improvements to the way we manage the vc problem for Windows.

Ping @msarahan as there are some important implications here.

Things to note / answer:

  • - The build component could be handled by conda-build automatically based on the current vc version being used, or alternatively by having yet another special environment variable.
  • - conda-build-all knows about vc and expands the build matrix to use it and expose as a VC_VERSION environment variable
  • - the only reason I'm using features here at all is to give the distributions unique filenames. I want extensibility of the build-string so that I don't have to re-invent the wheel (@msarahan - you may have missed this subtlety in https://github.com/conda/conda-build/pull/747/files#diff-c86934c27c2f25579540ddb447fa45bdR584 with all of the other noise). As soon as I have that, features have no place in this recipe.

@conda-forge-linter
Copy link

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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe (might be a conda-smithy bug) 😢


requirements:
build:
- gmp [not win]
- vc # [win]
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this should also be pinned.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure actually - the vc package doesn't contain anything, and I'm not sure it even influences any package resolution. Maybe it does need to be pinned though in order to control the dependencies being installed.

@msarahan
Copy link
Member

This is a very interesting proposal. I like it. I think it could get shorter over time, but this is a very good start. I kept asking myself "but where's Python?" - and of course, it doesn't need to be there. Thanks for pushing this forward.

- python [win]
- gmp [not win]
- vc {{ VC_VERSION }} # [win]
- gmp # [not win]
Copy link
Member

Choose a reason for hiding this comment

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

You can use the mpir package (https://github.com/conda-forge/mpir-feedstock) for windows build. MPIR is a fork of GMP and can be built on windows. API is the same except for some long vs long long differences.

Copy link
Member

Choose a reason for hiding this comment

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

Probably won't worry about it in this PR as this is aimed to try out the vc package with a matrix, but it is definitely something worth keeping in mind.

Copy link
Member

Choose a reason for hiding this comment

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

Put this idea in an issue ( #11 ) so we can better track and address it.

@pelson
Copy link
Member Author

pelson commented Aug 4, 2016

xref: conda-forge/tk-feedstock#2

@jakirkham
Copy link
Member

Another solution to this problem is in PR ( conda-forge/tk-feedstock#20 ). The same thing could easily be ported here.

@isuruf
Copy link
Member

isuruf commented Jun 2, 2018

Obsolete now that conda-build=3 can be used.

@isuruf isuruf closed this Jun 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants