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.

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

ARROW-16993: [C++] cmake: cannot create imported target "Boost::headers" #13845

Closed
wants to merge 5 commits into from

Conversation

assignUser
Copy link
Member

No description provided.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@github-actions
Copy link

Revision: 57bdb36

Submitted crossbow builds: ursacomputing/crossbow @ actions-28f9432361

Task Status
r-binary-packages Github Actions
test-build-cpp-fuzz Github Actions
test-conda-cpp Github Actions
test-conda-cpp-valgrind Azure
test-debian-10-cpp-amd64 Github Actions
test-debian-10-cpp-i386 Github Actions
test-debian-11-cpp-amd64 Github Actions
test-debian-11-cpp-i386 Github Actions
test-fedora-35-cpp Github Actions
test-ubuntu-18.04-cpp Github Actions
test-ubuntu-18.04-cpp-release Github Actions
test-ubuntu-18.04-cpp-static Github Actions
test-ubuntu-20.04-cpp Github Actions
test-ubuntu-20.04-cpp-14 Github Actions
test-ubuntu-20.04-cpp-17 Github Actions
test-ubuntu-20.04-cpp-bundled Github Actions
test-ubuntu-20.04-cpp-thread-sanitizer Github Actions
test-ubuntu-22.04-cpp Github Actions

COMPONENTS
system
filesystem
${ARROW_BOOST_COMPONENTS}
Copy link
Member

Choose a reason for hiding this comment

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

We can specify empty COMPONENTS like #13846.

Copy link
Member Author

Choose a reason for hiding this comment

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

What is the advantage of your approach? The COMPONENTS arg completely missing or being empty has the same effect as if(ARG_COMPONENTS) will be false.

Copy link
Member Author

Choose a reason for hiding this comment

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

(also sorry for just opening a PR for a ticket you are assigned to. This issue was preventing us from submitting the R package to CRAN so we needed a fix fast and wanted to merge it back now)

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Both approaches have the same effect.
I think that the empty COMPONENTS approach is easiar to read. Putting the COMPONENTS keyword by a variable expansion may be a bit tricky.

If you like your approach, could you use ARROW_BOOST_COMPONENTS_ARGS or something instead of ARROW_BOOST_COMPONENTS for the variable name? Because the COMPONENTS keyword isn't a component.

Copy link
Member

Choose a reason for hiding this comment

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

Like @kou I would find COMPONENTS ${ARROW_BOOST_COMPONENTS} to be slightly more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok than let's go with @kou s version in #13846

COMPONENTS
system
filesystem
${ARROW_BOOST_COMPONENTS}
Copy link
Member

Choose a reason for hiding this comment

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

Yes. Both approaches have the same effect.
I think that the empty COMPONENTS approach is easiar to read. Putting the COMPONENTS keyword by a variable expansion may be a bit tricky.

If you like your approach, could you use ARROW_BOOST_COMPONENTS_ARGS or something instead of ARROW_BOOST_COMPONENTS for the variable name? Because the COMPONENTS keyword isn't a component.

cpp/cmake_modules/ThirdpartyToolchain.cmake Outdated Show resolved Hide resolved
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@assignUser
Copy link
Member Author

@github-actions crossbow submit -g cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants