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

Change Build Tutorial Image CI job to only build main against Rolling #782

Merged
merged 14 commits into from
Oct 5, 2023

Conversation

MarqRazz
Copy link
Contributor

@MarqRazz MarqRazz commented Oct 4, 2023

This PR started out trying to fix the CI job Build Tutorial Image so that it supported both humble and rolling but as we investigated further it did not make sense to build both humble and main when changes were only made to one branch. A follow up PR will be made to the humble branch to only build and update the corresponding image.

This is an attempt to fix the Build Tutorial Image step for humble. It's currently failing with the following error.

#18 517.2 --- stderr: hello_moveit
#18 517.2 /root/ws_moveit/src/hello_moveit/src/hello_moveit.cpp: In function 'int main(int, char**)':
#18 517.2 /root/ws_moveit/src/hello_moveit/src/hello_moveit.cpp:107:36: error: 'std::tuple_element<1, const std::pair<bool, moveit::planning_interface::MoveGroupInterface::Plan> >::type' {aka 'const struct moveit::planning_interface::MoveGroupInterface::Plan'} has no member named 'trajectory'; did you mean 'trajectory_'?
#18 517.2   107 |     draw_trajectory_tool_path(plan.trajectory);
#18 517.2       |                                    ^~~~~~~~~~
#18 517.2       |                                    trajectory_
#18 517.2 gmake[2]: *** [CMakeFiles/hello_moveit.dir/build.make:76: CMakeFiles/hello_moveit.dir/src/hello_moveit.cpp.o] Error 1

@MarqRazz MarqRazz requested a review from henningkayser October 4, 2023 16:07
@MarqRazz
Copy link
Contributor Author

MarqRazz commented Oct 4, 2023

Thanks for the help @rhaschke!

The default git context would use the main branch...
@rhaschke
Copy link
Contributor

rhaschke commented Oct 4, 2023

You are welcome. The main issue is that the docker/build-push-action uses the current git branch by default:
https://github.com/docker/build-push-action#git-context

However, fixing this via 40dd10e raises new issues... I'm looking into this shortly, but I will handover to you again, if my time window is over.

... as we want to access the files in there
@rhaschke
Copy link
Contributor

rhaschke commented Oct 4, 2023

Why do you want to build both docker images (for rolling and humble) from the same branch, while they need different source branches:
https://github.com/ros-planning/moveit2_tutorials/blob/6ec65420681b26efb3b2ee57bce79bfddece9922/.github/workflows/docker-images.yml#L12

I suggest building the images only from their corresponding branch...
If you don't like that approach, I think you will need to sync the Dockerfile from main to humble branch.

I need to continue with other stuff now.

@MarqRazz
Copy link
Contributor Author

MarqRazz commented Oct 4, 2023

I suggest building the images only from their corresponding branch...

I agree! If you are okay with the change I made I will fix the description of this PR and also open up one for the humble branch.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

I reverted some changes that are not needed anymore if we can just use the git context.

@MarqRazz MarqRazz changed the title Fix Humble CI Docker image build Change Build Tutorial Image CI job to only build main against Rolling Oct 5, 2023
@MarqRazz MarqRazz merged commit 8cd7875 into moveit:main Oct 5, 2023
8 of 9 checks passed
@MarqRazz MarqRazz deleted the pr-fix_humble_ci branch October 5, 2023 14:06
disvorten added a commit to Natashe4ka/moveit2_tutorials that referenced this pull request Oct 12, 2023
sjahr pushed a commit to sjahr/moveit2_tutorials that referenced this pull request Oct 26, 2023
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.

2 participants