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

refactor(docker): introduce src-imported stage #4709

Closed
wants to merge 1 commit into from

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented May 13, 2024

Description

This PR adds the src-imported stage to the top stage of the Dockerfile.
This allows the base and src-imported stages to be processed in parallel, reducing the download latency of the vcs import command. It also eliminates the need to run the vcs import command again in the runtime stage.
The trial in my local environment speeded up the docker build by about 5 minutes.

This PR does not cut out rosdep install, based on the reflection of the previous PR:
#4656 (comment)

Tests performed

Not applicable.

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk requested a review from oguzkaganozt May 13, 2024 05:17
@youtalk youtalk marked this pull request as ready for review May 13, 2024 06:31
@youtalk youtalk requested review from mitsudome-r and xmfcx as code owners May 13, 2024 06:31
@youtalk youtalk self-assigned this May 13, 2024
Copy link
Member

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

LGTM

@youtalk
Copy link
Member Author

youtalk commented May 13, 2024

@mitsudome-r Please merge it because I have no longer the write access anyhow.

@xmfcx
Copy link
Contributor

xmfcx commented May 13, 2024

How was this tested?

I can merge it but I'm worried it might break the containers since I didn't test it.

@youtalk
Copy link
Member Author

youtalk commented May 13, 2024

@xmfcx How can I test the container? I'm sorry but I don't know the test function since I'm new.

@youtalk youtalk closed this May 13, 2024
@youtalk youtalk deleted the src-imported branch May 13, 2024 12:27
@youtalk youtalk removed their assignment Jun 11, 2024
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.

3 participants