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

feat: add boost.detail #2580

Merged
merged 4 commits into from
Aug 14, 2024
Merged

feat: add boost.detail #2580

merged 4 commits into from
Aug 14, 2024

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Aug 10, 2024

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (boost.detail) have been updated in this PR. Please review the changes.

@fmeum fmeum added presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR labels Aug 10, 2024
@Vertexwahn
Copy link
Contributor

Vertexwahn commented Aug 10, 2024

@fmeum @wep21 For me it seems that the BCR validation has some issue. Maybe module_file.readlink().as_posix() needs to be replaced with module_file.resolve().as_posix() - not 100% sure... - on my local machine the BCR validation works without the error reported on CI

Vertexwahn
Vertexwahn previously approved these changes Aug 10, 2024
Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@bazel-io bazel-io dismissed Vertexwahn’s stale review August 11, 2024 15:07

Require module maintainers' approval for newly pushed changes.

@wep21 wep21 requested review from Vertexwahn and fmeum August 11, 2024 23:07
@wep21 wep21 force-pushed the add-boost.detail branch 4 times, most recently from 7934d30 to 817d2d9 Compare August 12, 2024 04:17
Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@wep21
Copy link
Contributor Author

wep21 commented Aug 12, 2024

@Vertexwahn @fmeum I have encountered 2 problems.

  • I cannot build targets with --process_headers_in_dependencies
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from include/boost/detail/utf8_codecvt_facet.ipp:13:
include/boost/detail/utf8_codecvt_facet.hpp:99:1: error: unknown type name 'BOOST_UTF8_BEGIN_NAMESPACE'
BOOST_UTF8_BEGIN_NAMESPACE
^
include/boost/detail/utf8_codecvt_facet.hpp:218:1: error: unknown type name 'BOOST_UTF8_END_NAMESPACE'
BOOST_UTF8_END_NAMESPACE
^
In file included from include/boost/detail/utf8_codecvt_facet.ipp:16:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/cassert:24:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/assert.h:81:1: error: expected unqualified-id
__BEGIN_DECLS
^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/cdefs.h:71:32: note: expanded from macro '__BEGIN_DECLS'
#define __BEGIN_DECLS   extern "C" {
                               ^
include/boost/detail/utf8_codecvt_facet.ipp:28:1: error: unknown type name 'BOOST_UTF8_BEGIN_NAMESPACE'
BOOST_UTF8_BEGIN_NAMESPACE
^
include/boost/detail/utf8_codecvt_facet.ipp:33:1: error: expected unqualified-id
namespace detail {
^
include/boost/detail/utf8_codecvt_facet.ipp:294:1: error: unknown type name 'BOOST_UTF8_END_NAMESPACE'
BOOST_UTF8_END_NAMESPACE
^
include/boost/detail/utf8_codecvt_facet.ipp:296:7: error: expected unqualified-id
#endif
      ^
7 errors generated.
INFO: Elapsed time: 1.795s, Critical Path: 1.42s
INFO: 30 processes: 12 internal, 18 darwin-sandbox.
ERROR: Build did NOT complete successfully
  • boost.array dependency is not resolved only on bcr ci.
    Any comments are appreciated.

@Vertexwahn
Copy link
Contributor

Vertexwahn commented Aug 12, 2024

@wep21@fmeum The problem here seems to be boost/detail-boost-1.83.0/include/boost/detail/utf8_codecvt_facet.hpp - if you look into this header it says:

// The header is NOT STANDALONE, and is not to be included by the USER.

i.e. --process_headers_in_dependencies does not work here - I would simply drop it, comment out the feature in the build file and add a comment "Because of utf8_codecvt_facet.hpp this does not work"

@fmeum
Copy link
Contributor

fmeum commented Aug 12, 2024

@wep21@fmeum The problem here seems to be boost/detail-boost-1.83.0/include/boost/detail/utf8_codecvt_facet.hpp - if you look into this header it says:

// The header is NOT STANDALONE, and is not to be included by the USER.

i.e. --process_headers_in_dependencies does not work here - I would simply drop it, comment out the feature in the build file and add a comment "Because of utf8_codecvt_facet.hpp this does not work"

If the header is not meant to be included by the user, but is still included by other files in the target, it should be excluded from the hdrs glob and instead added to textual_headers. This is preferable to disabling the check.

@Vertexwahn
Copy link
Contributor

I tried this for overlay/BUILD.bazel

load("@rules_cc//cc:defs.bzl", "cc_library")

package(default_visibility = ["//visibility:public"])

cc_library(
    name = "boost.detail",
    hdrs = glob(
        [
            "include/**/*.hpp",
            "include/**/*.h",
            "include/**/*.ipp",
        ],
        exclude = [
            "include/boost/detail/utf8_codecvt_facet.hpp",
            "include/boost/detail/utf8_codecvt_facet.ipp",
        ],
    ),
    features = [
        "parse_headers",
    ],
    includes = ["include"],
    textual_hdrs = [
        "include/boost/detail/utf8_codecvt_facet.hpp",
    ],
    deps = [
        "@boost.config",
        "@boost.core",
        "@boost.preprocessor",
        "@boost.static_assert",
        "@boost.type_traits",
    ],
)

All tests work, except //:test_utf8_codecvt

@Vertexwahn
Copy link
Contributor

Vertexwahn commented Aug 12, 2024

I recommend to comment out the test_utf8_codecvt for now. There seem to be 3 more tests under test/container_fwd - maybe those could also be bazelized - you could add: overlay/test/container_fwd/BUILD.bazel with the following content:

cc_test(
    name = "container_fwd_test",
    srcs = ["container_fwd_test.cpp"],
    deps = [
        "@boost.detail",
    ],
)

cc_test(
    name = "container_no_fwd_test",
    srcs = ["container_no_fwd_test.cpp"],
    deps = [
        "@boost.detail",
    ],
)

cc_test(
    name = "correctly_disable_fail",
    srcs = ["correctly_disable_fail.cpp"],
    deps = [
        "@boost.detail",
    ],
)

Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@wep21
Copy link
Contributor Author

wep21 commented Aug 13, 2024

@Vertexwahn

I recommend to comment out the test_utf8_codecvt for now. There seem to be 3 more tests under test/container_fwd - maybe those could also be bazelized - you could add: overlay/test/container_fwd/BUILD.bazel with the following content:

I have encountered another build issues when adding the tests you suggested, so I gave up adding it once.

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/search_n.h:77:6: error: reference to 'pair' is ambiguous
std::pair<_Iter, _Iter> __search_n_random_access_impl(_Iter __first, _Sent __last,
     ^
include/boost/detail/container_fwd.hpp:145:42: note: candidate found by name lookup is 'std::pair'
    template <class T1, class T2> struct pair;
                                         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:81:29: note: candidate found by name lookup is 'std::__1::pair'
struct _LIBCPP_TEMPLATE_VIS pair
                            ^
In file included from test/container_fwd/correctly_disable_fail.cpp:34:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:321:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/formatter_bool.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/concepts.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/format_parse_context.h:16:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/string_view:1059:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1908:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/ranges_unique.h:14:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/unique.h:29:80: error: reference to 'pair' is ambiguous
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 std::pair<_Iter, _Iter>
                                                                               ^
include/boost/detail/container_fwd.hpp:145:42: note: candidate found by name lookup is 'std::pair'
    template <class T1, class T2> struct pair;
                                         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:81:29: note: candidate found by name lookup is 'std::__1::pair'
struct _LIBCPP_TEMPLATE_VIS pair
                            ^
In file included from test/container_fwd/correctly_disable_fail.cpp:34:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:321:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/formatter_bool.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/concepts.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/format_parse_context.h:16:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/string_view:1059:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1908:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/ranges_unique.h:14:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/unique.h:40:17: error: reference to 'pair' is ambiguous
    return std::pair<_Iter, _Iter>(std::move(__first), std::move(__i));
                ^
include/boost/detail/container_fwd.hpp:145:42: note: candidate found by name lookup is 'std::pair'
    template <class T1, class T2> struct pair;
                                         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:81:29: note: candidate found by name lookup is 'std::__1::pair'
struct _LIBCPP_TEMPLATE_VIS pair
                            ^
In file included from test/container_fwd/correctly_disable_fail.cpp:34:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:321:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/formatter_bool.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/concepts.h:17:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/format_parse_context.h:16:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/string_view:1059:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1908:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/ranges_unique.h:14:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/unique.h:42:15: error: reference to 'pair' is ambiguous
  return std::pair<_Iter, _Iter>(__first, __first);
              ^
include/boost/detail/container_fwd.hpp:145:42: note: candidate found by name lookup is 'std::pair'
    template <class T1, class T2> struct pair;
                                         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:81:29: note: candidate found by name lookup is 'std::__1::pair'
struct _LIBCPP_TEMPLATE_VIS pair
                            ^
test/container_fwd/correctly_disable_fail.cpp:38:10: error: reference to 'vector' is ambiguous
    std::vector<int> x;
         ^
include/boost/detail/container_fwd.hpp:135:47: note: candidate found by name lookup is 'std::vector'
    template <class T, class Allocator> class vector;
                                              ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:383:28: note: candidate found by name lookup is 'std::__1::vector'
class _LIBCPP_TEMPLATE_VIS vector
                           ^
test/container_fwd/correctly_disable_fail.cpp:38:10: error: too few template arguments for class template 'vector'
    std::vector<int> x;
         ^
include/boost/detail/container_fwd.hpp:135:47: note: template is declared here
    template <class T, class Allocator> class vector;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~       ^
8 errors generated.
INFO: Elapsed time: 0.506s, Critical Path: 0.41s
INFO: 4 processes: 4 internal.
ERROR: Build did NOT complete successfully

@wep21
Copy link
Contributor Author

wep21 commented Aug 13, 2024

@Vertexwahn @fmeum I would appreciate it if you could review again.

@wep21 wep21 requested a review from Vertexwahn August 13, 2024 18:22
Vertexwahn
Vertexwahn previously approved these changes Aug 13, 2024
Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@bazel-io bazel-io dismissed Vertexwahn’s stale review August 14, 2024 01:45

Require module maintainers' approval for newly pushed changes.

@meteorcloudy meteorcloudy enabled auto-merge (squash) August 14, 2024 07:56
@meteorcloudy
Copy link
Member

@Vertexwahn Please take a final look, if all looks good, just resolve the comments to get this merged.

@Vertexwahn
Copy link
Contributor

Vertexwahn commented Aug 14, 2024

@meteorcloudy LGTM

@meteorcloudy meteorcloudy merged commit 678740d into bazelbuild:main Aug 14, 2024
19 checks passed
@wep21 wep21 deleted the add-boost.detail branch August 14, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants