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

CMake plugin template (used by cookiecutter): add NOVA_SIMD build option and include boost. #4354

Merged

Conversation

mtmccrea
Copy link
Member

@mtmccrea mtmccrea commented Mar 10, 2019

Purpose and Motivation

The cookiecutter plugin build workflow doesn't currently support nova-simd or include the boost library.

Types of changes

  • New feature
    This adds the NOVA_SIMD build option and includes the boost library by default so it's available to the cookiecutter plugin build workflow.

Note: The default DNOVA_SIMD flag is ON.

This gets the job done for making these libraries available to plugins, but I'm open to other options for optionally including these.

  • Code is tested
    • plugin built using both libraries
  • All tests are passing
    • SC built just fine
  • Updated documentation
    • I didn't see any build flag documentation in the cookiecutter repo to update.
  • This PR is ready for review

@mtmccrea
Copy link
Member Author

Just wanted to ping this PR to see if there were any thoughts on these changes, alternative approaches, etc. I'd be happy to follow up with those suggestions.

Otherwise, it'd be great to see a milestone tag pop up on this one ;)

@nhthn
Copy link
Contributor

nhthn commented Apr 13, 2019

hi mike! right now we can't merge anything to 3.10 or develop branches because there is a big code reformat under way.

@mtmccrea
Copy link
Member Author

mtmccrea commented Apr 13, 2019

Thanks, Nathan. No problem, just wanted to see if there had been any eyeballs on this, through comments or a milestone assignment, so I could address any suggestions for changes. Thanks for checking in!

@nhthn nhthn added the comp: build CMake build system label Jun 2, 2019
@nhthn nhthn added this to the 3.11 milestone Jun 2, 2019
@nhthn nhthn self-assigned this Jun 8, 2019
@mossheim
Copy link
Contributor

@mtmccrea Thanks for the PR! I'm just looking at this now. I am OK with the nova-simd option but IMO people should include Boost as a dependency themselves if they need it, and they should also IMO use packaged boost rather than relying on the one in SC, which is first of all not a full copy of boost, second of all tailored for SC's purposes, and third of all updated on its own schedule.

Is there something specific you need it for? I can add a note in the README for the cookiecutter repo on how to include boost if that would satisfy you.

Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

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

per comment above

@mtmccrea
Copy link
Member Author

@brianlheim sorry to have let this sit for so long. I think it's reasonable to have people include boost only if they need it. I've removed that include. And sure I think it would be helpful to add a note in the docs about including external libraries. Thanks!

@mossheim
Copy link
Contributor

@mtmccrea no need to apologize. thanks for updating!

@mossheim mossheim merged commit 5fcf876 into supercollider:develop Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: build CMake build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants