Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding conda-forge recipe for libbezier. #10762

Merged
merged 21 commits into from
Feb 13, 2020
Merged

Adding conda-forge recipe for libbezier. #10762

merged 21 commits into from
Feb 13, 2020

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 5, 2020

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml"
  • License file is packaged (see here for an example)
  • Source is from official source
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged)
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there

@dhermes
Copy link
Contributor Author

dhermes commented Feb 5, 2020

@conda-forge-admin, please ping @conda-forge/help-c-cpp

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/libbezier) and found it was in an excellent condition.

:: - `%LIBRARY_PREFIX%`
:: H/T: https://docs.conda.io/projects/conda-build/en/latest/user-guide/environment-variables.html

mkdir build
Copy link
Contributor Author

@dhermes dhermes Feb 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took my inspiration from the libpng-provided bld.bat, which used

mkdir build-%SUBDIR%-%c_compiler%

I couldn't find any mention of SUBDIR or c_compiler (or f_compiler) in the environment variables doc, so am not sure what would be the best move here. (Similar question applies to the build directory created in build.sh.)

UPDATE: I see in a build that fortran_compiler=flang and m2w64_fortran_compiler=m2w64-toolchain get set.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 5, 2020

...
CMake Error: The source directory "/home/conda/staged-recipes/build_artifacts/libbezier_1580879417015/work" does not appear to contain CMakeLists.txt.
...

Oh right, I forgot that the libbezier CMake configs live in src/fortran.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 5, 2020

Note, I tagged @conda-forge/help-c-cpp even though this is a Fortran package. The reason being that gfortran ships with gcc (so shared toolchain) and the built artifact is a shared library that provides a C API/ABI the same way a C/C++ library would. (Though there is no C++ API.)

-DCMAKE_INSTALL_PREFIX:PATH="%LIBRARY_PREFIX%" ^
-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON ^
-DTARGET_NATIVE_ARCH:BOOL=OFF ^
-S "%SRC_DIR%\src\fortran" ^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove a lot of the fortran source stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"%SRC_DIR%\src\fortran" is because "%SRC_DIR%\src\fortran\CMakeLists.txt" is the entry point to my CMake build. A more common thing to see would be "%SRC_DIR%\CMakeLists.txt" at the root of the project, then -S "%SRC_DIR% is how I'd specify the build.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 11, 2020

OK @scopatz we are all green. Anything else left to do?


I'm unclear if there needs to be support for the different possible variants of compiler("fortran").

requirements:
build:
- {{ compiler("fortran") }}
- cmake
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to add make here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love some guidance here actually. So I use

cmake \
    --build "${BUILD_DIR}" \
    --config Release \
    --target install

which means whatever the CMAKE_GENERATOR that was used during the configure step will get used during the build / install step.

I've observed

  • Windows: CMAKE_GENERATOR=NMake Makefiles
  • macOS: ??
  • Linux: CMAKE_GENERATOR=Unix Makefiles (based on this)

However I am manually overriding on Windows to be -G "MinGW Makefiles". So it seems I do need to depend on make (since Unix Makefiles is the default on macOS and Linux) and on Windows I need to depend on something that will ensure mingw is present.

After reading a bit more, I think I was doing the "wrong" thing by relying on AppVeyor %PATH% manipulation to find gfortran. I also now realize what the *_compiler suffix actually does with the Jinja2 compiler() helper, so that on Windows when it prints

...
(base) %SRC_DIR%>set "fortran_compiler=flang" 
...
(base) %SRC_DIR%>set "m2w64_fortran_compiler=m2w64-toolchain" 
(base) %SRC_DIR%>set "CMAKE_GENERATOR=NMake Makefiles" 
...

That means I should be using {{ compiler("m2w64_fortran") }}.

A build [1] has failed with

```
CMake Error at C:/Miniconda/conda-bld/libbezier_1580880865251/_build_env/Library/share/cmake-3.16/Modules/CMakeTestFortranCompiler.cmake:45 (message):
  The Fortran compiler

    "C:/Miniconda/conda-bld/libbezier_1580880865251/_build_env/Library/bin/flang.exe"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: C:/Miniconda/conda-bld/libbezier_1580880865251/work/build/CMakeFiles/CMakeTmp

...

-- The Fortran compiler identification is Flang 99.99.1
-- The C compiler identification is GNU 8.1.0
-- Check for working Fortran compiler: C:/Miniconda/conda-bld/libbezier_1580880865251/_build_env/Library/bin/flang.exe
-- Check for working Fortran compiler: C:/Miniconda/conda-bld/libbezier_1580880865251/_build_env/Library/bin/flang.exe  -- broken
-- Configuring incomplete, errors occurred!
```

