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

Addition of cmake flag FETCHCONTENT_FULLY_DISCONNECTED=ON breaks formulas #17187

Open
3 tasks done
jackjansen opened this issue Apr 30, 2024 · 5 comments
Open
3 tasks done
Labels
bug Reproducible Homebrew/brew bug

Comments

@jackjansen
Copy link

brew doctor output

Your system is ready to brew.

Verification

  • My "brew doctor output" above says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update twice and am still able to reproduce my issue.
  • This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

brew config output

HOMEBREW_VERSION: 4.2.20-26-g45e0ce0
ORIGIN: https://github.com/Homebrew/brew
HEAD: 45e0ce0635f3a98e7f7b797c7953cc5ab0cce1c7
Last commit: 19 minutes ago
Core tap HEAD: ec19424c2a357cfeb1d277812058371e049db6bc
Core tap last commit: 44 minutes ago
Core tap JSON: 30 Apr 11:06 UTC
Core cask tap HEAD: 531029e3b0a4f22c73c7bac95d17aa832d895cfd
Core cask tap last commit: 3 hours ago
Core cask tap JSON: 30 Apr 11:06 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 8
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.1.4 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.1.4/bin/ruby
CPU: octa-core 64-bit arm_firestorm_icestorm
Clang: 15.0.0 build 1500
Git: 2.45.0 => /opt/homebrew/bin/git
Curl: 8.4.0 => /usr/bin/curl
macOS: 14.4.1-arm64
CLT: 15.3.0.0.1.1708646388
Xcode: 15.3
Rosetta 2: false

What were you trying to do (and why)?

Trying to build a formula with --HEAD that worked until a few weeks ago. The formula is a tap, cwi-dis/cwipc, but I'm pretty convinced that many other formulae will run into this problem.

What happened (include all command output)?

It did not build. One of the cmake dependencies, nlohmann_json::nlohman_json, was not found. The CMakeLists.txt of the cwipc repo uses the FetchContent macro to obtain the nlohmann_json package (which is a header-file-only package, and FetchContent is the preferred way of including it in your build.

Since the merge of #17075 brew will disable downloading of additional content by FetchContent. I fully understand the reasoning behind this merge (and I even somewhat agree:-), but as of this merge I presume many formulae have been broken, and moreover there is no clear error message that the formula maintainer gets that indicates what the problem is.

Unfortunately FetchContent() does not give an error or warning message if it has to download a package but is not allowed to do so. So the errors occur only much later in the build process, when the package is used but found to be missing.

What did you expect to happen?

Build should work.

Step-by-step reproduction instructions (by running brew commands)

brew tap cwi-dis/cwipc
brew install --HEAD cwipc
@carlocab
Copy link
Member

I'm open to a PR that restricts this flag only to Homebrew/core. I'd prefer that this be our last resort, though.

However, in case you didn't already know, you can override the flag by passing -DFETCHCONTENT_FULLY_DISCONNECTED=OFF after std_cmake_args. (I suspect this is already what you are doing, but I want to make sure!)

I also agree that the resulting cmake error is very difficult to understand. I think it's worth reporting this to CMake so it can be improved.

@jackjansen
Copy link
Author

@carlocab agreed on all points. I think the change is in principle a good idea, I will change my repo to not use FetchContent when building with brew, in stead adding a dependency.

I've opened https://gitlab.kitware.com/cmake/cmake/-/issues/25946

@jackjansen
Copy link
Author

For reference to others coming across this issue in the future: for my formula the solution was to first try to find the package with a normal find_package(nlohmann_json) and only use FetchContent if that couldn't find it.

And then adding a depends_on to my formula.

Can be closed for me, but I'll leave that to @carlocab in case you want to keep it open.

@craigscott-crascit
Copy link

I've responded with a comment on the PR that added FETCHCONTENT_FULLY_DISCONNECTED. I recommend reverting that change, it is an abuse of that option and it violates one of the documented preconditions for using it.

@alebcay
Copy link
Member

alebcay commented May 18, 2024

This should be addressed (partly) via #17310 - the intention is still to block FetchContent usage by default but we no longer rely on FETCHCONTENT_FULLY_DISCONNECTED. Add -DHOMEBREW_ALLOW_FETCHCONTENT=ON to explicitly allow FetchContent usage.

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

No branches or pull requests

4 participants