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

specifying vcs-repo-file-url to "" for windows/mac causes failure #305

Closed
flynneva opened this issue Aug 14, 2020 · 10 comments · Fixed by #306
Closed

specifying vcs-repo-file-url to "" for windows/mac causes failure #305

flynneva opened this issue Aug 14, 2020 · 10 comments · Fixed by #306
Labels
bug Something isn't working

Comments

@flynneva
Copy link
Contributor

Description

specifying vcs-repo-file-url to "" for windows & mac causes this action to skip the first "rosdep update" step...which causes failures down the line.

Expected Behavior

specifying vcs-repo-file-url to "" should still run the action and build & test the ros package if ros is properly setup and sourced before this action.

Actual Behavior

action fails on the rosdep install step later on

To Reproduce

see it in action here

System (please complete the following information)

  • OS: Windows 10
  • ROS 2 Distro: Foxy

Additional context

the linux version of the action seems to run the rosdep update step first even if you specify the vcs-repo-file-url

@flynneva flynneva added the bug Something isn't working label Aug 14, 2020
@christophebedard
Copy link
Member

Looking at the code, it simply runs rosdep update if (!isWindows):

// rosdep does not reliably work on Windows, see
// ros-infrastructure/rosdep#610 for instance. So, we do not run it.
if (!isWindows) {
await execBashCommand("rosdep update");
}

It doesn't seem to be tied to vcs-repo-file-url.

@christophebedard
Copy link
Member

christophebedard commented Aug 14, 2020

So I guess for now that means it's up to you to make sure all dependencies are properly installed for Windows? (rhetorical question 😆)

Tully suggests using --as-root here ros-infrastructure/rosdep#610 (comment), so maybe we could do that instead.

@flynneva
Copy link
Contributor Author

from ros-infrastructure/rosdep/issues/703 it looks like there was a PR already merged with rosdep that might have helped solve some issues with rosdep on windows. the other PR is still open so im not sure if full support is there yet.

@flynneva
Copy link
Contributor Author

if for windows this action skips "rosdep update", should it then also skip "rosdep install"?

@flynneva
Copy link
Contributor Author

flynneva commented Aug 14, 2020

for windows would it make sense to add "call path-to-ros2-install> &&" before all the commands? similar to what is done for linux with source?

@christophebedard
Copy link
Member

if for windows this action skips "rosdep update", should it then also skip "rosdep install"?

I've never used action-ros-ci/setup-ros for Windows, so I looked into/tried it. If you run rosdep install without ever running rosdep update, it'll fail (but || true prevents a job failure):

ERROR: your rosdep installation has not been initialized yet. Please run:

rosdep update

This isn't Windows-specific obviously, but that's what happens here. So you probably do need to manually install any dependency you need.

for windows would it make sense to add "call path-to-ros2-install> &&" before all the commands? similar to what is done for linux with source?

Good point. Looking at #79 and the related issues, it appears that Windows support is still experimental.

@flynneva
Copy link
Contributor Author

flynneva commented Aug 16, 2020

i think i mightve been a little quick to report an error here.

it seems to be running just fine for windows for some packages (see grbl_ros for example) as long as you dont specify the vcs-repo-file-url argument.

note that it does fail the rosdep install step, but prevents job failure and continues on.

it does look like though the action throws a failure if the vcs-repo-file-url arg is set to ""

@flynneva flynneva changed the title specifying vcs-repo-file-url to "" for windows causes it to skip "rosdep update" specifying vcs-repo-file-url to "" for windows/mac causes failure Aug 16, 2020
flynneva referenced this issue in flynneva/action-ros-ci Aug 16, 2020
flynneva referenced this issue in flynneva/action-ros-ci Aug 16, 2020
trying to resolve #305

Signed-off-by: Evan Flynn <evanflynn.msu@gmail.com>
@christophebedard
Copy link
Member

it does look like though the action throws a failure if the vcs-repo-file-url arg is set to ""

Can you provide a link to a failure like that? The link in the issue description above doesn't work anymore.

it seems to be running just fine for windows for some packages (see grbl_ros for example) as long as you dont specify the vcs-repo-file-url argument.

If you remove vcs-repo-file-url: "" it'll recompile ROS 2 from source. However, it looks like you setup both actions to use ROS 2 binaries (https://github.com/flynneva/grbl_ros/blob/603bbbe3d5ca2c9fce332b14fc4076802c4c9bf6/.github/workflows/ros_ci.yml#L59), so you shouldn't need to re-compile from source.

@flynneva
Copy link
Contributor Author

yeah sorry ive been testing it with/without the vcs-repo-file-url param and seeing the behavior.

here it is again after I specified the vcs-repo-file-url to ""

@christophebedard
Copy link
Member

here it is again after I specified the vcs-repo-file-url to ""

It's failing because ament_copyright/ament_flake8/ament_pep257 cannot be found for testing, so it's either because the binary installation wasn't sourced (not sure how it works on Windows) or because it just wasn't installed, i.e. ros-tooling/setup-ros#243.

Removing vcs-repo-file-url makes it import the ros2.repos file and compile everything from source so it ends up working fine, so it's not actually the problem.

I think this could be closed in favour of ros-tooling/setup-ros#243

flynneva referenced this issue in flynneva/action-ros-ci Aug 18, 2020
trying to resolve #305

Signed-off-by: Evan Flynn <evanflynn.msu@gmail.com>
flynneva referenced this issue in flynneva/action-ros-ci Aug 18, 2020
trying to resolve #305

Signed-off-by: Evan Flynn <evanflynn.msu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants