Add vc meta-package to track VC-features #363

183amir · 2016-04-14T20:44:20Z

Can incompatible vc=9 python=3.5 environments be created? Yes, but there is nothing that we can do about it with this recipe. A proper approach would be to make python depend on vc.

conda create -n t vc=9 python=3.5 -c file://C:/Users/Amir/Miniconda2/conda-bld --show-channel-urls
Fetching package metadata: ......
Solving package specifications: .........

Package plan for installation in environment C:\Users\Amir\Miniconda2\envs\t:

The following NEW packages will be INSTALLED:

    pip:            8.1.1-py35_1    defaults
    python:         3.5.1-4         defaults
    setuptools:     20.7.0-py35_0   defaults
    vc:             9-0             file://C:/Users/Amir/Miniconda2/conda-bld/win-64/
    vs2008_runtime: 9.00.30729.1-0  defaults
    vs2015_runtime: 14.00.23026.0-0 defaults
    wheel:          0.29.0-py35_0   defaults

Would be nice to add packages for runtimes (maybe using CMake). That would be a different pull request.
Supporting other VC's not used with CPython (some packages already would benefit from this)? Yes, multiple branches can be created to accommodate several versions.

conda-forge-admin · 2016-04-14T20:44:22Z

Hi! This is the friendly conda-forge-admin automated user.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/vc-feature) and found some lint.

Here's what I've got...

For recipes/vc-feature:

The recipe must have some tests.

jakirkham · 2016-04-14T20:45:28Z

recipes/vc-feature/meta.yaml

+    - vc9     # [win and py27]
+    - vc10    # [win and py34]
+    - vc14    # [win and py35]
+  skip: true  # [unix]


To make this work, I think we need to pull in Python as a build requirement at least.

Why is that?

Because we need a dependency during build time to activate the features. That is why we have used Python in the past. The hope though is that once we do that one hack here. We will never need to do it anywhere else again.

jakirkham · 2016-04-14T20:49:09Z

cc @msarahan

jakirkham · 2016-04-14T20:50:31Z

recipes/vc-feature/meta.yaml

+  track_features:
+    - vc9     # [win and py27]
+    - vc10    # [win and py34]
+    - vc14    # [win and py35]


See these are tied to Python version so that is how we are able to get this to work.

Wouldn't something like vs2008_runtime be better than Python?

I think we still need it initially because the features are selecting on Python, but that is probably a good idea to add to the run section. Do you have thoughts on this, @patricksnape?

Yeah its tough. On the one hand you want to make sure they are kicked off correctly on the Python version, but you also want the flexibility of just selecting the right compiler depending on whatever features are activated. I think this is kind of cycling back around to all of the discussions about how best to sort out features/different build matrices. For now its probably easiest to ensure that Python is there - then when we sort everything out we can pull Python and add in the proper matrices. Although the runtimes may also be a better idea but @msarahan probably knows better than me if that is a good idea.

conda-forge-admin · 2016-04-14T21:05:45Z

Hi! This is the friendly conda-forge-admin automated user.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/vc) and found some lint.

Here's what I've got...

For recipes/vc:

The recipe must have some tests.

jakirkham · 2016-04-14T21:06:11Z

recipes/vc/meta.yaml

+  run:
+    - vs2008_runtime  # [win and py27]
+    - vs2010_runtime  # [win and py34]
+    - vs2015_runtime  # [win and py35]


Do you think this won't work, @patricksnape? Personally, I think @183amir is on to something here.

Looks like a solid idea to me!

Right, I don't know that anyone has tried this, but it seems like this will give us what we want. Honestly, we might be able to lose the track features entirely.

Usually you want to depend on a meta package that tracks one feature. But this one is kinda a meta-package that tracks 3 features. I think we also can add 3 meta packages each for a VC feature. But it also works like this with the versioning hack we did.

If we did not have these [win and py27] selectors, we could not do this. So I think the best way would be to go the hard way and create 3 meta packages. Then we can also have vc11, vc12 and any other version if needed in future.

Hmm...definitely worth discussing how we go from here. Though we don't need to cover all cases in the first pass.

jakirkham · 2016-04-14T21:17:09Z

@pelson, your dreams of a versioned vc package may come true. 💭

jakirkham · 2016-04-14T21:25:09Z

