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

feat: allow configuring WITH_SHARED_LIBRARY and WITHOUT_PLUGIN #940

Merged
merged 17 commits into from
Sep 15, 2023
Merged

feat: allow configuring WITH_SHARED_LIBRARY and WITHOUT_PLUGIN #940

merged 17 commits into from
Sep 15, 2023

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Sep 9, 2023 •
edited
Loading

Briefly, what does this PR introduce?

Allow to create shared instead of static algorithms libraries, and allow skipping the creation of an algorithms plugin.

Depends on:

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

@wdconinc
Copy link
Contributor Author

wdconinc commented Sep 9, 2023 •
edited
Loading

@veprbl Any idea why llvm-cov fails when we don't build the algorithms as plugins? The rest of the reconstruction doesn't use the algorithms as plugins but through the libraries. Resolved in f3353d5 and 2a7a9a9.

@wdconinc
Copy link
Contributor Author

wdconinc commented Sep 9, 2023

Btw, this is the solution to removing the duplicate objects.

@wdconinc wdconinc temporarily deployed to github-pages September 9, 2023 23:30 — with GitHub Actions Inactive
@wdconinc wdconinc temporarily deployed to github-pages September 10, 2023 02:42 — with GitHub Actions Inactive
@wdconinc wdconinc temporarily deployed to github-pages September 10, 2023 05:39 — with GitHub Actions Inactive
@github-actions github-actions bot added topic: tracking Relates to tracking reconstruction topic: PID Relates to PID reconstruction labels Sep 10, 2023
@wdconinc wdconinc marked this pull request as ready for review September 10, 2023 15:04
@wdconinc wdconinc temporarily deployed to github-pages September 10, 2023 17:38 — with GitHub Actions Inactive
@wdconinc wdconinc marked this pull request as draft September 10, 2023 22:17
@wdconinc wdconinc marked this pull request as ready for review September 11, 2023 00:44
@wdconinc
Copy link
Contributor Author

Comparison this branch (https://github.com/eic/EICrecon/actions/runs/6140234598/job/16658926482) vs main (https://github.com/eic/EICrecon/actions/runs/6138018645):

  • build gcc release duration: 24 min 46 sec vs 32 min 51 sec
  • build gcc release install artifact: 194 MB in 280 files vs 406 MB in 287 files (both after compression, 654 MB vs 1.63 GB before compression)

@wdconinc wdconinc temporarily deployed to github-pages September 11, 2023 02:02 — with GitHub Actions Inactive
@wdconinc wdconinc temporarily deployed to github-pages September 11, 2023 22:43 — with GitHub Actions Inactive
@wdconinc wdconinc requested a review from veprbl September 13, 2023 00:35
@wdconinc wdconinc temporarily deployed to github-pages September 13, 2023 02:24 — with GitHub Actions Inactive
@wdconinc wdconinc temporarily deployed to github-pages September 14, 2023 23:43 — with GitHub Actions Inactive
veprbl
veprbl approved these changes Sep 15, 2023
View reviewed changes
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

Compiles on macOS. Ship it.

@wdconinc wdconinc added this pull request to the merge queue Sep 15, 2023
Merged via the queue into main with commit 633c211 Sep 15, 2023
@wdconinc wdconinc deleted the shared-libraries branch September 15, 2023 02:06
This was referenced Sep 15, 2023
Closed
Closed
@veprbl veprbl mentioned this pull request Sep 24, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 25, 2023
@veprbl
Revert "CKFTracking: disable RTTI type checks until EICrecon conforms…
5c987a1 
… to ODR (#551)" (#1015)

I don't have a proof of ODR compliance at this time, but, at least, this
issue was recently resolved (likely by #940).

This reverts commit f375494.

Closes: #340
@wdconinc wdconinc mentioned this pull request Sep 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veprbl veprbl veprbl approved these changes

Assignees
No one assigned
Labels
topic: calorimetry relates to calorimetry topic: infrastructure topic: PID Relates to PID reconstruction topic: tracking Relates to tracking reconstruction
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wdconinc @veprbl