Skip to content

Add cotire#897

Merged
pelson merged 1 commit intoconda-forge:masterfrom
jakirkham:add_cotire
Jun 30, 2016
Merged

Add cotire#897
pelson merged 1 commit intoconda-forge:masterfrom
jakirkham:add_cotire

Conversation

@jakirkham
Copy link
Copy Markdown
Member

@jakirkham jakirkham commented Jun 28, 2016

Adds a recipe for cotire (written from scratch). This is useful for getting a unity build up and running quickly. May be useful to have this around for some of our longer builds.

cc @jschueller @frol @ivoflipse @Korijn @patricksnape

@conda-forge-linter
Copy link
Copy Markdown

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/cotire) and found it was in an excellent condition.

@jakirkham
Copy link
Copy Markdown
Member Author

Feedback and maintainers welcome.

@jakirkham jakirkham force-pushed the add_cotire branch 3 times, most recently from dd39512 to 96f4e62 Compare June 28, 2016 01:04
@msarahan
Copy link
Copy Markdown
Member

Looks useful. I probably don't need to be a maintainer, but I'll back you up if you want. The recipe looks fine to me - as long as the CIs come back OK.

number: 0
script:
- cp "${SRC_DIR}/CMake/cotire.cmake" "${PREFIX}"/share/cmake*/Modules # [unix]
- copy "%SRC_DIR%\\CMake\\cotire.cmake" "%LIBRARY_PREFIX%"\\share\\cmake*\\Modules # [win]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Am I doing something wrong here, @msarahan? It seem to have issues copying this on Windows. I really don't know the batch syntax.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe the quotes are causing this issues. Will try dropping them.

@jakirkham
Copy link
Copy Markdown
Member Author

No worries. I'm guessing others might get excited about this.

For the most part, maintaining this is mind-numbingly simple so I'm hoping its just not a big deal.

commands:
# Verify that the cotire CMake file is in place.
- test -f "${PREFIX}"/share/cmake*/Modules/cotire.cmake # [unix]
- if not exist %LIBRARY_PREFIX%\\share\\cmake*\\Modules\\cotire.cmake exit 1 # [win]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ugh...Windows doesn't like the wildcard here. Any suggestions?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

NVM guess I can cd here too.

@jakirkham jakirkham force-pushed the add_cotire branch 3 times, most recently from 6d21e75 to 67cd95e Compare June 28, 2016 02:06
- cmake -G "Unix Makefiles" .. # [unix]
- cmake -G "NMake Makefiles" .. # [win]
# Cannot find `cl` on Windows during tests.
- cmake --build . # [unix]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Have to skip on Windows. See this issue ( conda/conda-build#1059 ).

# https://github.com/conda/conda-build/issues/1059
#
#- cmake -G "NMake Makefiles" .. # [win]
- cmake --build . # [unix]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Had to skip building on Windows. Would be a nice test to include on all platforms, but it seems a bit challenging to get Windows to work. See this issue ( conda/conda-build#1059 ).

@jakirkham jakirkham changed the title WIP: Add cotire Add cotire Jun 28, 2016
@jakirkham
Copy link
Copy Markdown
Member Author

This is now passing. 😄

@Korijn
Copy link
Copy Markdown
Contributor

Korijn commented Jun 28, 2016

Awesome. Shame about the Windows testing bug. Anyway, LGTM!

@jschueller
Copy link
Copy Markdown
Contributor

Nice, I think this kind of things tends to be included in projects though rather than used as an optional client library. At least that's what we did for openturns. Or maybe your approcah is to patch the target package yourself to use cotire ?

@jakirkham
Copy link
Copy Markdown
Member Author

Yeah, I figured it might make it easier for people to play with. Also, this way it can be versioned separately, which should reduce some maintenance burden on devs (or that was my thinking at least).

@pelson pelson merged commit 32c089b into conda-forge:master Jun 30, 2016
@jakirkham jakirkham deleted the add_cotire branch July 2, 2016 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants