Conversation
bb08eb6 to
58965a9
Compare
e394e8c to
9c3fdae
Compare
9c3fdae to
c441622
Compare
|
@hofbi Ready for re-review |
|
@mvukov Please review |
mvukov
left a comment
There was a problem hiding this comment.
I left a couple of comments, mostly about possible optimizations. Please review those and lemme know what you think.
| symlinks = { | ||
| "/chatter/chatter.runfiles/_main/external": "/chatter/chatter.runfiles", | ||
| } |
There was a problem hiding this comment.
If you specify the workdir down below, you shoudn't need this symlink. Please double-check.
There was a problem hiding this comment.
Without the symlink, an error occurs:
RLException: Invalid roslaunch XML syntax: [Errno 2] No such file or directory: 'external/rules_ros~/third_party/ros/roslaunch/roscore.xml'
The traceback for the exception was written to the log file
Something is expecting all the runfiles to be in an 'external' directory below the working directory.
| ) | ||
|
|
||
|
|
||
| # packages up the nodes, launch file, and all the other required runfiles. |
There was a problem hiding this comment.
rules_oci have a good example for to structure python apps, see https://github.com/aspect-build/bazel-examples/tree/main/oci_python_image, py_layer.bzl file in particular. Maybe not for this PR, but it's a nice optimization.
| oci = use_extension("@rules_oci//oci:extensions.bzl", "oci") | ||
| oci.pull( | ||
| name = "rules_ros_base_image", | ||
| image = "python:3.10-bookworm", |
There was a problem hiding this comment.
IMO this is an overkill, as we need system python (-like app) to bootstrap the Bazel-managed Python. You could fetch https://raw.githubusercontent.com/buildbarn/bb-remote-execution/96c4fdce659fabfaba7ee2a60fd4e2ffab8352e2/cmd/fake_python/main.go with http_file, compile it using rules_go and inject it into the output image as e.g. /usr/bin/python3 (using ubuntu:22.04 image in this case should be sufficient).
There was a problem hiding this comment.
perhaps overkill.... but needing to replicate your suggestion in each and every repo feels way to repetitive. Could we bake this into rules_ros or better yet, rules_python?
There was a problem hiding this comment.
why not use ubuntu:22.04 as base image?
There was a problem hiding this comment.
I don't think ubuntu:22.04 includes a python.
There was a problem hiding this comment.
Really? I thought it comes with 3.10
There was a problem hiding this comment.
Does not appear that the image does.
oci = use_extension("@rules_oci//oci:extensions.bzl", "oci")
oci.pull(
name = "rules_ros_base_image",
image = "ubuntu:22.04",
platforms = ["linux/amd64"],
)
use_repo(oci, "rules_ros_base_image", "rules_ros_base_image_linux_amd64")
$ docker run -ti --rm chatter:latest
/usr/bin/env: 'python3': No such file or directory
added example of packing up in to a docker container using rules_oci and rules_pkg
closes GH-10