This basically looks fine to me. I'd like to get some other people's feedback on it before we merge it. Definitely putting this on the docket for tomorrow's meeting ( conda-forge/conda-forge.github.io#89 (comment) ). It might not take that long to discuss, but it deserves visibility. Everyone is welcome to attend BTW. Here is the info for joining ( conda-forge/conda-forge.github.io#89 (comment) ).

msarahan · 2016-04-14T21:29:08Z

Be extremely careful adding this to any build or run requirement. If two vc features are ever activated at once, the solver will get very confused and you'll end up with incredibly mysterious unresolved dependency errors.

Think of the features this way: they are a way of ensuring that a single process always uses one type of some thing (runtimes, blas, etc.). Having Python track features makes a lot of sense in conda - you can only have one Python in a given environment. However, you can (and I would say, logically should) be able to install different programs that use different sets of features, but each of these features must be internally consistent. I'm not sure we can do that with features - certainly not right now, and maybe not ever. The alternative is to allow installations of different builds of a single package, and provide a more intelligent way of figuring out which one to load in a process. This is similar to the idea that WinSxS tries to fix.

jakirkham · 2016-04-14T21:34:09Z

I was debating eliminating the features entirely. Do you have any thoughts on that, @msarahan?

msarahan · 2016-04-14T21:44:07Z

If you remove the features entirely here, then I don't see the value of this recipe at all. There are already vs20??_runtime packages, and you could depend on this directly. The point of this package is to activate a feature, so that the correct build that you just built is actually resolvable.

If you have another scheme to line up runtimes so that (for example) we have 3 different stacks of software for something like, say, GDAL, and each of those stacks are all built with the same version of MSVC that the Python used to run GDAL is built with, then I'd love to hear it. I don't currently know a good way to keep all those ducks in a row. It's possible that something like conda/conda-build#848 might allow this.

jakirkham · 2016-04-14T21:47:35Z

Then could you please explain what is wrong with how we have done features then? It is not clear to me that this conflicts with Python at all. AFAICT this follows Python's lead.

msarahan · 2016-04-14T21:55:25Z

The solver evaluates features on a per-environment basis, not a per-process basis. If two different processes ever activate two or more different features that conflict, the solver will barf. Having Python be the only thing in the system activating features makes it pretty simple to avoid conflicts. This metapackage is a conceptually more correct approach, but you need to be extremely careful about avoiding multiple vc features active at once. This is different from installing multiple vs20??_runtime packages - that's totally allowed, but it doesn't play into selecting particular builds of a package.

pelson · 2016-04-15T08:13:09Z

recipes/vc/meta.yaml

+    - vc9      # [win and py27]
+    - vc10     # [win and py34]
+    - vc14     # [win and py35]
+  skip: true   # [unix]


Let's call it not win.

Was thinking about that.

jakirkham · 2016-04-15T15:24:38Z

