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

Audit build caching #40

Closed
wjwwood opened this issue Apr 15, 2015 · 6 comments
Closed

Audit build caching #40

wjwwood opened this issue Apr 15, 2015 · 6 comments
Assignees

Comments

@wjwwood
Copy link
Member

wjwwood commented Apr 15, 2015

It seems that repeat builds of the ROS 2 stack with ament build will result in rebuilding or even regenerating code in parts of the system.

See: ament/ament_tools#32 (comment)

@dirk-thomas
Copy link
Member

The first item to address is that templated message files are always overwritten. The above four PRs change that so that the generated files are only overwritten when their content actually changes. Otherwise they are not touched.

Update: The job finally confirms that it still builds and passes tests: http://54.183.26.131:8080/view/ros2/job/ros2_batch_ci_linux/44/

@esteve @tfoote @wjwwood Please review the four PRs.

@dirk-thomas
Copy link
Member

I can't find any unnecessary operations in the log of a repeated build anymore.

@esteve @tfoote @wjwwood Please run a repeated build (with the above PRs checked out / merged) and review the build log. Then either confirm that this can be closed or comment with specific parts from the build log which should be skipped.

@tfoote
Copy link
Contributor

tfoote commented Apr 21, 2015

These individually looks reasonable. However there's a lot of repeated code snippets for the write file if changed which would be much better to consolidate into a function. (11 instances)

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 21, 2015
@dirk-thomas
Copy link
Member

@tfoote I agree that refactoring the functionality would be desired but I would consider that for a separate PR.

@tfoote
Copy link
Contributor

tfoote commented Apr 21, 2015

separate is fine by me ticketed: #44 to not forget.

@dirk-thomas
Copy link
Member

Closing for now. If anybody notices any rebuilding on repeated invocation please comment here and the ticket can be reopened.

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

No branches or pull requests

3 participants