search

ros-tooling / cross_compile

A tool to build ROS and ROS2 workspaces for various targets
Apache License 2.0
187 stars 59 forks source link

First pass at specifying a custom docker image #330

Closed kjeremy closed 1 year ago

kjeremy commented 3 years ago

285

kjeremy commented 3 years ago

Can I get some guidance on writing a test for this?

codecov[bot] commented 3 years ago

Codecov Report

Merging #330 (9875324) into master (6414428) will decrease coverage by 0.51%. The diff coverage is 75.00%.

:exclamation: Current head 9875324 differs from pull request most recent head 28b71be. Consider uploading reports for the commit 28b71be to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
- Coverage   93.23%   92.72%   -0.52%     
==========================================
  Files          11       11              
  Lines         414      426      +12     
==========================================
+ Hits          386      395       +9     
- Misses         28       31       +3
Flag Coverage Δ
unittests 92.72% <75.00%> (-0.52%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
ros_cross_compile/ros_cross_compile.py 83.14% <57.14%> (-2.22%) :arrow_down:
ros_cross_compile/platform.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6414428...28b71be. Read the comment docs.

emersonknapp commented 3 years ago

This looks pretty good! note - we'll want an addition to the README as well once this is finalized.

Can I get some guidance on writing a test for this?

Some simple unit testing around Platform would be good. There isn't (currently) a Python-based test that runs the full pipeline to try out the CLI entrypoint in that way - but the Github Action workflows call run_e2e_test.sh, you could add another test condition there to try out this with a base image. I might try using rostooling/setup-ros-docker:ubuntu-focal-ros-foxy-ros-base-latest as the override sysroot, for something that only needs dependencies from the ros-base set.

kjeremy commented 3 years ago

@emersonknapp

In my testing this doesn't seem to trigger a build. Do I need to somehow copy the build_workspace.sh file into the container and execute it?

emersonknapp commented 3 years ago

In my testing this doesn't seem to trigger a build. Do I need to somehow copy the build_workspace.sh file into the container and execute it?

Oh - yes, I made a mistake suggesting that image as the sysroot. Given the way the build currently works, you'll probably need to match the setup from this line on https://github.com/ros-tooling/cross_compile/blob/master/ros_cross_compile/docker/sysroot.Dockerfile#L89

Thinking about it further, this feature is probably best suited (in the first version) to just reusing an image from a prior cross-compile build. Does that sound right to you? If so, then we could somehow specify or get the name of the sysroot image from a previous test build, and run using that.

kjeremy commented 3 years ago

That sounds reasonable. Ultimately I would like to be able to save that earlier generated image off (push it to my gitlab registry) and refer to it in the next invocation to actually use it.

On Wed, Jun 30, 2021, 2:11 PM Emerson Knapp @.***> wrote:

In my testing this doesn't seem to trigger a build. Do I need to somehow copy the build_workspace.sh file into the container and execute it?

Oh - yes, I made a mistake suggesting that image as the sysroot. Given the way the build currently works, you'll probably need to match the setup from this line on https://github.com/ros-tooling/cross_compile/blob/master/ros_cross_compile/docker/sysroot.Dockerfile#L89

Thinking about it further, this feature is probably best suited (in the first version) to just reusing an image from a prior cross-compile build. Does that sound right to you? If so, then we could somehow specify or get the name of the sysroot image from a previous test build, and run using that.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ros-tooling/cross_compile/pull/330#issuecomment-871622062, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBACREBSF6FYJRARHLPBF3TVNM3PANCNFSM47QMG67A .

emersonknapp commented 3 years ago

Ultimately I would like to be able to save that earlier generated image off (push it to my gitlab registry) and refer to it in the next invocation to actually use it.

Makes sense! That should work with this approach - the main point is we need to use a sysroot image that was generated by the tool, rather than some other bare image.

kjeremy commented 3 years ago

It sounds like we need another argument that would override the image generated from the previous stage.

If so could we use one of the existing customization script points to push the image to a registry or would the program need to be modified to do that as well?

On Wed, Jun 30, 2021, 7:38 PM Emerson Knapp @.***> wrote:

Ultimately I would like to be able to save that earlier generated image off (push it to my gitlab registry) and refer to it in the next invocation to actually use it.

Makes sense! That should work with this approach - the main point is we need to use a sysroot image that was generated by the tool, rather than some other bare image.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ros-tooling/cross_compile/pull/330#issuecomment-871794877, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBACRE7BZUUH3JFM6ISY53TVOTJFANCNFSM47QMG67A .

emersonknapp commented 3 years ago

If so could we use one of the existing customization script points to push the image to a registry or would the program need to be modified to do that as well?

There's not a good way to hook this in as-is, since the customization scripts run inside a container, rather than on the host.

High level, what could be done is

MichaelOrlov commented 1 year ago