Implement conda-build 3 support; change to pytest #625

msarahan · 2017-12-08T14:46:06Z

Replacing #618 because I force-pushed and github won't let me reopen that PR.

There are 2 major changes here:

conda-build-all and the platform spoofing/fudging that conda-smithy used to do is now done with conda-build. Its API takes platform and arch arguments that achieve the same purpose.
The build matrix is done with conda-build 3's new conda_build_config.yaml files. Init and regenerate (rerender) commands take a new argument to point at your desired matrix configuration file. The actually used matrix files are saved into the ci_support/matrix folder, with names like circle_python2.7.yaml or travis_r_base3.4.2.yaml.

This implies that pinning updates should be done by maintaining a central conda_build_config.yaml file, and using that when performing init or rerender operations. Each feedstock will then carry its own subset of this configuration until it gets re-rendered.

The configure_feedstock tests ended up being so completely different that I needed to rewrite them. Since I am much more familiar with pytest fixtures, and because pytest seems prominent, I switched over to that. I hope that's OK. Pytest still runs the old test suite as well.

…oise.

…es in configs

…ix linter on jinja2 functions

…-coveralls

msarahan · 2017-12-08T14:49:26Z

I have some extra stuff to replace travis with circle CI. I didn't include it here to limit the scope of this PR. If you guys want it, let me know.

msarahan · 2017-12-08T14:54:17Z

conda_smithy/templates/appveyor.yml.tmpl

@@ -47,12 +47,15 @@ install:
    {% if build_setup -%}
    {{ build_setup }}{% endif %}

+    # testing purposes: get conda-build from defaults
+    - cmd: conda install -n root --quiet --yes -c defaults conda-build


Remove before merging

msarahan · 2017-12-08T14:54:34Z

conda_smithy/templates/run_docker_build.tmpl

@@ -72,9 +59,12 @@ conda install --yes --quiet conda-forge-build-setup
 {% if build_setup -%}
 {{ build_setup }}{% endif -%}

-conda build /recipe_root --quiet || exit 1
+# testing purposes: get conda-build from defaults
+conda install -n root --quiet --yes -c defaults conda-build


Remove before merging

msarahan · 2017-12-08T14:54:43Z

conda_smithy/templates/travis.yml.tmpl

@@ -68,9 +63,10 @@ install:
      conda install --yes --quiet conda-forge-build-setup
      {% if build_setup -%}
      {{ build_setup }}{% endif %}
+      conda install -n root --quiet --yes -c defaults conda-build


Remove before merging

msarahan · 2017-12-08T14:55:19Z

tests/recipes/click-test-feedstock/.travis.yml

+      conda config --set show_channel_urls true
+      conda install --yes --quiet conda-forge-build-setup
+      source run_conda_forge_build_setup
+      conda install -n root --quiet --yes -c defaults conda-build


Can remove before merging, but it doesn't matter much

msarahan · 2017-12-08T14:55:26Z

tests/recipes/click-test-feedstock/appveyor.yml

+    - cmd: run_conda_forge_build_setup
+
+    # testing purposes: get conda-build from defaults
+    - cmd: conda install -n root --quiet --yes -c defaults conda-build


Can remove before merging, but it doesn't matter much

msarahan · 2017-12-08T14:55:33Z

tests/recipes/click-test-feedstock/ci_support/run_docker_build.sh

+source run_conda_forge_build_setup
+
+# testing purposes: get conda-build from defaults
+conda install -n root --quiet --yes -c defaults conda-build


Can remove before merging, but it doesn't matter much

isuruf · 2017-12-08T17:04:00Z

Thanks @msarahan, I tried this out in some feedstocks. (Using conda-build 3.1.1 and conda 4.3.29) Some observations

win-32 is skipped. (eg: python-feedstock)
only python 3.6 is built (eg: numpy-feedstock)
rerendering is much faster than before. 💯

msarahan · 2017-12-08T20:04:41Z

How were you introducing a build matrix? Both win-32 and different python versions are things that you need to set in some conda_build_config.yaml file, and then point conda-smithy at that file with the -m argument when re-rendering.

If you don't use a conda_build_config.yaml file, yeah, you won't get a matrix.

isuruf · 2017-12-08T23:59:49Z

Ah. That's it. How about if no yaml file is given, checking whether conda-forge-pinnning package is up to date and use it if it is?

msarahan · 2017-12-09T00:24:30Z

Yes, that's one way to do it. I'm honestly totally agnostic on that. I put up the conda-forge-pinning package PR on staged recipes as a straw man to talk about. It could also be downloaded directly, as is done at https://github.com/conda-forge/conda-smithy/blob/master/conda_smithy/configure_feedstock.py#L117

I'd really rather not try to perfect the workflow with this PR. It is already very large. Let's use this PR to talk about the direction to head, and we can build on it in future work.

