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

Watchman fails to build with fmt 8.1.0+ #2717

Closed
carlocab opened this issue Jan 14, 2022 · 8 comments
Closed

Watchman fails to build with fmt 8.1.0+ #2717

carlocab opened this issue Jan 14, 2022 · 8 comments

Comments

@carlocab
Copy link

carlocab commented Jan 14, 2022

Hi there, we're trying to update Homebrew's version of Watchman at Homebrew/homebrew-core#92869. We were last able to build Watchman against fmt 8.0.1, and it now no longer builds since Homebrew's fmt was updated to 8.1.0. The failure occurs on multiple versions of macOS and on Linux.

The Watchman build fails with this error:

  In file included from /tmp/watchman-20220110-77236-142y8ko/watchman-2022.01.10.00/watchman/PendingCollection.cpp:8:
  In file included from /tmp/watchman-20220110-77236-142y8ko/watchman-2022.01.10.00/watchman/PendingCollection.h:11:
  In file included from /opt/homebrew/include/folly/futures/Promise.h:23:
  In file included from /opt/homebrew/include/folly/futures/detail/Core.h:27:
  In file included from /opt/homebrew/include/folly/Executor.h:25:
  In file included from /opt/homebrew/include/folly/Range.h:45:
  In file included from /opt/homebrew/include/fmt/format.h:48:
  /opt/homebrew/include/fmt/core.h:1722:3: error: static_assert failed due to requirement 'formattable_pointer' "Formatting of non-void pointers is disallowed."
    static_assert(formattable_pointer,
    ^             ~~~~~~~~~~~~~~~~~~~

Complete build logs available at https://github.com/Homebrew/homebrew-core/actions/runs/1680135847.

I tried to follow the call stack (see the log after the snippet I pasted above), and it doesn't look to me that Watchman is passing any pointers at all to fmt. However, I don't really do C++, and I'm unfamiliar with Watchman's codebase, so I could be mistaken.

Any assistance trying to resolve this would be appreciated. Thank you!

@vitaut
Copy link
Contributor

vitaut commented Jan 14, 2022

I'll look into it, thanks for reporting.

@vitaut
Copy link
Contributor

vitaut commented Jan 14, 2022

The problem is that w_string is implicitly convertible to a pointer which is caught by a new diagnostic (compile-time error) introduced in {fmt} 8.1:

Added missing diagnostic when trying to format function and member pointers as well as objects convertible to pointers which is explicitly disallowed

Fixed in 8f8a1a0 by only giving the diagnostic if there is no formatter specialization.

@vitaut vitaut closed this as completed Jan 14, 2022
carlocab added a commit to chenrui333/homebrew-core that referenced this issue Jan 15, 2022
@carlocab
Copy link
Author

Thanks for the lightning-quick fix. I've confirmed that it works.

BrewTestBot pushed a commit to Homebrew/homebrew-core that referenced this issue Jan 15, 2022
See fmtlib/fmt#2717.

Closes #92869.

Signed-off-by: Carlo Cabrera <[email protected]>
Signed-off-by: BrewTestBot <[email protected]>
@gh-andre
Copy link

gh-andre commented Feb 8, 2022

@vitaut I just ran into the same bug. When this is planned to be released? If it's going to be some time, would you mind pointing out which commits can be used as a patch for 8.1.0 or 8.1.1 in the meantime? Thanks.

@carlocab
Copy link
Author

carlocab commented Feb 8, 2022

8f8a1a0 (mentioned above) applies cleanly to at least 8.1.1.

@gh-andre
Copy link

gh-andre commented Feb 8, 2022

@carlocab Thanks. There's a lot of commits around I was hoping to hear from the maintainers what would work well for a patch until the next release. It would be nice if an 8.1 branch was maintained for these patch fixes.

@vitaut
Copy link
Contributor

vitaut commented Feb 8, 2022

8f8a1a0 is pretty self-contained. It will also be included in the next release but I don't have any specific dates yet.

@gh-andre
Copy link

gh-andre commented Feb 8, 2022

@vitaut Sounds good. Thanks for confirming.

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

No branches or pull requests

3 participants