Skip to content

Conversation

@jankatins
Copy link
Contributor

Followup after #5604

  • enable the tests
  • Install tools which are needed for the complete image comparison tests? Or don't use skiptests, so that one could at least understand what is coming from the svg tests and what are real "known fails" which might be investigated?
  • Switch to obviousCI
  • put the selectors in the conda meta.yml file as comments
  • Switch to the template based version information in meta.yml -> not possible (see comment below)
  • switch addon channel to janschulz to get functools32 and msinttypes (added a PR for functools in staged-recipe: functools32 3.2.3.2 conda-forge/staged-recipes#33)
  • Update meta.yml and build.sh with versions from my package builder
  • remove the py27 restriction in appveyor.yml after the channel switch
  • write short instruction how to compile on windows with conda (basically it should be "add the janschulz conda channel, install the following libs, copy some libs, run ms scripts on some python versions, run setup..." -> what appveyor does + the run_in_env command...)
  • investigate if the copy step can be replaced with runtime depencies... the conda matplotlib package suggests that this works...
  • look at Added the matplotlib 1.5 release candidate. conda-forge/staged-recipes#3, maybe there is something in there which I haven't found...
  • use local_freetype = True in setup.cfg to get image comparison tests? Probably needs windows fixes in the setup code? yep: Fix #5519: Patch now support the 'None' linestyle #5696 (comment)

Closes: #5627

@jankatins
Copy link
Contributor Author

@pelson: Could you comment on this:

{% set data = load_setuptools() %}
package:
   name: my_package
   version: {{ data.get('version') }}

-> Why not use the documented way with __conda_version__.txt?

@pelson
Copy link
Member

pelson commented Dec 9, 2015

Could you comment on this

Thanks @JanSchulz. I've responded in #5604 (comment). Essentially, if we do it the latter way, we are able to do things like conda build <recipe> --output. We go from a build time determination of the version to a meta.yaml parse time one.

@pelson
Copy link
Member

pelson commented Dec 9, 2015

switch addon channel to janschulz to get functools32 and msinttypes

I would suggest a healthier choice would be to look at extending conda-forge to include functools32. I'm happy to support you in doing this (see https://github.com/conda-forge/staged-recipes).

In general, anything that involves a single-user anaconda.org channel should be avoided, IMHO.

@jankatins
Copy link
Contributor Author

In general, anything that involves a single-user anaconda.org channel should be avoided, IMHO.

Yes, true. PR submitted...

@tacaswell
Copy link
Member

@JanSchulz state of this?

@tacaswell tacaswell added this to the proposed next point release (2.1) milestone Dec 19, 2015
@jankatins
Copy link
Contributor Author

on it during the next hours on the train...

@jankatins
Copy link
Contributor Author

@pelson I tried your way, but it doesn't work for multiple reasons:

c:\data\external\pydata\matplotlib (appveyor2)
λ conda build ci\conda_recipe --output
C:\portabel\miniconda\conda-bld\win-64\matplotlib-test-None-py27_0.tar.bz2

First, setup (which is monkey patched internally by conda build during the meta.yaml processing to set the version) isn't called if the requirments aren't satisfied (there is a sys.exit(1) in line 227). Unfortunately, the requirements usually aren't satisfied, as they are only installed into the _built environment and not the root env where conda build is getting it's modules from.

The second reason is that matplotlibs setup call is protected by a if __name__ == '__main__': and as far as I understand python, this will not the case:

c:\data\external\pydata\matplotlib (appveyor2)
λ cat test.py
code = """
print("before")
if __name__ == '__main__':
   print("yes")
"""
exec(code)
c:\data\external\pydata\matplotlib (appveyor2)
λ python test.py
before
yes

c:\data\external\pydata\matplotlib (appveyor2)
λ python -c "import test"
before

I've asked for a new way to get the version directly from versioneer: conda/conda-build#714 [update: there won't be such a change, so I can't see a way to replace the current version overwrite with a template one. @pelson if you have an idea how to fix this, that would be great...]

@jankatins
Copy link
Contributor Author

Current status: I want to have a look how to compile with a local freetype version, so that the tests can run (and afterwards enable the tests again). But other than this, I'm happy with the PR...

@Tillsten
Copy link
Contributor

Since FT 2.6 it uses ctypes and is easly build if the compiler is already set up,
based on matplotlib-winbuild i have something like that, maybe it is helpful.

https://gist.github.com/Tillsten/d0f176667fed7cea17cd

@jankatins
Copy link
Contributor Author

Ok, I looked into the freetype building stuff (local_freetype=True) and it looks more or less like if one wants to compile it, it would need to include the relevant functions from https://github.com/jbmohler/matplotlib-winbuild/blob/master/utils.py, which is half/most of the file...

From there it's almost easier/cleaner to include the complete source of that repo (adjusted to download the included sources (tars/zips) instead of using checked in ones...).

@tacaswell @mdboom Any preference: just include the minimal source to compile the downloaded freetype or include the complete utils.py and let it compile everything which isn't found on the (windows-)system?

The first will mean that I inline all relevant helper functions from utils.py and refatcor the local_freetype path in setupext.py to swithc between *ix (current codepath) and windows (utils.py stuff). If the latter, it would mean that each dependency would gain a "local_whatever" switch and a do_custom_build() to download and build the relevant lib. In any case, I would add a setup_local_build.py which gets all the download and compile steps to not blow up setupext.py

def do_custom_build(self):
        # We're using a system freetype
        if not options.get('local_freetype'):
            return
        from setup_local_build import freetype_build
        freetype_build()

and in setup_local_build.py:

FREETYPE = (<url>, <hash>)

[...]
def freetype_build():
    download_and_unpack(FREETYPE)
    if sys.platform == 'win32':
        freetype_build_win()
    else:
        freetype_build_unix()

def freetype_build_unix():
    [... current codepath...]

def freetype_build_win():
    [... new codepath from utils.py...]

def tcl_build():
    download_and_unpack(TCL)
    if sys.platform == 'win32':
        tcl_build_win()
    else:
        raise NotImplementedException("Please use TCL included in your distro")

def tcl_build_win():
    [... new codepath from utils.py...]

CC: @jbmohler @Tillsten

@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

I think I prefer the latter. Seems better organized.

@jankatins
Copy link
Contributor Author

@mdboom Had a closer look and it seems that this would also need fixes for most other parts of the individual tests (e.g. add_flags). So it seems that it is easier to just add to the current setupext infrastructure :-/

@jankatins
Copy link
Contributor Author

Got the "local freetype" build going...

@tacaswell
Copy link
Member

Looks like the current problem is that meta.yml needs functools32 for 2.7

@jankatins
Copy link
Contributor Author

Ok, the current problem looks like we need to clean the build area before starting the conda build... tomorrow...

@jankatins
Copy link
Contributor Author

I'm a bit perplexed:

[...]
he following NEW packages will be INSTALLED: 
[...] 
    numpy:           1.8.2-py27_0                         defaults        
[...]
===== testing package: matplotlib-1.5.0+1192.gd81ebcf.dirty-np18py27_0 ===== 
import: u'matplotlib'
import: u'matplotlib.pyplot' 
RuntimeError: module compiled against API version a but this version of numpy is 9 
Traceback (most recent call last): 
  File "C:\conda\conda-bld\test-tmp_dir\run_test.py", line 28, in <module> 
    import matplotlib.pyplot 
  File "C:\conda\envs\_test\lib\site-packages\matplotlib\pyplot.py", line 29, in <module>
    import matplotlib.colorbar 
  File "C:\conda\envs\_test\lib\site-packages\matplotlib\colorbar.py", line 32, in <module>
    import matplotlib.artist as martist
  File "C:\conda\envs\_test\lib\site-packages\matplotlib\artist.py", line 15, in <module>
    from .transforms import (Bbox, IdentityTransform, TransformedBbox, 
  File "C:\conda\envs\_test\lib\site-packages\matplotlib\transforms.py", line 39, in <module>
    from matplotlib._path import (affine_transform, count_bboxes_overlapping_bbox, 
ImportError: numpy.core.multiarray failed to import 
TESTS FAILED: matplotlib-1.5.0+1192.gd81ebcf.dirty-np18py27_0

Why "version a"? https://github.com/numpy/numpy/blob/master/numpy/core/code_generators/generate_numpy_api.py#L91

@mdboom
Copy link
Member

mdboom commented Jan 25, 2016

Why "version a"?

Numpy ABI versions don't have a 1-to-1 correspondance with Numpy versions. And the code is displaying the number in hexadecimal so a == 10.

My guess here is that the build is not being entirely cleaned before being rebuilt for a different numpy version.

EDIT: Oh, and I see you already said that...

@jankatins
Copy link
Contributor Author

re the string failure:

ok in py27, 64bit during the regular build:

C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\Bin\amd64\cl.exe /c /nologo /Ox /MD /W3 /GS- /DNDEBUG -DFREETYPE_BUILD_TYPE=system -DPY_ARRAY_UNIQUE_SYMBOL=MPL_matplotlib_ft2font_ARRAY_API -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -IC:\conda\envs\test-environment\lib\site-packages\numpy\core\include -IC:\conda\envs\test-environment\Library\include -I. -IC:\conda\envs\test-environment\Library\include/freetype2 -IC:\conda\envs\test-environment\include -IC:\conda\envs\test-environment\PC /Tpsrc/ft2font.cpp /Fobuild\temp.win-amd64-2.7\Release\src/ft2font.obj 
ft2font.cpp
C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\Include\xlocale(342) : warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc

not ok on 2.7, 64bit during the conda build:

C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\Bin\amd64\cl.exe /c /nologo /Ox /MD /W3 /GS- /DNDEBUG -DFREETYPE_BUILD_TYPE=system -DPY_ARRAY_UNIQUE_SYMBOL=MPL_matplotlib_ft2font_ARRAY_API -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -IC:\conda\envs\_build\lib\site-packages\numpy\core\include -IC:\conda\envs\_build\Library\include -I. -IC:\conda\envs\_build\Library\include/freetype2 -IC:\conda\envs\_build\include -IC:\conda\envs\_build\PC /Tpsrc/ft2font.cpp /Fobuild\temp.win-amd64-2.7\Release\src/ft2font.obj 
ft2font.cpp
src/ft2font.cpp(5) : fatal error C1083: Cannot open include file: 'string': No such file or directory

-> No difference :-(

I currently suspect that somehow the proper vcvarsall is not called and therfore not all include path are set: conda/conda-build#729

Lets see what the build log for the current debug build tells us...

CC: @pelson

@jankatins
Copy link
Contributor Author

py27, 64bit:
https://ci.appveyor.com/project/JanSchulz/matplotlib/build/1.0.104/job/k30t1bkb9s1ljya6#L7294

[test-environment] C:\conda\conda-bld\work>call "C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\vcvarsall.bat" amd64 
The specified configuration type is missing.  The tools for the
configuration might not be installed.

-> This is a problem in conda-build which does not prefers the installed python vs compiler over the normal VC install (which seems to be missing the 64bit compilers... :-()

PY34, 64bit: https://ci.appveyor.com/project/JanSchulz/matplotlib/build/1.0.104/job/qk7e8eska42nsv39#L7402

[test-environment] C:\conda\conda-bld\work>call "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\vcvarsall.bat" amd64 
The specified configuration type is missing.  The tools for the
configuration might not be installed.

-> This is point 4 on this entry: http://blog.ionelmc.ro/2014/12/21/compiling-python-extensions-on-windows/#for-python-3-4

Instead of call <...>vcvarsall.bat amd64 it should run CALL "C:\Program Files\Microsoft SDKs\Windows\v7.1\Bin\SetEnv.cmd" /x64

@pelson
Copy link
Member

pelson commented Jan 25, 2016

FWIW This is the same issue discussed at conda-forge/matplotlib-feedstock#1. I'm interested to learn that this may be a conda issue. Unfortunately, I don't own a 64-bit Windows machine, and therefore haven't been able to reproduce locally (I do own a 32-bit machine, and that has been invaluable in debugging many issues). I suspect if you can reproduce quickly, it will be reasonably quick to determine how conda / the build system should be behaving.

@jankatins
Copy link
Contributor Author

@pelson It could be argued that the PY34, 64bit is a problem by appveyor, that they don't setup the python build environment properly, but on the other hand that's the way it is directly after installing without the manual corrections... I'm currently trying a workaround by using the following in bld.bat:

:: CMD_IN_ENV is needed because of https://github.com/conda/conda-build/issues/729
%CMD_IN_ENV% python setup.py install

@jankatins
Copy link
Contributor Author

@pelson It would be really great if the %CMD_IN_ENV% app could be made a real app:

  • getting the VS tools from the python in path (parse python --version)
  • some error messages if this isn't setup as it is expected
  • be useable ala
python_build_env python setup.py build

@jankatins
Copy link
Contributor Author

@pelson Ok, that didn't work: now it can't find any python package?

REQUIRED DEPENDENCIES AND EXTENSIONS
                 numpy: yes [not found. pip may install it below.]
              dateutil: yes [dateutil was not found. It is required for date
                        axis support. pip/easy_install may attempt to
                        install it after matplotlib.]
           functools32: yes [functools32 was not found. It is required for
                        forpython versions prior to 3.2 pip/easy_install may
                        attempt to install it after matplotlib.]
                  pytz: yes [pytz was not found. pip/easy_install may
                        attempt to install it after matplotlib.]
                cycler: yes [cycler was not found. pip/easy_install may
                        attempt to install it after matplotlib.]
               tornado: yes [tornado was not found. It is required for the
                        WebAgg backend. pip/easy_install may attempt to
                        install it after matplotlib.]
             pyparsing: yes [pyparsing was not found. It is required for
                        mathtext support. pip/easy_install may attempt to
                        install it after matplotlib.]

damn...

@jankatins
Copy link
Contributor Author

@pelson: Ok, new hypothesis: the things we set in bld.bat undo the setup which is done by conda and therefore the linker doesn't find the files anymore.

The biggest difference is that there is a difference what is run when:

normal build:

  • we setup individual env vars
  • let VC handle its own env vars (via %CMD_IN_ENV%)
  • setup.py is run

conda build:

  • let VC handle its own env vars (via condas prepended commands)
  • conda sets additional env vars
  • we setup some vars to initial values # <- overwrites the above...
  • setup.py is run

This is what is called by the bld.bat (after conda build adds it's own commands to the start):

C:\conda\conda-bld\work>call "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\vcvarsall.bat" amd64 
The specified configuration type is missing.  The tools for the
configuration might not be installed

C:\conda\conda-bld\work>set INCLUDE=C:\conda\envs\_build\Library\include;c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\INCLUDE;C:\Program Files\Microsoft SDKs\Windows\v7.1\INCLUDE;C:\Program Files\Microsoft SDKs\Windows\v7.1\INCLUDE\gl; 

C:\conda\conda-bld\work>set LIB=C:\conda\envs\_build\Library\lib;c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\Lib\amd64;C:\Program Files\Microsoft SDKs\Windows\v7.1\Lib\X64; 

C:\conda\conda-bld\work>REM ===== end generated header ===== 

C:\conda\conda-bld\work>set LIB=C:\conda\envs\_build\Library\lib 

C:\conda\conda-bld\work>set LIBPATH=C:\conda\envs\_build\Library\lib; 

C:\conda\conda-bld\work>set INCLUDE=C:\conda\envs\_build\Library\include;C:\conda\envs\_build\Library\include\freetype2 

If this hypothesis is right, then including the old variables in the new set will let the build succeed... Lets see...

@jankatins
Copy link
Contributor Author

And yay, it seems to work.

@pelson you might want to implement something similar to https://github.com/JanSchulz/matplotlib/commit/63db4decca2501265db0f7b4ffae8147894776a1

[edit: or not, not the 3.4 64bit build gets a 32bit 2.7 python?]

Conda prefers the normal VS tools for py27 instead of the python
specific ones and appeveyor has no 64bit versions installed there.

Same for the py34 version of VS, which lacks the vcvars64.bat "hack":
point 4 in
http://blog.ionelmc.ro/2014/12/21/compiling-python-extensions-on-windows/#for-python-3-4

Also fix some environtment variable leaks from the test environment
to the conda build environment.

Also fix the build error on 64bit builds
@jankatins
Copy link
Contributor Author

Ok the 3.4 build is hopefully fixed by using %PYTHON% instead of python... No clue why this could happen...

I enabled the tests (still not failing, only showing...) and the wheel build again, lets see how this works...

@jankatins
Copy link
Contributor Author

Yay, green...

@jankatins
Copy link
Contributor Author

Ok, this is now green but the tests still fail (and are silenced to let the build continue).

Not sure how to proceed here. IMO the best is to merge and then handle the test failure in a different PR (e.g. #5748)?

There is also the case that some image comparison tools are missing (GS,...), so only 1 of the 3 tests per image are run. There are currently no conda packages for these tools, so this needs tooling support in other parts of the community :-/

@jenshnielsen
Copy link
Member

This is great progress and I fully support merging it now and improve further in a new PR.

tacaswell added a commit that referenced this pull request Jan 26, 2016
WIP: more windows build and CI changes
@tacaswell tacaswell merged commit 2a4863c into matplotlib:master Jan 26, 2016
@tacaswell
Copy link
Member

🍻

@mdboom
Copy link
Member

mdboom commented Jan 26, 2016

Yes, thanks @JanSchulz. This is tiresome work with long feedback loops. But really worthwhile, given the imbalance between users on Windows and developers on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Followup for Windows CI stuff

7 participants