Skip to content

Fix vs2017 build - #61

Merged
jnbrunet merged 7 commits into
sofa-framework:masterfrom
guparan:fix_vs2017_build
Nov 6, 2020
Merged

Fix vs2017 build#61
jnbrunet merged 7 commits into
sofa-framework:masterfrom
guparan:fix_vs2017_build

Conversation

@guparan

@guparan guparan commented Oct 28, 2020

Copy link
Copy Markdown
Contributor

This is a bunch of changes I had to do to be able to configure and build SofaPython3 on my machine.

I am not sure about the last commit. What is the point of install(EXPORT ... on tests?

@jnbrunet

Copy link
Copy Markdown
Contributor

What is the point of install(EXPORT ... on tests?

None I guess, probably a bad copy/paste on my part. The only important thing is the install(DIRECTORY ...) as we package those for the CI (tests are done on an installed SP3, not compiled).

@jnbrunet

jnbrunet commented Oct 29, 2020

Copy link
Copy Markdown
Contributor

Hum, the CI cannot find the tests anymore. I guess you do need the install(EXPORT ...).

Edit:
@guparan , we need to install the test executable. They are part of the installed package and will be used by the CI to run the test (the CI is a plain VM that installs SP3 and run the tests from there)

Comment thread CMakeLists.txt Outdated

if(MSVC)
# define BOOST_ALL_DYN_LINK needed for dynamic linking with boost libraries
add_definitions(-DBOOST_ALL_DYN_LINK)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not needed anymore, see sofa-framework/sofa#1578

@guparan

guparan commented Nov 3, 2020

Copy link
Copy Markdown
Contributor Author

Test failures are not my fault anymore! 👼

@guparan
guparan requested a review from jnbrunet November 3, 2020 08:40
@guparan

guparan commented Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

Please merge me! 🙏

@jnbrunet
jnbrunet merged commit 69de28b into sofa-framework:master Nov 6, 2020
@guparan
guparan deleted the fix_vs2017_build branch November 18, 2021 14:20
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.

2 participants