wait for ros binaries before returning

[flynneva]

Description

• Does this PR check in generated JS code (npm run build)?

[flynneva]

#243

[christophebedard]

I was wondering why the tests didn't catch this, so I took a look. Weirdly enough, it seemed to work fine in a previous test 😅 https://github.com/ros-tooling/setup-ros/runs/942469149#step:5:374

I don't know a lot about async or JS/TS, but maybe it's about timing?

Could you try this branch with your own repo and see if it actually works? Like uses: flynneva/setup-ros@windows-fix

[flynneva]

running now on grbl_ros.

it shouldnt matter, but I also put it for the linux ci tests just to make sure nothing funky goes on there

[flynneva]

it looks like this change successfully does what its trying to do - download the ros2 binaries for windows. but it looks like later on there are still failures...maybe from not sourcing the ros2 install?

[christophebedard]

it looks like this change successfully does what its trying to do - download the ros2 binaries for windows.

🎊

but it looks like later on there are still failures...maybe from not sourcing the ros2 install?

Only one way to find out 😁 can you try this branch with ros-tooling/action-ros-ci#306 in your repo?

[flynneva]

re-running because it looks like a failure on one of the mac runs caused all windows/mac runs to be canceled.

it looks like for mac there is the same issue....only stops at rosdep init and doesnt actually install ROS2 binaries

[christophebedard]

it looks like for mac there is the same issue....only stops at rosdep init and doesnt actually install ROS2 binaries

I don't think the action even tries to install binaries on Mac: https://github.com/ros-tooling/setup-ros/blob/1a56282ce2ee5651bf97ad6229b5fa2260773b16/src/setup-ros-osx.ts. Not sure what's the intention here. Also, the README (https://github.com/ros-tooling/setup-ros#modifications-performed-by-the-action) only says that it installs system dependencies but does not mention installing (or having the option to install) ROS/ROS 2 itself, so I'm a bit confused.

Ohh wait, binary install isn't yet supported on OS X: #107

[christophebedard]

Ohh wait, binary install isn't yet supported on OS X: #107

So maybe try to remove the MacOS part for now? This way we can continue with the Windows stuff.

[flynneva]

i think this PR is OK - it does what is expected and downloads the ROS2 binaries.

there are more issues with action-ros-ci i think - looks like the "isWindows" doesnt actually return true if its windows because the windows commands are still being run with "bash".

[christophebedard]

i think this PR is OK - it does what is expected and downloads the ROS2 binaries.

Definitely!

there are more issues with action-ros-ci i think - looks like the "isWindows" doesnt actually return true if its windows because the windows commands are still being run with "bash".

By default it doesn't show you the actual command. If you click on a command ("Invoking ...") it'll show the full command, which seems to be fine, e.g.:

C:\Windows\system32\cmd.exe /E:ON /V:OFF /S /C call "%programfiles(x86)%\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" amd64 & "C:\Program Files\Git\bin\bash.exe" -c "colcon test --event-handlers console_cohesion+ --pytest-with-coverage --return-code-on-test-failure --packages-select grbl_ros

[emersonknapp]

Are you actually seeing changed behavior on your test runs against this branch? My understanding is that the action runner runs dist/index.js which needs to be generated locally and checked in with the PR

npm install
npm run build

Should give you the outputs you need

[christophebedard]

oh, right. Maybe it's just luck/timing then?

I assume it should be fine, since runWindows() simply returns a promise that is awaited in setup-ros.ts, but we do see a difference:

• without this change (targeting a released version of setup-ros): https://github.com/flynneva/grbl_ros/runs/990649641#step:2:362
• with this change/branch: https://github.com/flynneva/grbl_ros/runs/990671195?check_suite_focus=true#step:2:362

first one stops at rosdep init but the second one goes on and downloads+unzips the binary.