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.

Sign up for GitHub

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

Jump to bottom

[SofaSparseSolver][SofaPreconditioner] modularization #668

Merged
merged 19 commits into from
Jul 20, 2018
Merged

[SofaSparseSolver][SofaPreconditioner] modularization #668

merged 19 commits into from
Jul 20, 2018

Conversation

damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented May 28, 2018 •
edited by guparan
Loading

As said in the title this PR puts SofaSparseSolver and SofaPreconditioner as a real module that can be activated/deactivated.

CHANGLELOG/

  • move source files to src/SofaSparseSolver & src/SofaPreconditioner so the .h are not leaked anymore.
  • conditional code based on #define SOFA_HAVE_METIS/SOFA_HAVE_CSPARSE is now changed to favor the use of find_package to detect and activate conditional building depending on dependencies.
  • add a modules/CMakeLists.txt to integrate that
  • add the touched component into the list of 'moved' component

This PR:

  • builds with SUCCESS for all platforms on the CI.
  • does not generate new warnings.
  • does not generate new unit test failures.
  • does not generate new scene test failures.
  • does not break API compatibility.
  • is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

@damienmarchal
[SofaPreconditioner] MOVE files in the src directory.
68dbf5f 
Using code directly in the root of the a module/plugin has the dis-advantage of
of leaking the .h too easily.

Eg:
   -I"modules" or worse ("-I../") allows to include any modules that is into the directory without needs to
    have a corresponding find_package(SofaPreconditioner) in the CMakeLists.txt
@damienmarchal
Update the CMakeLists.txt
bd9d5c5
@damienmarchal
[SofaSparseSolver] MOVE files to the 'src/SofaSparseSolver' subdirect…
2b9a8ab 
…ory.

Not because it is fun, but because it improves the packaging of the module
by preventing user of the module to do "-Imodules" then #include<SofaSparseSolver/xx>
without having the corresponding find_package(SofaSparseSolver)

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>
@damienmarchal
SofaSparseSolver update plugin.
468558d
@damienmarchal
[SofaPreconditionner] Make a plugin
6f0e6c6
@damienmarchal
[SofaPreconditioner] Update CMakeLists.txt to make the module a real …
bad1f53 
…one.
@damienmarchal
[SofaSparseSolver] UPDATE The code to make it a real module.
125ccc2 
RENAMES in the defines
 - SOFA_SPARSE_SOLVER_API -> SOFA_SOFASPARSESOLVER_API
 - SOFA_BUILD_SPARSE_SOLVER -> SOFA_BUILD_SOFASPARSESOLVER
 - SOFA_HAVE_SPARSESOLVER -> SOFA_HAVE_SOFASPARSESOLVER
@damienmarchal
[all] CHANGE to taking into account modularization of SofaSparseSolve…
9809b4e 
…r & SofaPreconditioner
@damienmarchal
[SofaKernel] ADD components that have been putted into a module.
2cee9fa
@damienmarchal
[all] UPDATE dependencies of SofaPreconditioner & SofaSparseSolver to…
bcd1207 
… properly integrate the module.
@damienmarchal
[SofaSparseSolver] move & rename the example CSPARSE_SparseCholeskySo…
d2675f4 
…lver.scn
@damienmarchal
[SofaSparseSolver] ADD other examples (to be used as tests)
5d44066
@damienmarchal
[SofaBoundaryCondition] FIX test that was depending on SparseLDLSolver
f8b729b
@guparan guparan self-requested a review May 28, 2018 15:16
@guparan guparan added project: Sofa NG pr: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels May 28, 2018
@damienmarchal
Copy link
Contributor Author

[ci-build][with-scene-tests]

@damienmarchal
Copy link
Contributor Author

@guparan, @hugtalbot Could we considering progressing on those...they are now wainting for review since weeks :)

@guparan
Copy link
Contributor

guparan commented Jun 26, 2018 •
edited
Loading

I'm on it 😉

@guparan
Copy link
Contributor

guparan commented Jun 26, 2018

[ci-build][with-scene-tests]

@guparan guparan added the pr: breaking Change possibly inducing a compilation error label Jun 26, 2018
@guparan
Copy link
Contributor

guparan commented Jun 26, 2018

Nice work @damienmarchal, this PR is approved.
Please have a quick look at my few changes.
I set the "breaking" label since every code linking to those two modules will have to add the appropriate find_package.

@guparan
Copy link
Contributor

guparan commented Jun 26, 2018

[ci-build][with-scene-tests]

@hugtalbot hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Jun 27, 2018
@guparan guparan mentioned this pull request Jul 2, 2018
Closed
@guparan guparan added this to the v18.12 milestone Jul 4, 2018
@guparan
Copy link
Contributor

guparan commented Jul 18, 2018

TODO: resolve conflict then merge
Do not forget to add "every code linking to those two modules will have to add the appropriate find_package" to commit message.

@epernod
Copy link
Contributor

epernod commented Jul 19, 2018

[ci-build][with-scene-tests]

1 similar comment
@guparan
Copy link
Contributor

guparan commented Jul 19, 2018

[ci-build][with-scene-tests]

@guparan guparan changed the title [SofaSparseSolver& SofaPreconditioner] modularization [SofaSparseSolver & SofaPreconditioner] modularization Jul 20, 2018
@guparan guparan changed the title [SofaSparseSolver & SofaPreconditioner] modularization [SofaSparseSolver][SofaPreconditioner] modularization Jul 20, 2018
@guparan guparan merged commit 18bb092 into sofa-framework:master Jul 20, 2018
@damienmarchal damienmarchal deleted the Sofa_modularize_SparseSolverPR branch August 6, 2018 15:07
@guparan guparan mentioned this pull request Oct 9, 2020
Closed
@guparan guparan added the NG2: pluginization See https://github.com/sofa-framework/sofa/issues/1527 label Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guparan guparan guparan approved these changes

Assignees
No one assigned
Labels
NG2: pluginization See https://github.com/sofa-framework/sofa/issues/1527 pr: breaking Change possibly inducing a compilation error pr: clean Cleaning the code pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Milestone
v18.12
Development

Successfully merging this pull request may close these issues.
4 participants
@damienmarchal @guparan @epernod @hugtalbot