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

Don't search system paths for Boost #4723

Closed
wants to merge 1 commit into from
Closed

Conversation

davisp
Copy link
Contributor

@davisp davisp commented Feb 13, 2024

Title says it all.


TYPE: NO_HISTORY
DESC: Don't search system paths for Boost

@@ -66,6 +66,7 @@ commence(object_library baseline)
pmr.cc
)
find_package(Spdlog_EP REQUIRED)
set(Boost_NO_SYSTEM_PATHS ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Beautiful.

Copy link
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

This unblocks the breakage seen before the PR and following the recent container changes.

@davisp
Copy link
Contributor Author

davisp commented Feb 13, 2024

I've opened [sc-40951] for tracking this.

Copy link

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

The root cause of the nightly failures is that we run them with vcpkg disabled and don't have an ExternalProject that downloads Boost if not found in the system.

I might be missing something but I don't see how this PR will fix it. If Boost will not be found from the system, where will it come from?

@teo-tsirpanis
Copy link
Member

@davisp
Copy link
Contributor Author

davisp commented Feb 13, 2024

@teo-tsirpanis We what now? This wasn't for nightlies, it was for avoiding using a system installed boost and forcing the use of the vcpkg installed version.

I had no idea we were still running ExternalProject builds so that'll take some doing to fix.

@teo-tsirpanis
Copy link
Member

Since we added Boost in vcpkg.json the vcpkg-provided Boost already takes precedence.

@davisp
Copy link
Contributor Author

davisp commented Feb 13, 2024

Since we added Boost in vcpkg.json the vcpkg-provided Boost already takes precedence.

It would if Boost ports provided CMake config files, but they do not. See this issue for more background: microsoft/vcpkg#23460

@eddelbuettel
Copy link
Contributor

Since we added Boost in vcpkg.json the vcpkg-provided Boost already takes precedence.

@teo-tsirpanis No. See the slack thread I started.

@teo-tsirpanis
Copy link
Member

Nightlies still fail.

@teo-tsirpanis
Copy link
Member

After looking at the documentation, Boost_NO_SYSTEM_PATHS means that CMake will only look at the paths specified by certain hint variables, which neither we nor vcpkg set. So we likely shouldn't merge this.

@davisp
Copy link
Contributor Author

davisp commented Feb 13, 2024

After looking at the documentation, Boost_NO_SYSTEM_PATHS means that CMake will only look at the paths specified by certain hint variables, which neither we nor vcpkg set. So we likely shouldn't merge this.

I saw that too. But... it works? If you know how to reliably get a path for ${VCPKG_INSTALLED_DIR}/${VCPKG_TRIPLET} to set for the BOOST_ROOT variable, I'd be more than happy to add it. But when I tried those they were empty and it was working regardless so I stopped caring at that point.

Also, my assumption is that its searching the vcpkg configured include/link directories which aren't system paths.

@teo-tsirpanis
Copy link
Member

I will more thoroughly look at it tomorrow.

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

Successfully merging this pull request may close these issues.

4 participants