ros-tooling / cross_compile

A tool to build ROS and ROS2 workspaces for various targets
Apache License 2.0
187 stars 59 forks source link

Quick fix for issue #360 #361

Closed Le5tes closed 1 year ago

Le5tes commented 2 years ago

This fixes the bug that I had: https://github.com/ros-tooling/cross_compile/issues/360

Not sure if anyone else is having this issue, so if not feel free to ignore!

MichaelOrlov commented 2 years ago

@Le5tes I don't mind about this fix. I am only worry about failing tests on CI. Could you please try to fix warnings reported from linters?

=================================== FAILURES ===================================
_________________________________ test_flake8 __________________________________
    @pytest.mark.flake8
    @pytest.mark.linter
    def test_flake8():
        logging.getLogger('flake8').setLevel(logging.INFO)
        rc = main(argv=[])
>       assert rc == 0, 'Found errors'
E       AssertionError: Found errors
E       assert 1 == 0
E         +1
E         -0
test/test_flake8.py:26: AssertionError
----------------------------- Captured stdout call -----------------------------
./ros_cross_compile/docker_client.py:72:26: Q000 Double quotes found but single quotes preferred
            network_mode="host"
                         ^
1     Q000 Double quotes found but single quotes preferred
codecov[bot] commented 2 years ago

Codecov Report

Base: 93.01% // Head: 92.99% // Decreases project coverage by -0.01% :warning:

Coverage data is based on head (2ba8acc) compared to base (0caa4cd). Patch has no changes to coverable lines.

:exclamation: Current head 2ba8acc differs from pull request most recent head 9b693b7. Consider uploading reports for the commit 9b693b7 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #361 +/- ## ========================================== - Coverage 93.01% 92.99% -0.02% ========================================== Files 10 11 +1 Lines 415 414 -1 ========================================== - Hits 386 385 -1 Misses 29 29 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `92.99% <ø> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/ros-tooling/cross_compile/pull/361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling) | Coverage Δ | | |---|---|---| | [ros\_cross\_compile/docker\_client.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling#diff-cm9zX2Nyb3NzX2NvbXBpbGUvZG9ja2VyX2NsaWVudC5weQ==) | `97.67% <ø> (ø)` | | | [ros\_cross\_compile/dependencies.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling#diff-cm9zX2Nyb3NzX2NvbXBpbGUvZGVwZW5kZW5jaWVzLnB5) | `100.00% <0.00%> (ø)` | | | [ros\_cross\_compile/\_\_init\_\_.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling#diff-cm9zX2Nyb3NzX2NvbXBpbGUvX19pbml0X18ucHk=) | `100.00% <0.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-tooling)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

christophebedard commented 2 years ago

Looks okay to me. Could you sign off your commit(s) so that they pass the DCO check? https://github.com/ros-tooling/cross_compile/pull/361/checks?check_run_id=7336140466

Not sure what's going on with the shellcheck linter failure. The other failures are in the nightly job as well: https://github.com/ros-tooling/cross_compile/actions/runs/2665228259

Le5tes commented 2 years ago

I signed off the commits. I'll have a look into the test failures a bit over the weekend.

MichaelOrlov commented 1 year ago

@Le5tes Could you please address linter errors in you PR? We are going to archive entire repo very soon and this is last chance to get it throw.

TSC21 commented 1 year ago

@Le5tes Could you please address linter errors in you PR? We are going to archive entire repo very soon and this is last chance to get it throw.

@MichaelOrlov can you please clarify what this means? Are you guys stopping supporting the tool?

MichaelOrlov commented 1 year ago

@Le5tes Yes we are going to archive this repo and deprecate. The rational for that:

  1. There are no volunteers to support it and I don't have enough knowledge and time to support it as well.
  2. We have made a post on Discourse about it in July https://discourse.ros.org/t/call-for-help-maintainership-of-the-ros-cross-compile-tool/26511
  3. Community mostly expressed opinion that it's outdated and inefficient. It was recommended to use the native cross-compilation instead of QUEMU