Skip to content

Fix build_and_run_tests.sh command error.#1319

Closed
tsogoo wants to merge 1 commit into
google-deepmind:masterfrom
tsogoo:master
Closed

Fix build_and_run_tests.sh command error.#1319
tsogoo wants to merge 1 commit into
google-deepmind:masterfrom
tsogoo:master

Conversation

@tsogoo

@tsogoo tsogoo commented Mar 7, 2025

Copy link
Copy Markdown

First, kudos to the OpenSpiel team for maintaining such an impressive and valuable framework for reinforcement learning and game theory research!

This PR addresses an issue with the pybind11 dependency in OpenSpiel, stemming from the smart_holder branch being archived and renamed to archive/smart_holder in the pybind11 repository (see: https://github.com/pybind/pybind11/tree/archive/smart_holder). The original build process attempted to clone the now-deprecated smart_holder branch, causing errors.

Changes Made

  1. Updated pybind11 Branch: Modified the cached_clone command in ./open_spiel/scripts/build_and_run_tests.sh to use the master branch of pybind11 instead of smart_holder or archive/smart_holder. The smart_holder features are now fully integrated into master, making this the appropriate target.
  2. Removed Deprecated Code: As per the pybind11 migration guidelines, removed the obsolete #include <pybind11/smart_holder.h> and the following deprecated macros from the codebase:
    • PYBIND11_SMART_HOLDER_TYPE_CASTERS
      These are no longer needed with the latest pybind11.

Result

These changes resolve the build errors encountered when running ./open_spiel/scripts/build_and_run_tests.sh, ensuring compatibility with the current pybind11 master branch.

Please review and let me know if additional adjustments are needed!

@google-cla

google-cla Bot commented Mar 7, 2025

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@tsogoo tsogoo closed this Mar 7, 2025
@tsogoo

tsogoo commented Mar 7, 2025

Copy link
Copy Markdown
Author

I realized another PR #1318 that fixes the error. so i closed.

@lanctot

lanctot commented Mar 7, 2025

Copy link
Copy Markdown
Collaborator

Oh, wow, wonderful. Thanks for this!

@lanctot

lanctot commented Mar 7, 2025

Copy link
Copy Markdown
Collaborator

(Yes I've been chatting with @rwgk from the pybind11 team who has been looking at fixing this in a similar way -- but thank you for fixing this nonetheless.)

@lanctot

lanctot commented Mar 7, 2025

Copy link
Copy Markdown
Collaborator

Opened a new version now (#1320) based on a unification of this with #1318

@lanctot lanctot closed this Mar 7, 2025
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

Successfully merging this pull request may close these issues.

2 participants