[1]: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=117712&view=logs&j=240f1fee-52bc-5498-a14a-8361bde76ba0&t=78a0099b-3f13-5ae9-c1f0-24f71d1eac20
I'm hoping this resolves the issues I saw with using `IF`
statements (but I have very little experience with Windows).
Also using ALL_CAPS for DOS batch keywords.
It didn't turn out to be helpful.
Also adding **explicit** dependency on `make` on macOS and Linux.
I'm not sure if this will work out, but I intentionally removed my
hack to remove `sh.exe` (via `git` bash) from `%PATH%` and it caused
the following failure (which hints to use MSYS Makefiles)

```
...
CMake Error at C:/bld/libbezier_1581570932884/_build_env/Library/share/cmake-3.16/Modules/CMakeMinGWFindMake.cmake:12 (message):
  sh.exe was found in your PATH, here:

  C:/Program Files/Git/usr/bin/sh.exe

  For MinGW make to work correctly sh.exe must NOT be in your path.

  Run cmake from a shell that does not have sh.exe in your PATH.

  If you want to use a UNIX shell, then use MSYS Makefiles.

Call Stack (most recent call first):
  CMakeLists.txt:3 (project)

CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!
...
```

Also note this failed in the same way on both AppVeyor and Azure
pipelines.
Some reports (e.g. [2]) on the web indicate that it's easier just to
use "MinGW Makefiles" than to work with "MSYS Makefiles". An
`msys2` [1] mailing list indicates that their `mingw-cmake` should be
used.

[1]: https://sourceforge.net/p/msys2/mailman/msys2-users/?limit=250&page=5&style=flat
[2]: tpaviot/oce#578
Taking this approach [1] rather than directly modifying `%PATH%` for
"everyone".

[1]: https://cmake.org/cmake/help/latest/variable/CMAKE_IGNORE_PATH.html
This time for Azure pipelines.
See failure:

```
2020-02-13T07:16:09.6463110Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s: Assembler messages:
2020-02-13T07:16:09.6464067Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:60: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6464664Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:62: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6465122Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:64: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6465609Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:66: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6466065Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:68: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6466545Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:70: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6466977Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:72: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6467459Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:74: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6467940Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:76: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6468429Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:78: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6468858Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:80: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6469329Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:82: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6469770Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:84: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6470243Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:86: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6470676Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:88: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6471140Z C:\Users\VSSADM~1\AppData\Local\Temp\cc50mxtI.s:90: Error: invalid register for .seh_savexmm
2020-02-13T07:16:09.6522908Z mingw32-make.exe[2]: *** [quadpack/CMakeFiles/quadpack.dir/dqagse.f.obj] Error 1
```

and some mentions [1] [2] of a fix.

[1]: https://stackoverflow.com/a/43210527/1068170
[2]: https://sourceforge.net/p/mingw-w64/mailman/message/36287627/
@dhermes
Copy link
Contributor Author

dhermes commented Feb 13, 2020

@scopatz OK I think it's at a place we can be happy with (with the make / mingw dependencies explicitly called out)

@scopatz
Copy link
Member

scopatz commented Feb 13, 2020

I think we can go ahead and merge this, and fix up issues in the feedstock, if needed.

@scopatz scopatz merged commit b226c42 into conda-forge:master Feb 13, 2020
@dhermes dhermes deleted the add-libbezier branch February 13, 2020 16:41
@dhermes
Copy link
Contributor Author

dhermes commented Feb 13, 2020

Thanks for the review and for everything else you do for the community @scopatz!

Sorry for the n00b question but when can I expect https://github.com/conda-forge/libbezier-feedstock to exist?

Update: I see in https://github.com/conda-forge/staged-recipes/commits/master/recipes how it generally works.

@scopatz
Copy link
Member

scopatz commented Feb 13, 2020

It takes ~1 hr, if everything is working

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

Successfully merging this pull request may close these issues.

3 participants