isuruf · 2017-12-09T00:34:24Z

isuru@dell:~/projects/numpy-feedstock$ conda smithy rerender -m ../cf_config.yaml 
setting build platform. This is only useful when pretending to be on another another platform, such as for rendering necessary dependencies on a non-native platform. I trust that you know what you're doing.
setting build arch.  This is only useful when pretending to be on another arch, such as for rendering necessary dependencies on a non-native arch.  I trust that you know what you're doing.
Traceback (most recent call last):
  File "/home/isuru/miniconda3/bin/conda-smithy", line 11, in <module>
    load_entry_point('conda-smithy==2.4.5.dev11+msarahan.conda.build.3', 'console_scripts', 'conda-smithy')()
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_smithy-2.4.5.dev11+msarahan.conda.build.3-py3.6.egg/conda_smithy/cli.py", line 311, in main
    args.subcommand_func(args)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_smithy-2.4.5.dev11+msarahan.conda.build.3-py3.6.egg/conda_smithy/cli.py", line 215, in __call__
    variant_config_files=args.variant_config_files)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_smithy-2.4.5.dev11+msarahan.conda.build.3-py3.6.egg/conda_smithy/configure_feedstock.py", line 460, in main
    render_appveyor(env, config, forge_dir)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_smithy-2.4.5.dev11+msarahan.conda.build.3-py3.6.egg/conda_smithy/configure_feedstock.py", line 361, in render_appveyor
    platform_specific_setup=_appveyor_specific_setup)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_smithy-2.4.5.dev11+msarahan.conda.build.3-py3.6.egg/conda_smithy/configure_feedstock.py", line 140, in _render_ci_provider
    bypass_env_check=True)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_build/api.py", line 44, in render
    permit_unsatisfiable_variants=permit_unsatisfiable_variants)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_build/render.py", line 692, in render_recipe
    allow_no_other_outputs=True, bypass_env_check=bypass_env_check)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_build/render.py", line 485, in distribute_variants
    squished_variants = list_of_dicts_to_dict_of_lists(variants)
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_build/variants.py", line 425, in list_of_dicts_to_dict_of_lists
    values = list(zip(*set(zip(*(squished[key] for key in group)))))
  File "/home/isuru/miniconda3/lib/python3.6/site-packages/conda_build/variants.py", line 425, in <genexpr>
    values = list(zip(*set(zip(*(squished[key] for key in group)))))
KeyError: 'python'

jakirkham · 2018-01-08T03:08:49Z

click-feedstock. Wanted to update given the changes to conda-smithy, but couldn't figure out how you re-rendered.

msarahan · 2018-01-08T14:00:16Z

command history shows conda-smithy init --feedstock-directory tests/recipes/click-test-feedstock ../aggregate/click-feedstock/recipe -m ./conda_build_config.yaml

but given the addition of conda-forge-pinning, you'd change that last part to point to wherever you have the one from conda-forge-pinning installed. It looks like I had hacked together a one-off loop over python versions that I never committed and later removed.

…a-build-3'

jakirkham · 2018-01-12T18:01:17Z

Would you mind committing a working file with it?

We can certainly have an option to disable using conda-forge-pinning as noted in the other PR. Alternatively we could just forgo command line options altogether and just specify these things in conda-forge.yml (thus having conda-forge-pinning as a default option there along with the equivalent file in the recipe when nothing else is specified).

msarahan · 2018-01-26T21:47:20Z

@jakirkham For the click feedstock, it's really nothing more than

python:
  - 2.7
  - 3.6

That file is written with a test fixture, py_recipe: https://github.com/conda-forge/conda-smithy/pull/625/files#diff-484462fced51d1a06b1d93b4a44dd535R113

which in turn uses another fixture: https://github.com/conda-forge/conda-smithy/pull/625/files#diff-484462fced51d1a06b1d93b4a44dd535R45

I have added a new option that I think captures your request of an option to help people avoid accidentally using the wrong config files. See conda/conda-build#2661

isuruf

@msarahan, this looks good to me. One more thing, would it be possible to remove unneeded packages from pin_run_as_build when saving the matrix files? (We can merge this PR and do it another PR if you like)

For how to use conda-forge-pinning, I was thinking of using conda-forge-pinning as a dependency of conda-smithy and then checking that conda-forge-pinning and conda-smithy itself is up-to-date. I'll do that next weekend if nobody beats me to it.

mingwandroid

Should you call this again here:

https://github.com/msarahan/conda-smithy/blob/5bb8b26c319a454e5353d82645f2a410f66a4f18/conda_smithy/configure_feedstock.py#L195

isuruf · 2018-02-10T18:21:53Z

conda_smithy/configure_feedstock.py

