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 removal of duplicate repo on Windows #579

Merged
merged 2 commits into from
Mar 19, 2021
Merged

Fix removal of duplicate repo on Windows #579

merged 2 commits into from
Mar 19, 2021

Conversation

christophebedard
Copy link
Member

Fixes #578

This makes sure that the paths returned by vcs are valid POSIX paths so that rm -rf works.

I tried for hours to use sed directly in the execBashCommand() call, but it never worked. It feels like something somewhere is messing up with the escape characters (it worked fine on Ubuntu).

Signed-off-by: Christophe Bedard bedard.christophe@gmail.com

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard requested a review from a team as a code owner March 14, 2021 20:16
@christophebedard christophebedard requested review from thomas-moulard and prajakta-gokhale and removed request for a team March 14, 2021 20:16
@codecov
Copy link

codecov bot commented Mar 14, 2021

Codecov Report

Merging #579 (635c8cb) into master (987e5c6) will decrease coverage by 2.17%.
The diff coverage is 0.00%.

❗ Current head 635c8cb differs from pull request most recent head 798d4c1. Consider uploading reports for the commit 798d4c1 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
- Coverage   39.90%   37.72%   -2.18%     
==========================================
  Files           2        2              
  Lines         213      220       +7     
  Branches       39       41       +2     
==========================================
- Hits           85       83       -2     
- Misses        128      137       +9     
Impacted Files Coverage Δ
src/action-ros-ci.ts 28.27% <0.00%> (-2.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 987e5c6...798d4c1. Read the comment docs.

@christophebedard
Copy link
Member Author

This CI job for rmw_cyclonedds shows that this PR fixes #578*: https://github.com/christophebedard/rmw_cyclonedds/pull/1/checks?check_run_id=2107896554#step:6:1153

*i.e. colcon build does run; it fails later, but that's unrelated

@emersonknapp emersonknapp enabled auto-merge (squash) March 17, 2021 22:04
@emersonknapp emersonknapp merged commit 2466232 into ros-tooling:master Mar 19, 2021
@christophebedard christophebedard deleted the fix-windows-duplicate-removal branch March 19, 2021 01:26
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.

Action fails to remove duplicate repos on Windows
2 participants