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

Add pybind11_abseil #2031

Merged
merged 1 commit into from
May 21, 2024
Merged

Add pybind11_abseil #2031

merged 1 commit into from
May 21, 2024

Conversation

mering
Copy link
Contributor

@mering mering commented May 14, 2024

No description provided.

@bazel-io
Copy link
Member

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

@mering mering marked this pull request as ready for review May 14, 2024 11:26
@fmeum
Copy link
Contributor

fmeum commented May 14, 2024

@rwgk @junyer Would you be willing to maintain this BCR entry and are you okay with its contents?

@mering
Copy link
Contributor Author

mering commented May 14, 2024

As discussed internally, I propse @frigus02 as maintainer. Are you willing to accept?

@mering
Copy link
Contributor Author

mering commented May 14, 2024

@fmeum could you please trigger the tests?

@frigus02
Copy link

I accept

@mering mering force-pushed the add-pybind11-abseil branch 2 times, most recently from bcf6d74 to 072b9ec Compare May 14, 2024 17:00
@mering
Copy link
Contributor Author

mering commented May 14, 2024

@fmeum could you please trigger the tests?

@fmeum fmeum added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label May 14, 2024
@mering
Copy link
Contributor Author

mering commented May 15, 2024

Anyone has an idea why the tests can't find @python or @pypi even though they are defined within MODULE.bazel?

@fmeum
Copy link
Contributor

fmeum commented May 15, 2024

It's because of dev_dependency = True. The module is tested from an anonymous root module, so it won't be the root module itself.

@mering
Copy link
Contributor Author

mering commented May 15, 2024

@fmeum Would there be any problems with just removing this?

@mering mering force-pushed the add-pybind11-abseil branch 5 times, most recently from c0a6b86 to dfac6dd Compare May 15, 2024 11:15
@mering
Copy link
Contributor Author

mering commented May 15, 2024

@fmeum Hmm. Even after removing the dev_dependency it still seems to fail with the same error.

@fmeum
Copy link
Contributor

fmeum commented May 15, 2024

It's still failing, but for a different reason. You may just need to pass in --cxxopt=-std=c++14.

@mering mering force-pushed the add-pybind11-abseil branch 3 times, most recently from bed4fda to faae23c Compare May 21, 2024 10:01
@mering
Copy link
Contributor Author

mering commented May 21, 2024

@fmeum Thanks. Linux builds seem to run fine with --cxxopt=-std=c++14 but Windows and MacOS builds fail now. I don't have any such architecture available to test.

Windows tests now fail with

cl : Command line warning D9002 : ignoring unknown option '-std=c++14'
C:\b\ehbqvbze\execroot\_main\external\pybind11_abseil~202402.0\pybind11_abseil/compat/py_base_utilities.h(9): error C2429: language feature 'nested-namespace-definition' requires compiler flag '/std:c++17'
external/pybind11_abseil~202402.0/pybind11_abseil/compat/py_base_utilities.cc(12): error C2429: language feature 'nested-namespace-definition' requires compiler flag '/std:c++17'

What is the best way to set a minimum C++ version across all supported OSes?

@junyer
Copy link
Contributor

junyer commented May 21, 2024

If it helps, RE2 does this.

@mering
Copy link
Contributor Author

mering commented May 21, 2024

@junyer IIUC .bazelrc is not taken into account from BCR tests as the module under test is added as dependency. What is the best way to specify different flags per architecture in presubmit.yml in BCR?

@mering
Copy link
Contributor Author

mering commented May 21, 2024

MacOS fails with

ld: symbol(s) not found for architecture x86_64

Any idea what could be wrong here?

@mering
Copy link
Contributor Author

mering commented May 21, 2024

Windows now fails with

external/pybind11_abseil~202402.0/pybind11_abseil/tests/cpp_capsule_tools_testing.cc(24): error C2466: cannot allocate an array of constant size 0

@junyer
Copy link
Contributor

junyer commented May 21, 2024

https://buildkite.com/bazel/bcr-presubmit/builds/5056#018f2f66-e390-4cb3-9159-7c13bd0741d0 indicates otherwise:

…
(14:27:18) INFO: Reading rc options for 'test' from c:\b\bk-windows-60p1\bazel-org-repo-root\output\re2-2024-05-01\.bazelrc:
  Inherited 'build' options: --features=layering_check --features=parse_headers --enable_platform_specific_config --experimental_enable_bzlmod --registry=file:///c:/b/bk-windows-60p1/bazel/bcr-presubmit
(14:27:18) INFO: Reading rc options for 'test' from c:\b\bk-windows-60p1\bazel-org-repo-root\output\re2-2024-05-01\.bazelrc:
  'test' options: --test_output=errors
(14:27:18) INFO: Found applicable config definition build:windows in file c:\b\bk-windows-60p1\bazel-org-repo-root\output\re2-2024-05-01\.bazelrc: --cxxopt=/std:c++14
…

@mering
Copy link
Contributor Author

mering commented May 21, 2024

I removed tests for Windows and MacOS as upstream also doesn't test this: https://github.com/pybind/pybind11_abseil/blob/master/.github/workflows/actions.yml.

Tests are passing now.

@mering mering mentioned this pull request May 21, 2024
@fmeum fmeum merged commit e2cea1c into bazelbuild:main May 21, 2024
12 checks passed
@mering mering deleted the add-pybind11-abseil branch May 21, 2024 12:35
fmeum pushed a commit that referenced this pull request May 29, 2024
This is using the same patches as in #2031.

As there is no release process, we are using a version string including
the commit sha like it's done for `boringssl` as well.
aiuto pushed a commit to aiuto/bazel-central-registry that referenced this pull request Jun 3, 2024
aiuto pushed a commit to aiuto/bazel-central-registry that referenced this pull request Jun 3, 2024
This is using the same patches as in bazelbuild#2031.

As there is no release process, we are using a version string including
the commit sha like it's done for `boringssl` as well.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants