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

Read bazel lib version using bazel CLI instead of hardcoding it. #76

Merged
merged 2 commits into from
May 7, 2024

Conversation

agalbachicar
Copy link
Contributor

🎉 New feature

Closes #74

Summary

Executes bazel mod explain to obtain maliput and maliput_malidrive library versions in maliput-sdk/build.rs.

Test it

By executing cargo check / build.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if it affects the public API)

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Agustin Alba Chicar <ag.albachicar@ekumenlabs.com>
francocipollone
francocipollone previously approved these changes May 6, 2024
Copy link
Contributor

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Check clippy. CI is also failing weirdly.

maliput-sdk/build.rs Outdated Show resolved Hide resolved
maliput-sdk/build.rs Outdated Show resolved Hide resolved
Signed-off-by: Franco Cipollone <franco.c@ekumenlabs.com>
@francocipollone
Copy link
Contributor

One interesting thing happened: CC @agalbachicar
Remember that the print is mainly used at build.rs for indicating to cargo some stuff (links,rustc envs, etc)

You were using print macro for printing information (with no new_line) so if we check the output of the maliput-sdk build:

cargo:rerun-if-changed=src/lib.rs
cargo:rerun-if-changed=BUILD.bazel
cargo:rerun-if-changed=MODULE.bazel
Detected maliput version: <1.2.0>Detected maliput_malidrive version: <0.2.1>cargo:CXXBRIDGE_DIR0=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/geometry_base
cargo:CXXBRIDGE_DIR1=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/api
cargo:CXXBRIDGE_DIR2=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/math
cargo:CXXBRIDGE_DIR3=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/utility
cargo:CXXBRIDGE_DIR4=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/drake
cargo:CXXBRIDGE_DIR5=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/common
cargo:CXXBRIDGE_DIR6=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/base
cargo:CXXBRIDGE_DIR7=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/drake_private_systems
cargo:CXXBRIDGE_DIR8=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/plugin
cargo:CXXBRIDGE_DIR9=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0/_virtual_includes/drake_private_common
cargo:rustc-env=MALIPUT_BIN_PATH=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0
cargo:rustc-env=MALIPUT_MALIDRIVE_BIN_PATH=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput_malidrive~0.2.1
cargo:rustc-env=MALIPUT_MALIDRIVE_PLUGIN_PATH=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput_malidrive~0.2.1/maliput_plugins
cargo:rustc-env=MALIPUT_MALIDRIVE_RESOURCE_PATH=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/libmaliput_sdk.so.runfiles/maliput_malidrive~0.2.1/resources
cargo:root=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out
cargo:maliput_bin_path=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput~1.2.0
cargo:maliput_malidrive_bin_path=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput_malidrive~0.2.1
cargo:maliput_malidrive_plugin_path=/workspaces/maliput-rs/target/debug/build/maliput-sdk-bdf1a05400489123/out/bazel_output_base/bazel-bin/external/maliput_malidrive~0.2.1/maliput_plugins

You see in the 4th line that the cargo:CXXBRIDGE_DIR0 command isn't parsed correctly and then the cxx configuration go bananas.

The bottom line is: Avodining regular prints in build.rs files might be the best.

Copy link
Contributor

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@francocipollone
Copy link
Contributor

Extra comment: @agalbachicar
You can execute ./tools/reformat_code.sh script for making clippy and fmt happy before git commit (+presubmit)

@agalbachicar
Copy link
Contributor Author

Thanks @francocipollone !

@agalbachicar agalbachicar merged commit d858a4a into main May 7, 2024
3 checks passed
@agalbachicar agalbachicar deleted the agalbachicar/#74_unhardcode_bazel_lib_version branch May 7, 2024 07:01
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.

Use the MODULE.bazel version string and avoid hardcoding it into build.rs files.
2 participants