+def _render_ci_provider(provider_name, jinja_env, forge_config, forge_dir, platform, arch,
+                        fast_finish_text, platform_target_path, platform_template_file,
+                        platform_specific_setup, keep_noarch=False, extra_platform_files=None):
+    metas = conda_build.api.render(os.path.join(forge_dir, 'recipe'),


@msarahan, shouldn't the channel urls be sent to the render method?

they're in forge_config? Yes, they should be passed in here. Please confirm where they are, and I'll add them in.

It's in forge_config['channels']['sources']

It's a list with highest order at the beginning

isuruf · 2018-02-10T19:47:35Z

For how to use conda-forge-pinning, I was thinking of using conda-forge-pinning as a dependency of conda-smithy and then checking that conda-forge-pinning and conda-smithy itself is up-to-date. I'll do that next weekend if nobody beats me to it.

I've got a working implementation for this. I'll send it separate after this is merged. This PR is huge enough as it is.

isuruf · 2018-02-10T19:56:09Z

tests/test_cli.py

+    # the matrix should be consolidated among all outputs, as well as the top-level
+    # reqs. Only the top-level reqs should have indedependent config files,
+    # though - loops within outputs are contained in those top-level configs.
+    assert len(os.listdir(matrix_dir)) == 13


Shouldn't this be 25? Tests failing because of this. (25 = 2 (libpq) * 2 (libpng) * 2 (jpeg) * 3 (os) + 1)

Seems to be a problem in conda-build. The matrix has two levels. Things at the "top" level are things that affect the top-level recipe. Essentially all of conda-forge recipes are just simply top-level recipes. There's also output-level stuff. In the recipe, take a look at where libpng, libpq, and jpeq occur. libpng and libpq are top-level, but jpeg is output-level. As an output-level thing, it should not enter into the outer loop, but instead should be a loop inside the build (a list in the generated conda_build_config.yaml), and ultimately would be a loop within a single build job on the given CI worker.

I hope that helps clarify things. The test is correct, but the conda-build behavior is wrong.

Thanks for the explanation. Makes sense to me.

jakirkham · 2018-02-10T20:02:14Z

Thanks for working on the conda-forge-pinning stuff, @isuruf.

Have you looked at PR ( conda/conda-build#2661 )? If you haven't, would suggest taking a look. I think that PR provides the right option for using conda-forge-pinning. Otherwise user content might muddle rerendering.

isuruf · 2018-02-10T20:05:14Z

Have you looked at PR ( conda/conda-build#2661 )? If you haven't, would suggest taking a look. I think that PR provides the right option for using conda-forge-pinning. Otherwise user content might muddle rerendering.

Thanks for reminding me. I'll fix that as well, but not in this PR

msarahan · 2018-02-10T21:04:35Z

Thanks for taking a look @isuruf. Good comments, and good catch on the conda-build matrix issue. I'll fix that as soon as I can manage. I don't have a strong opinion on whether it should block merging this PR or not. It won't affect anyone right now - just an annoying detail for me, and would make CIs less efficient for recipes with multiple outputs.

isuruf · 2018-02-10T21:20:48Z

tests/test_cli.py

+    matrix_folder = os.path.join(feedstock_dir, 'ci_support', 'matrix')
+
+    initial_commit = subprocess.check_output(['git', 'rev-parse', 'HEAD'],
+                                             cwd=feedstock_dir).strip()


This git command is working on the conda-smithy git repo right? It just reset my changes.

I hope not - it should be in the repo that lives within the conda-smithy repo, but it may have gotten mixed up. Sorry.

It's not this line. There's a git reset --hard line below

msarahan · 2018-02-10T21:36:50Z

woohoo! thanks @isuruf! Let's try to have a meeting sometime soon to talk about what the next steps are, and who can help in what way.

isuruf · 2018-02-10T21:37:12Z

Thank you very much for the PR @msarahan. And @jakirkham and @mingwandroid for the review

I've opened #650, #651 for some comments I left here and added them to 3.0.0 milestone due a week from now.

msarahan added 10 commits December 6, 2017 21:19

initial work on cb3. Needs to add dynamic recipe folders somehow.

3b115b1

make new config files be added into git. Sort entries for less diff n…

4e627cb

…oise.

refactor; move configs to ci_support/matrix; keep only 'used' variabl…

2584c3d

…es in configs

record all used vars, not just loop vars (req cb 3.0.31)

2653054

port tests to pytest, fix more configure_feedstock cli stuff

852366f

move lint and io tests into pytest structure; add cb3 linting test, f…

25b3b30

…ix linter on jinja2 functions

update recipe deps; update travis yml for pytest

5ec4db8

add python matrix and win target_platform tests

80ed123

add integration tests for init and rerendering

ff68e55

consolidate .coveragerc to setup.cfg; use coveralls instead of python…

98b2e11

…-coveralls

msarahan force-pushed the conda-build-3 branch from 998e22f to 39330a7 Compare December 8, 2017 14:48

msarahan force-pushed the conda-build-3 branch from 39330a7 to 454b1d0 Compare December 8, 2017 14:53

msarahan commented Dec 8, 2017

View reviewed changes

msarahan force-pushed the conda-build-3 branch 4 times, most recently from 9a643ce to fcdc5c3 Compare December 8, 2017 15:12

limit scope of find_package to just conda_smithy

f58fd4c

msarahan force-pushed the conda-build-3 branch from fcdc5c3 to f58fd4c Compare December 8, 2017 15:18

Merge remote-tracking branch 'conda-forge/master' into 'msarahan/cond…

89a6c9e

…a-build-3'

isuruf mentioned this pull request Jan 24, 2018

Use Anaconda-5 gcc packages conda-forge/parcels-feedstock#3

Closed

msarahan mentioned this pull request Jan 26, 2018

implement exclusive config file conda/conda-build#2661

Merged

msarahan mentioned this pull request Feb 5, 2018

Fix noarch: python with "python-scripts" for 2.1.x conda/conda-build#2672

Closed

Merge branch 'master' of github.com:conda-forge/conda-smithy

2e9e410

isuruf approved these changes Feb 5, 2018

View reviewed changes

trim_unused_pin_run_as_build

5bb8b26

mingwandroid reviewed Feb 10, 2018

View reviewed changes

Run trim_unused_pin_run_as_build again

28308f3

isuruf reviewed Feb 10, 2018

View reviewed changes

Fix _trim_unused_pin_run_as_build

4b5426a

isuruf reviewed Feb 10, 2018

View reviewed changes

add in configured channels when rendering

37702e1

isuruf reviewed Feb 10, 2018

View reviewed changes

Mark a test as xfail

01ee530

isuruf merged commit ac5114c into conda-forge:master Feb 10, 2018

msarahan mentioned this pull request Feb 12, 2018

Fix failing test due to conda-build #650

Closed

rth mentioned this pull request Feb 24, 2018

Package vowpal wabbit conda-forge/staged-recipes#5252

Open

jakirkham mentioned this pull request Apr 10, 2018

Conda smithy incompatible with conda 4.4 (missing conda.config) #630

Closed

jakirkham mentioned this pull request Apr 12, 2023

Re-rendering with conda-build 3 (post conda-build-all) #614

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement conda-build 3 support; change to pytest #625

Implement conda-build 3 support; change to pytest #625

msarahan commented Dec 8, 2017

msarahan commented Dec 8, 2017

msarahan Dec 8, 2017

msarahan Dec 8, 2017

msarahan Dec 8, 2017

msarahan Dec 8, 2017

msarahan Dec 8, 2017

msarahan Dec 8, 2017

isuruf commented Dec 8, 2017

msarahan commented Dec 8, 2017

isuruf commented Dec 8, 2017

msarahan commented Dec 9, 2017

isuruf commented Dec 9, 2017

jakirkham commented Jan 8, 2018

msarahan commented Jan 8, 2018

jakirkham commented Jan 12, 2018

msarahan commented Jan 26, 2018

isuruf left a comment

mingwandroid left a comment

isuruf Feb 10, 2018

msarahan Feb 10, 2018

isuruf Feb 10, 2018

isuruf Feb 10, 2018

isuruf commented Feb 10, 2018

isuruf Feb 10, 2018

msarahan Feb 10, 2018

isuruf Feb 10, 2018

jakirkham commented Feb 10, 2018

isuruf commented Feb 10, 2018

msarahan commented Feb 10, 2018

isuruf Feb 10, 2018

msarahan Feb 10, 2018

isuruf Feb 10, 2018

msarahan commented Feb 10, 2018

isuruf commented Feb 10, 2018

Implement conda-build 3 support; change to pytest #625

Implement conda-build 3 support; change to pytest #625

Conversation

msarahan commented Dec 8, 2017

msarahan commented Dec 8, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isuruf commented Dec 8, 2017

msarahan commented Dec 8, 2017

isuruf commented Dec 8, 2017

msarahan commented Dec 9, 2017

isuruf commented Dec 9, 2017

jakirkham commented Jan 8, 2018

msarahan commented Jan 8, 2018

jakirkham commented Jan 12, 2018

msarahan commented Jan 26, 2018

isuruf left a comment

Choose a reason for hiding this comment

mingwandroid left a comment

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

Choose a reason for hiding this comment

isuruf commented Feb 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Feb 10, 2018

isuruf commented Feb 10, 2018

msarahan commented Feb 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msarahan commented Feb 10, 2018

isuruf commented Feb 10, 2018