So, we have discussed this and ( #365 ) a little bit in our meeting today. Unfortunately, we haven't come to any conclusion about what to do here. However, we are not giving up on this issue and still would like to figure out some improvement here.

We are planning on having a meeting next Friday at 2 PM UTC simply to discuss VC features on Windows. We would really appreciate if everyone here could make it. We really want to figure out a good path forward. Please let us know if this time works. We can try to put together a few thoughts on hackpad before the meeting and add to it as we discuss (if that is agreeable to everyone).

patricksnape · 2016-04-15T15:29:54Z

@jakirkham 1400 UTC next Friday works for me - I put it in the Calendar. Sorry I couldn't make it today - slammed with work.

jakirkham · 2016-04-15T15:31:10Z

No worries. Glad to hear that will work.

pelson · 2016-04-22T14:48:09Z

Is it possible with this package to:

conda install vc=9 python=35

or do features prevent me from doing this?

msarahan · 2016-04-22T14:52:04Z

recipes/vc/meta.yaml

+
+requirements:
+  build:
+    - python


Python should not necessary here. Have you tried without it?

msarahan · 2016-04-22T14:57:40Z

As discussed in our meeting today, I think this is a very promising solution.

Please consider removing Python as a build requirement.

Add me as a maintainer if you want.

Otherwise, let's merge, and I'll come up with an example recipe based on @ukoethe's system at https://github.com/ilastik/ilastik-build-conda/tree/toolset-config (described well at conda/conda#1959)

I should note that we need not wait on that example to start using this recipe instead of @patricksnape's jinja2 python solution. Whatever I come up with might require changes to conda build, and might be a longer-term (but nicer) fix.

mcg1969 · 2016-05-04T20:13:47Z

Certainly, finding a way to do this with our new variant metapackages is the right way to go, in my view.

As I shared in our BLAS discussion, I'm concerned that we don't prevent different platforms from using different C runtimes; for example, Python and R could be linked to different runtimes. MSYS2 is certainly taking some of the pressure off but it would be great if we can come up with a conda-only solution. This means that we use a different set of variant metapackages for each platform.

Of course, if you want to just start with Python and get that right, it's OK. We can create a parallel set of variant metapackages for each platform once we nail down the right way to do it for Python.

jakirkham · 2016-05-04T20:33:31Z

Certainly, finding a way to do this with our new variant metapackages is the right way to go, in my view.

So, you don't like the idea of versioning vc as we did here, but would prefer to see the version in the build string instead?

As I shared in our BLAS discussion, I'm concerned that we don't prevent different platforms from using different C runtimes...

In this case, I'm more sympathetic to that desire. Though I don't know that we prevent this in the current model per se. We would have to be very clear about how this is used in a recipe.

183amir · 2016-05-04T20:43:05Z

I think we can also create packages for different environments like vc_python and vc_r so that you can mix Python and R with different runtimes.
Since conda packages are named mostly for python so I think having vc_python as vc would be ok.

jakirkham · 2016-05-04T20:46:45Z

I'm concerned that we don't prevent different platforms from using different C runtimes; for example, Python and R could be linked to different runtimes.
...we can also create packages for different environments like vc_python and vc_r...

We can, but I don't think we need to. I don't see this package interfering in the situation of having multiple runtimes. All it does is help us configure a build. After that it gets out of the way.

183amir · 2016-05-04T20:48:34Z

@jakirkham I thought the idea was to add this in requirements: build and run to make sure that runtimes don't mix.

mcg1969 · 2016-05-04T20:49:11Z

So, you don't like the idea of versioning vc as we did here, but would prefer to see the version in the build string instead?

Keep in mind that Microsoft runtimes live happily side by side in your filesystem. So they should be different packages with different names. That's really not the issue here---the issue is how you make sure that the Python platform picks just one of them.

So really, this isn't any different than BLAS, it's just that the names are more similar. Instead of openblas, atlas, and mkl, we have vc9, vc10, and vc14. They still need some sort of mutual exclusion soluton.

mcg1969 · 2016-05-04T20:49:52Z

And yes, I genuinely believe that our variant metapackage approach is superior to features when the choice is not binary.

mcg1969 · 2016-05-04T20:59:26Z

We can, but I don't think we need to. I don't see this package interfering in the situation of having multiple runtimes. All it does is help us configure a build. After that it gets out of the way.

But are you sure you want it to get out of the way? Don't you want to make sure that packages linked to different runtimes don't live in the same process?

jakirkham · 2016-05-04T21:01:53Z

I thought the idea was to add this in requirements: build and run to make sure that runtimes don't mix.

Initially, it was. Though I guess my viewpoint on how this should work has changed after seeing various use cases where one does need multiple runtimes. For instance, maybe we need xz to decompress a file, but we are not linking to it. IIRC xz requires vc14. We don't want that restriction when trying to build with vc9. So, maybe we have this as a build and test dependency instead.

What I'm still unclear on is if this is enough to force the build environment to use this runtime to build with. I feel like there is something from the conda-build perspective that I am missing.

mcg1969 · 2016-05-04T21:05:11Z

For instance, maybe we need xz to decompress a file, but we are not linking to it.

This is why you have to:

Distinguish between the runtime package itself and the variant metapackage that controls it; and
Distinguish between different platforms (Python, R, standalone).

Python, and any package that links to CPython, needs to use a variant metapackage to make sure that they all link to the same runtime. xz, as a standalone program, just loads the desired runtime directly.

jakirkham · 2016-05-04T21:12:17Z

Correct me if I'm wrong, but my understand of what you said @mcg1969 is we should have vc_python as another layer on top of this that ensures that CPython and anything that links against is built against the same runtime. Yeah, that seems fine. That would be a different package though.

mcg1969 · 2016-05-05T14:07:36Z

That's what I'm saying indeed. I'm not sure we want to call it vc_python, simply because we may want to be able to consider other toolchains or runtimes that are not vc. But I don't feel strongly about that.

I know these "extra layers" may appear inconvenient but they're only modestly more work than features themselves. And if the key/value PR (conda/conda#2427) shakes out the way it's going, we can use syntactic sugar to handle the variant metapackage and the package version in a single step.

pelson · 2016-05-06T21:33:42Z

If you were to revert the recipe to 3626fe3 and we had an answer to the critical question:

Is it possible with this package to:
conda install vc=9 python=35

I would be happy to merge this.

There is a very real danger of bikeshedding here. This is a simple recipe which adds a critical piece for us to develop from in conda-forge. It is certainly not sacrosanct and it is completely fine for this to be iterated on in the future (please add me as a maintainer given its significance to conda-forge's integrity).

The python 2 && vc != 9 case will not be solved in this PR - we are aware of it, and have no intention on closing the door on it, but it is simply lower on the priority list than having a simple python recipe on conda-forge is.

183amir · 2016-05-06T22:31:10Z

@pelson, I think we need to wait on this recipe a little more since @mcg1969 and @jakirkham are trying something with build strings that could be more practical than this.

mcg1969 · 2016-05-06T22:35:22Z

:-) Heh heh. Oh, he knows. #525 (comment)

pelson · 2016-05-06T22:39:48Z

This PR is holding up Python, which itself is holding up other things in conda-forge. We shouldn't be waiting on a feature which hasn't yet been tried and tested in the wild (though this isn't the PR to discuss such details - #525 is the place for that discussion).

The PR proposed here is pragmatic, and less troublesome than using features (though I completely agree with #525 in that we may even be able to improve on this PR in the near future).

This reverts commit 6ef9d34.

183amir · 2016-05-06T22:46:33Z

I made the changes @pelson, you may merge this pull.

pelson · 2016-05-06T22:47:42Z

you may merge this pull.

Add vc meta-package to track VC-features #363 (comment)

183amir · 2016-05-06T22:48:43Z

Yes it is possible. I had that in the first comment. Tested with the new recipe:

conda create -n test1 --use-local vc=9 python=3.5
Fetching package metadata: ......
Solving package specifications: .........

Package plan for installation in environment C:\Users\Amir\Miniconda2\envs\test1:

The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    vc-9                       |                0          823 B

The following NEW packages will be INSTALLED:

    pip:            8.1.1-py35_1
    python:         3.5.1-4
    setuptools:     20.7.0-py35_0
    vc:             9-0
    vs2008_runtime: 9.00.30729.1-0
    vs2015_runtime: 14.00.23026.0-0
    wheel:          0.29.0-py35_0

Proceed ([y]/n)?

Fetching packages ...
vc-9-0.tar.bz2 100% |###############################| Time: 0:00:00   0.00  B/s
Extracting packages ...
[      COMPLETE      ]|##################################################| 100%
Linking packages ...
[      COMPLETE      ]|##################################################| 100%
#
# To activate this environment, use:
# > activate test1
#

pelson · 2016-05-08T07:01:21Z

Yes it is possible. I had that in the first comment. Tested with the new recipe:

Thanks @183amir. I'm going to keep a close eye on that, as I have some concerns about whether it is possible to accidentally mix conda-forge packages with defaults packages in a breaking way (as in, pulling in packages which were built with a different VC). It is easy enough to revoke the package if such concerns are founded though.

Thanks to everybody for the input on this PR, I'm glad we are merging at this stage, but would love to see us iterating on this solution to make it as easy as possible for packagers to use in the future.

jakirkham · 2016-05-08T07:04:42Z

I'm a little excited and a little terrified to see this merged. Though at some point doing something is better than the status quo (doing nothing). Let's be brave and see what happens. We will learn quite a bit no matter what happens. I hope we have a more stable and flexible ecosystem as a consequence.

bollwyvl · 2016-06-20T13:51:31Z

I haven't read this whole thread, but we did go forward adding a vs2015_runtime:
#824

Presumably, as python-feedstock was rerendered recently, these versions should match.

Of note, Visual Studio update 3 is incoming in the next month, perhaps: #824 (comment)

183amir added 2 commits April 14, 2016 22:41

Add vc-feature

e5e32c2

skip on unix

b78e910

jakirkham reviewed Apr 14, 2016
View reviewed changes

183amir mentioned this pull request Apr 14, 2016

Add LibSVM #358

Merged

jakirkham reviewed Apr 14, 2016
View reviewed changes

Depend on vsruntime

8a1a8c3

183amir changed the title ~~Add vc-feature meta-package to track VC-features~~ Add vc meta-package to track VC-features Apr 14, 2016

jakirkham reviewed Apr 14, 2016
View reviewed changes

jakirkham mentioned this pull request Apr 14, 2016

Agenda for 2016-04-15 conda-forge/conda-forge.github.io#89

Closed

183amir mentioned this pull request Apr 14, 2016

Add vc9,vc10,vc14 #365

Closed

pelson reviewed Apr 15, 2016
View reviewed changes

not win instead of unix

a563b47

msarahan reviewed Apr 22, 2016
View reviewed changes

recipes/vc/meta.yaml

requirements:

build:

- python

Copy link

Member

msarahan Apr 22, 2016

Choose a reason for hiding this comment

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

Python should not necessary here. Have you tried without it?

183amir added 2 commits May 7, 2016 00:43

Revert "Update meta.yaml"

8531779

This reverts commit 6ef9d34.

Add pelson.

1ed5ae5

pelson merged commit dbbfeb3 into conda-forge:master May 8, 2016

pelson removed the Needs Decision label May 8, 2016

jakirkham mentioned this pull request May 24, 2016

Windows Python 3.5 CI Failure #666

Closed

jakirkham mentioned this pull request Jun 17, 2016

add vs runtime packages #809

Merged

jakirkham mentioned this pull request Jan 3, 2018

Add vs2008_runtime #4728

Merged

183amir mentioned this pull request Oct 29, 2020

mamba resolves an environment while conda throws a conflict mamba-org/mamba#559

Closed

Add vc meta-package to track VC-features #363

Add vc meta-package to track VC-features #363

Conversation

183amir commented Apr 14, 2016 • edited Loading

conda-forge-admin commented Apr 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Apr 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conda-forge-admin commented Apr 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham Apr 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Apr 14, 2016

jakirkham commented Apr 14, 2016

msarahan commented Apr 14, 2016

jakirkham commented Apr 14, 2016

msarahan commented Apr 14, 2016

jakirkham commented Apr 14, 2016

msarahan commented Apr 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Apr 15, 2016 • edited Loading

patricksnape commented Apr 15, 2016

jakirkham commented Apr 15, 2016

pelson commented Apr 22, 2016

Choose a reason for hiding this comment

msarahan commented Apr 22, 2016

mcg1969 commented May 4, 2016

jakirkham commented May 4, 2016 • edited Loading

183amir commented May 4, 2016

jakirkham commented May 4, 2016

183amir commented May 4, 2016

mcg1969 commented May 4, 2016

mcg1969 commented May 4, 2016

mcg1969 commented May 4, 2016

jakirkham commented May 4, 2016

mcg1969 commented May 4, 2016 • edited Loading

jakirkham commented May 4, 2016

mcg1969 commented May 5, 2016

pelson commented May 6, 2016 • edited Loading

183amir commented May 6, 2016

mcg1969 commented May 6, 2016

pelson commented May 6, 2016

183amir commented May 6, 2016

pelson commented May 6, 2016 • edited by 183amir Loading

183amir commented May 6, 2016 • edited Loading

pelson commented May 8, 2016

jakirkham commented May 8, 2016

bollwyvl commented Jun 20, 2016

183amir commented Apr 14, 2016 •

edited

Loading

jakirkham Apr 14, 2016 •

edited

Loading

jakirkham commented Apr 15, 2016 •

edited

Loading

jakirkham commented May 4, 2016 •

edited

Loading

mcg1969 commented May 4, 2016 •

edited

Loading

pelson commented May 6, 2016 •

edited

Loading

pelson commented May 6, 2016 •

edited by 183amir

Loading

183amir commented May 6, 2016 •

edited

Loading