conda 4.4 compatibility

[kalefranz]

The only change here for conda 4.4 is a small tweak in logging initialization. I just got my first issue reported that I believe to be a result of thrashing between conda 4.3 and 4.4: https://github.com/conda/conda/issues/6804. Conda 4.4 has been out for over a month now, and it's been pretty stable since the initial release (was in canary for months). Probably time to get the conda-forge upgrade pushed through for conda-forge users.

[kalefranz]

So the failing test here caused me to find a problem in conda 4.4.8. Have a PR to fix it now.

[kalefranz]

Gosh I thought i had this fixed on conda 4.4.9. Guess it's going to take another go.

[mbargull]

conda.resolve.Resolve.version_key expects conda.resolve.Resolve.index[key] to have a .channel attribute. But conda.resolve.Resolve.index.values() are never converted from dict instances. Here is a pseudo-Python backtrace of the used index:

r.version_key(dist):
    info_dict = r.index[dist]
    channel = info_dict.channel  # BOOM
r = Resolve(index)
index = {conda.models.dist.Dist(...): info_dict, ... }  # index = copy_index(index) = {conda.models.dist.Dist(key): index[key] for key in index}
index = {"name-version-build_id": info_dict, ...}  # index = {distro.pkg_fn(): distro.info_index() for distro in distros}
distros = [conda_build.Metadata(...), ...]

So, in copy_index only the keys instantiate Dists, the values remain the same dicts as returned from conda_build.Metadata.info_index(). Resolve.version_key in conda 4.3.x only used .get on those values but never directly accessed an attribute like .channel.

[kalefranz]

So the quick, but brittle, fix here is to just use the get() method instead of attribute access I guess?

[kalefranz]

BTW, for future reference, we do plan on eliminating dist. The history here is that dist is derived just from the name of the tarball (or maybe the full URL of the tarball). That's not what we want to be using as the source of truth for package metadata information. We're all set up to switch out Dist for PackageRef in almost all situations. So that's coming... Someday...

[mbargull]

I'm a little confused why the keys are conda.models.dist.Dist(key) with key = distro.pkg_fn() and thus isinstance(key, str) -- did I miss something else?

[mbargull]

Also, I haven't looked at what happened with conda._vendor.auxlib.exceptions.ValidationError: Invalid value 0 for build_number.

[kalefranz]

I don't know why the build number thing just popped up all the sudden. I'm guessing it's complaining though because build_number needs to be an integer and not a string.

[mbargull]

Ah, your last comment didn't load for me while posting.

[mbargull]

It looked as though build_number comes from conda_build.Metadata.build_number() which returns an int AFAICT -- have to take a closer look though.

[mbargull]

Ah, other test file. Maybe that comes from https://github.com/conda-tools/conda-build-all/blob/v1.1.3/conda_build_all/tests/unit/dummy_index.py#L49 ?

[mbargull]

So, if a "0" was indeed a valid input at some point, you'd have to take care of that conversion while calling https://github.com/conda/conda/pull/6808/files#diff-39ec17febc24e90aa82fc8390d50ec87R207, I suppose? If you are very certain that it was never a valid input and didn't exist in real data, you could of course just adjust the DummyIndex to use ints instead of strings..

[kalefranz]

build_number has been enforced as an integer for quite some time. https://github.com/conda/conda/blob/4.3.0/conda/models/index_record.py

It feels like something recently changed with the test data in this repo. I’ll investigate.

[mbargull]

But in conda 4.3.x no IndexRecord is instantiated when a MatchSpec is created, I think. So for conda-build-all, which directly interacts with the conda.resolve.Resolve object, there might have previously be no IndexRecord instantiation at all?

[kalefranz]

Yeah I agree that’s what it’s looking like

[kalefranz]

Just pushed a change that makes that test build number an int. I think this is safe. We've been enforcing integer build numbers for over a year, and I've never seen a problem there.

[kalefranz]

Guess there's something going on with 4.4 and special_case_version_matrix(). Implementation details of MatchSpec have changed in 4.4, and it looks like that code is digging into some of the internals. So need to dig in there now and find a solution.

[kalefranz]

I've isolated the remaining test failure here to be coming from conda-build 3.x. When I locally install conda-build 2.1.x, the test passes.

[kalefranz]

🎉 Tests are finally passing.

[jakirkham]

FWIW there has been some work to drop conda-build-all from conda-forge in PR ( https://github.com/conda-forge/staged-recipes/pull/5274 ). ATM am not sure whether that is the best way to go or pushing conda-build-all is. @isuruf is probably in the best place to answer this question. That said, I think we should come to some conclusion on that and this may effect how you decided to prioritize this work @kalefranz. Admittedly people do use conda-build-all outside of conda-forge, so this work is likely to be appreciate regardless of what direction conda-forge picks.

[kalefranz]

I actually thought there wasn’t compatibility yet with conda-build 3. So maybe there’s just one interaction between conda 4.4, conda-build 3, and conda-build-all. Test failure is at https://github.com/conda-tools/conda-build-all/issues/97

[pelson]

I'm completely happy to merge the code-changes, but don't really want to loose the cb3 testing if possible. Is there anything I can do to help get your changes in combination with the original cb3 test?

[pelson]

Ping @kalefranz. I think this is pretty much good to go, I'm just looking to avoid removing the cb3 testing if possible.

[kalefranz]

There was one test failure that pinning to conda-build 2.1 fixed.

https://travis-ci.org/conda-tools/conda-build-all/jobs/341129488#L803

So I guess we have some interaction between the new matchspec in conda, new conda-build, and conda-build-all here. I so far haven't been able to track down what it is.