conda-tools / conda-build-all

BSD 3-Clause "New" or "Revised" License
30 stars 24 forks source link

Test `get_output_file_path` #71

Closed ocefpaf closed 7 years ago

ocefpaf commented 7 years ago

Yet another attempt to fix bldpkg_path for all conda-build versions.

See https://github.com/SciTools/conda-build-all/pull/68/files#r93306331

ocefpaf commented 7 years ago

@pelson and @marqh https://github.com/SciTools/conda-build-all/pull/71/commits/1d191dd0b7850db96ba72248226f262c68401273 fixed one of the tests but reveled further failures down the road. I do not believe they are related to this PR.

Note that the use of get_output_file_path, or bldpkg_path as get_output_file_path in https://github.com/SciTools/conda-build-all/pull/71/commits/17b3b4ac0d944c88a5152457e2137c286e1ad580 is preferred than checking for hasattr(conda_build, 'api') because conda-build 2 will be pass that but won't have the config argument in the bldpkg_path.

This change could fail only for early conda-build 2 versions. I am unclear to when get_output_file_path was introduced. But it works well for conda-build 1 and latest (non-buggy) conda-build 2.

marqh commented 7 years ago

hi @ocefpaf

this is failing the internal tests in a way i'm not confident I follow i'm concerned about how to fix this

ocefpaf commented 7 years ago

No problem. Those tests were already failing BTW. I actually fixed one :grimacing: See https://travis-ci.org/SciTools/conda-build-all/builds

I will see if I can fix the remaining tests too. But I am not sure what changed in conda-build that is caused those failures.

ocefpaf commented 7 years ago

Looking the the failures here I noticed that get_index calls prioritize_channels, which creates the following for fetch_index

OrderedDict([('file:///tmp/temporary_channel_bkrgxr3k/linux64',
              ('file:///tmp/temporary_channel_bkrgxr3k', 0)),
             ('file:///tmp/temporary_channel_bkrgxr3k/noarch',
              ('file:///tmp/temporary_channel_bkrgxr3k', 0))])

we have a failure because the integration tests creates only file:///tmp/temporary_channel_bkrgxr3k/linux64.

get_index has a platform keyword, but passing platform='linux-64' does not fix it as prioritize_channels still adds the noarch (that behavior makes sense as any platform can still rely on noarch).

@msarahan do you have any advice for me here? Maybe I am missing something obvious... @pelson Maybe the solution is to change the integration tests to also create the noarch directory.

msarahan commented 7 years ago

I think the absence of noarch is fine, but you need to make linux64 into linux-64 so that conda understands it

ocefpaf commented 7 years ago

Argh. That is probably it. Thanks @msarahan!

ocefpaf commented 7 years ago

Nope. I take that back. It was my bad when manually passing the platform arg. conda-build-all does create a linux-64 directory just fine.

/tmp/temporary_channel_itngmgdp/
/tmp/temporary_channel_itngmgdp/linux-64:
repodata.jsonrepodata.json.bz2
msarahan commented 7 years ago

I'll take a look later today. What conda version are you testing with?

On Dec 22, 2016 2:23 PM, "Filipe" notifications@github.com wrote:

Nope. I take that back. It was my bad when manually passing the platform arg. conda-build-all does create a linux-64 directory just fine.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SciTools/conda-build-all/pull/71#issuecomment-268882804, or mute the thread https://github.com/notifications/unsubscribe-auth/AACV-d6LDhhayAJuF33LpMRIeBDtYTeDks5rKtwVgaJpZM4LSNKr .

ocefpaf commented 7 years ago

Here is my conda info.

> conda info
Current conda install:

               platform : linux-64
          conda version : 4.2.13
       conda is private : False
      conda-env version : 4.2.13
    conda-build version : 2.0.12
         python version : 3.5.2.final.0
       requests version : 2.12.4
       root environment : /home/filipe/miniconda3  (writable)
    default environment : /home/filipe/miniconda3
       envs directories : /home/filipe/miniconda3/envs
          package cache : /home/filipe/miniconda3/pkgs
           channel URLs : https://conda.anaconda.org/conda-forge/linux-64
                          https://conda.anaconda.org/conda-forge/noarch
                          https://repo.continuum.io/pkgs/free/linux-64
                          https://repo.continuum.io/pkgs/free/noarch
                          https://repo.continuum.io/pkgs/pro/linux-64
                          https://repo.continuum.io/pkgs/pro/noarch
            config file : /home/filipe/.condarc
           offline mode : False

Thanks @msarahan!

msarahan commented 7 years ago

@ocefpaf I think that these test failures point to an issue with Requests that I'm not sure we've worked out.

https://travis-ci.org/SciTools/conda-build-all/jobs/186152343#L447

We pin requests to an older version in conda-build: https://github.com/conda/conda-build/blob/master/.travis.yml#L40

It would be good for conda to fix this issue. It may be that having the noarch folder in place would fix it. There's been some discussion on the issue at https://github.com/conda/conda/pull/3739

ocefpaf commented 7 years ago

Thanks for looking into this @msarahan!

Locally, if I debug that, I can see that the error is due to a missing noarch repo data, and creating the noarch folder structure makes error go away. However, I am not convinced that this is the best solution here. Requests issue or no conda should be OK without the noarch folder to build the index, because its presence should not be mandatory.

I don't think there is anything else left to do in conda-build-all besides changing the nature of the test or creating the noarch folder, right? Either way I won't do that in this PR as it is unrelated to its context.

I'll try something else tomorrow.

Thanks again!

jakirkham commented 7 years ago

Could be wrong, but the failures look like issue ( https://github.com/conda/conda/issues/3928 ). Might try pinning requests to 2.11. I don't think there is an official conda release with the fix (though there is a fix).

marqh commented 7 years ago

i think I would like to see this PR updated to pin requests to an earlier version

if that fixes the test running I would be more comfortable merging the change i have tried this in #78

if this is successful, this and #74 could rebase onto that and see if we can get tests passing before we cut a release

cheers marqh

marqh commented 7 years ago

looks like #78 is not the answer

i'm going to take this on without this and try to fix that another time