Skip to content

ASoA: Include TBufferFile.h - #15588

Closed
vkucera wants to merge 1 commit into
AliceO2Group:devfrom
vkucera:iwyu
Closed

ASoA: Include TBufferFile.h#15588
vkucera wants to merge 1 commit into
AliceO2Group:devfrom
vkucera:iwyu

Conversation

@vkucera

@vkucera vkucera commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

TBufferFile.h is needed by DECLARE_SOA_CCDB_COLUMN_FULL.
That's why Common/DataModel/GloCCDBObjects.h in O2Physics currently cannot be compiled.

@vkucera
vkucera requested a review from a team as a code owner July 7, 2026 17:14
@ktf ktf closed this Jul 21, 2026
@ktf

ktf commented Jul 21, 2026

Copy link
Copy Markdown
Member

The "fix" is wrong. As usual one should not blindly run IWYU, but do a proper review of the headers. For example, StringHelpers.h should probably export string and string_view.

@ktf

ktf commented Jul 21, 2026

Copy link
Copy Markdown
Member

Also, I am not sure this is actually the highest priority we have, if at all.

@vkucera

vkucera commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

The "fix" is wrong. As usual one should not blindly run IWYU, but do a proper review of the headers. For example, StringHelpers.h should probably export string and string_view.

What exactly is wrong?

@vkucera

vkucera commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Also, I am not sure this is actually the highest priority we have, if at all.

The header does not compile which prevents other headers that include it from compiling too. I would say it's quite important.

@ktf

ktf commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Log, please.

@ktf

ktf commented Jul 21, 2026

Copy link
Copy Markdown
Member

"wrong" as in "pollutes ASoA.h with unneeded include statements which should be considered exported by other headers. See the StringHelpers.h example".

@vkucera

vkucera commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

"wrong" as in "pollutes ASoA.h with unneeded include statements which should be considered exported by other headers. See the StringHelpers.h example".

OK. How about just adding TBufferFile.h for now?

@vkucera vkucera reopened this Sep 22, 2026
@vkucera
vkucera marked this pull request as draft September 22, 2026 15:09
@vkucera
vkucera marked this pull request as ready for review September 22, 2026 15:16
@vkucera vkucera changed the title ASoA: Fix includes ASoA: Include TBufferFile.h Sep 22, 2026
@alibuild

alibuild commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI_slc9 for a3ca11e at 2026-09-22 19:34:

No log files found

Full log here.

ktf
ktf reviewed Sep 30, 2026
View reviewed changes
Comment thread Framework/Core/include/Framework/ASoA.h
#include "Framework/SliceCache.h" // IWYU pragma: export
#include "Framework/VariantHelpers.h" // IWYU pragma: export

#include <TBufferFile.h> // IWYU pragma: keep (needed by DECLARE_SOA_CCDB_COLUMN_FULL)

@ktf ktf Sep 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, we should fix the including header, not this one. ASoA should not depend on root.

@vkucera vkucera Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in AliceO2Group/O2Physics#18129

@vkucera vkucera closed this Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ktf ktf ktf left review comments

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vkucera @ktf @alibuild