Skip to content

Prepare for Disabling MbsAPI#1056

Merged
dennisklein merged 5 commits intoFairRootGroup:devfrom
ChristianTackeGSI:pr/disable_mbs
Apr 20, 2021
Merged

Prepare for Disabling MbsAPI#1056
dennisklein merged 5 commits intoFairRootGroup:devfrom
ChristianTackeGSI:pr/disable_mbs

Conversation

@ChristianTackeGSI
Copy link
Copy Markdown
Member

@ChristianTackeGSI ChristianTackeGSI commented Apr 19, 2021

We want to remove MbsAPI.
So let's deprecate it first.

  • A BUILD_MBS option already exists.
  • Make sure, that setting it to OFF actually works.
  • Do not yet set it to OFF
  • Add libdl to MCStepLogger
  • Move myself from CONTRIBUTORS to AUTHORS

Checklist:

@ChristianTackeGSI
Copy link
Copy Markdown
Member Author

ChristianTackeGSI commented Apr 19, 2021 •
edited
Loading

I have left out the "set BUILD_MBS default to OFF" for now.
I can add that commit soon here, or create a new PR after this one is merged.
What do you prefer?

Oh, and please add anyone in, who might be interested.

@dennisklein dennisklein added this to the v19.0 milestone Apr 19, 2021
dennisklein
dennisklein requested changes Apr 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dennisklein dennisklein left a comment

Choose a reason for hiding this comment

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

See comment, rest looks good!

Comment thread CMakeLists.txt
@dennisklein
Copy link
Copy Markdown
Member

dennisklein commented Apr 20, 2021

I have left out the "set BUILD_MBS default to OFF" for now.
I can add that commit soon here, or create a new PR after this one is merged.
What do you prefer?

Oh, and please add anyone in, who might be interested.

ah, yes, please set the default to OFF already.

@dennisklein
Copy link
Copy Markdown
Member

dennisklein commented Apr 20, 2021

An option BUILD_MBS is already available.
Setting it to OFF breaks the build currently.

What breaks?

@ChristianTackeGSI
Copy link
Copy Markdown
Member Author

ChristianTackeGSI commented Apr 20, 2021

What breaks?

Phrased badly.
Better: "Without this PR".

So without this PR doing a -DBUILD_MBS=OFF did not work. Basically, because the examples were building, but Mbs did not.

That's why I added a bunch of more if(BUILD_MBS) around the place. To stop building any depenendencies of MBS.

@ChristianTackeGSI
FairMCStepLogger needs CMAKE_DL_LIBS
5bfbb36 
FairMCStepLogger uses dlopen().
So let's link with ${CMAKE_DL_LIBS}.
@ChristianTackeGSI
Prepare to disable MbsApi
0ac9e37 
An option BUILD_MBS is already available.
Setting it to OFF breaks the build currently.
So let's fix all those things.

But still keep the default at ON to make sure, that
compiling with BUILD_MBS=ON works now, at least.
@ChristianTackeGSI
Move myself from CONTRIBUTORS to AUTHORS
eaaa311
@ChristianTackeGSI
Disable MbsAPI
c3745b0 
We do not want to support MbsAPI any more.
We are deprecating it and thus disable it by default.
@ChristianTackeGSI
Re-enable MBS in the CI builds.
bf7cc53 
Running the build without MBS basically checks the build
system settings match up. This is the common case, and
every developer/etc will test that for us anyway (including
the FairSoft CI builds).

So until we remove MBS completely, let's still build it
during the CI builds.
@dennisklein dennisklein merged commit aa29bf0 into FairRootGroup:dev Apr 20, 2021
@ChristianTackeGSI ChristianTackeGSI deleted the pr/disable_mbs branch April 20, 2021 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dennisklein dennisklein dennisklein approved these changes

@MohammadAlTurany MohammadAlTurany Awaiting requested review from MohammadAlTurany

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v19.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ChristianTackeGSI @dennisklein