8000 Prepare for Disabling MbsAPI by ChristianTackeGSI · Pull Request #1056 · FairRootGroup/FairRoot · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Prepare for Disa 8000 bling MbsAPI #1056

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
Merged
merged 5 commits into from
Apr 20, 2021
Merged

Prepare for Disabling MbsAPI #1056

merged 5 commits into from
Apr 20, 2021

Conversation

ChristianTackeGSI
Copy link
Member

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
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
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!

@dennisklein
Copy link
Member

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
Member

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

What breaks?

@ChristianTackeGSI
Copy link
Member Author

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
0