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

Fix github workflows #291

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Fix github workflows #291

merged 2 commits into from
Mar 12, 2021

Conversation

ivanpauno
Copy link
Member

@ivanpauno ivanpauno commented Mar 11, 2021

Workflows started to fail after ament/ament_lint#268 was merged, e.g.: #288.

This should fix it 🤞

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the enhancement New feature or request label Mar 11, 2021
@ivanpauno ivanpauno self-assigned this Mar 11, 2021
@emersonknapp
Copy link
Contributor

emersonknapp commented Mar 11, 2021

I recommend that you upgrade to action-ros-ci@v0.1 instead of 0.0.19 - which has some important fixes

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

ivanpauno commented Mar 11, 2021

@emersonknapp do you know what could be wrong on the Windows jobs?

This is already greener than before, so I think that the windows issue can be addressed afterwards.

@ivanpauno ivanpauno requested a review from eboasson March 11, 2021 21:47
@emersonknapp
Copy link
Contributor

No, I'm not sure exactly what's wrong. I have not been running windows jobs with the actions recently. I agree that it is a strict improvement and should be merged.

@christophebedard
Copy link
Member

It seems like this step (removing the duplicate package if it's part of the ros2.repos file) doesn't work on Windows: https://github.com/ros-tooling/action-ros-ci/blob/64858bf3c148043e873c2985d7c9fed60914061c/src/action-ros-ci.ts#L338-L347

@clalancette
Copy link
Contributor

Also, I canceled the two builds on ubuntu-20.04. They were both blocked waiting for someone to agree to the RTI license. Somehow the ubuntu-18.04 builder gets around this (either by not installing Connext, or setting an environment variable to skip the prompt), but the 20.04 don't seem to be setup the same way.

@emersonknapp
Copy link
Contributor

Also, I canceled the two builds on ubuntu-20.04. They were both blocked waiting for someone to agree to the RTI license. Somehow the ubuntu-18.04 builder gets around this (either by not installing Connext, or setting an environment variable to skip the prompt), but the 20.04 don't seem to be setup the same way.

That's weird, this should have been fixed in ros-tooling/action-ros-ci#523, which is released to the v0.1 tag

@clalancette
Copy link
Contributor

That's weird, this should have been fixed in ros-tooling/action-ros-ci#523, which is released to the v0.1 tag

I see what you mean in terms of the source repository for action-ros-ci. However, ros-tooling/action-ros-ci#523 doesn't seem to be achieving the desired effect. It looks like rti-connext-5.3.1 is being dragged in as a recursive dependency of something else. I haven't looked closely, but maybe rosdep --skip-keys only looks at immediate key dependencies, and not recursive ones? I'm not sure.

Copy link
Collaborator

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

I always consider this as a bit of black magic that miraculously landed in the repo thanks to @rotu and had no idea that resuscitating it would be so straightforward. Thanks @ivanpauno !

It seems to me that hanging tests could be bit of an issue (I assume a hanging run on one PR might cause it not to run for another PR) but otherwise I all in favour of the changes. It going green for Linux is a major improvement over everything all red, all the time.

@ivanpauno ivanpauno merged commit 98c0665 into master Mar 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/fix-workflows branch March 12, 2021 12:59
clalancette pushed a commit to eboasson/rmw_cyclonedds that referenced this pull request May 18, 2022
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants