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

[fmt] Fix visibility #35793

Merged
merged 2 commits into from
Feb 27, 2024
Merged

[fmt] Fix visibility #35793

merged 2 commits into from
Feb 27, 2024

Conversation

xiaozhuai
Copy link
Contributor

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.
ld: warning: direct access in function 'fmt::v10::detail::format_error_code(fmt::v10::detail::buffer<char>&, int, fmt::v10::basic_string_view<char>)' from file '/usr/local/vcpkg/installed/x64-osx/lib/libfmt.a(format.cc.o)' to global weak symbol 'decltype(fp.begin()) fmt::v10::detail::parse_format_specs<int, fmt::v10::detail::compile_parse_context<char> >(fmt::v10::detail::compile_parse_context<char>&)' from file '/usr/local/vcpkg/installed/x64-osx/lib/libspdlog.a(spdlog.cpp.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

When link with spdlog, it shows this warning.

This problem was once reported in #29280 by @marcovc

And this pr fix it.

@BillyONeal
Copy link
Member

Has this been reported upstream? /cc @vitaut

@LilyWangLL LilyWangLL added the category:port-bug The issue is with a library, which is something the port should already support label Dec 21, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Dec 21, 2023

AFAIU #29280 points also at spdlog.
Is it right fix to generally deactivates fmt's visibility defaults?
And the patch is deactivating input (cache) variables. Is a patch needed at all?

@LilyWangLL LilyWangLL added the depends:upstream-changes Waiting on a change to the upstream project label Dec 22, 2023
@LilyWangLL LilyWangLL marked this pull request as draft December 22, 2023 08:48
@xiaozhuai
Copy link
Contributor Author

See fmtlib/fmt#3769 and fmtlib/fmt@f68f452

@xiaozhuai xiaozhuai marked this pull request as ready for review February 26, 2024 04:23
@xiaozhuai
Copy link
Contributor Author

@LilyWangLL @BillyONeal

@LilyWangLL LilyWangLL added info:reviewed Pull Request changes follow basic guidelines and removed depends:upstream-changes Waiting on a change to the upstream project info:reviewed Pull Request changes follow basic guidelines labels Feb 26, 2024
@LilyWangLL
Copy link
Contributor

@xiaozhuai Please fix the CI pipeline error: arrayfire:x64-osx.

CMake Error at CMakeModules/AFconfigure_forge_submodule.cmake:47 (configure_file):
  No such file or directory
Call Stack (most recent call first):
  CMakeLists.txt:115 (include)

@LilyWangLL LilyWangLL marked this pull request as draft February 27, 2024 02:45
@xiaozhuai
Copy link
Contributor Author

Please fix the CI pipeline error: arrayfire:x64-osx.

@LilyWangLL It seem nothing to do with this PR. See #33540

@LilyWangLL
Copy link
Contributor

Please fix the CI pipeline error: arrayfire:x64-osx.

@LilyWangLL It seem nothing to do with this PR. See #33540

Thanks for your reply, this error should add DISABLE_PARALLEL_CONFIGURE for arrayfire, PR #33540 has added this.

@xiaozhuai xiaozhuai marked this pull request as ready for review February 27, 2024 08:04
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Feb 27, 2024
@vicroms vicroms merged commit c764ada into microsoft:master Feb 27, 2024
16 checks passed
@xiaozhuai xiaozhuai deleted the dev-fmt branch March 4, 2024 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants