search

gazebosim / gz-tools

Command line tools for the Gazebo libraries.
https://gazebosim.org
Apache License 2.0
15 stars 18 forks source link

Fix use of Backward on macOS #67

Closed azeey closed 2 years ago

azeey commented 2 years ago

🦟 Bug fix

Summary

SDFormat CI (eg. https://build.osrfoundation.org/job/sdformat-ci-sdformat10-homebrew-amd64/48/) is failing on macOS because ign fails to find libignition-tools-backward. The problem is that $<TARGET_FILE> yields the full path of the library in the build directory, which will likely not be available in binary distributions of ign-tools. This instead uses the install path of the library.

Checklist

Note to maintainers: Remember to use Squash-Merge

scpeters commented 2 years ago

in downstream packages, we typically configure/generate two cmd*.rb scripts, one for testing and one for installation:

I think we should consider doing that here and adding a test that invokes ign

azeey commented 2 years ago

I think we should consider doing that here and adding a test that invokes ign

I've added tests that use cmake to invoke ign, but I don't know how to tell Jenkins about the test results. Do you have any recommendations? The other option is to add gtest and call the tests from an executable built with gtest.

scpeters commented 2 years ago

I think we should consider doing that here and adding a test that invokes ign

I've added tests that use cmake to invoke ign, but I don't know how to tell Jenkins about the test results. Do you have any recommendations? The other option is to add gtest and call the tests from an executable built with gtest.

here's an example from ign-cmake2 that configures junit files for pass and failure. It configures the failure case to the test_results folder, then copies the junit_pass file if the test succeeds:

it's a little clunky, but it doesn't need gtest

azeey commented 2 years ago

The changes in https://github.com/ignitionrobotics/ign-tools/pull/67/commits/311d5881e782542a2116c16aade25ea001e7b74b output test results that I thought were consumable by Jenkins based on the examples listed in https://github.com/ignitionrobotics/ign-tools/pull/67#issuecomment-947460012. But Jenkins still says no tests found.

Also, I see a pending ign_tools-pr-win build as well a completed ignition_tools-ci-pr_any-windows7-amd64 in the list of checks. Is that related to https://github.com/ignition-tooling/release-tools/pull/537 @scpeters?

scpeters commented 2 years ago

The changes in 311d588 output test results that I thought were consumable by Jenkins based on the examples listed in #67 (comment). But Jenkins still says no tests found.

there's a DSL change needed to expect tests from ign-tools

https://github.com/ignition-tooling/release-tools/blob/master/jenkins-scripts/dsl/ignition.dsl#L41

Also, I see a pending ign_tools-pr-win build as well a completed ignition_tools-ci-pr_any-windows7-amd64 in the list of checks. Is that related to ignition-tooling/release-tools#537 @scpeters?

yes it's related to https://github.com/ignition-tooling/release-tools/pull/537. I'll enable tests for ign-tools in that PR and then request your review

scpeters commented 2 years ago

@osrf-jenkins run tests please

scpeters commented 2 years ago

the ign_tools-pr-win build won't work until https://github.com/ignition-tooling/release-tools/pull/537 is merged since it can only run off the master branch of release-tools

scpeters commented 2 years ago

tests are working on ubuntu: https://build.osrfoundation.org/job/ignition_tools-ci-pr_any-ubuntu_auto-amd64/191/testReport/(root)/

azeey commented 2 years ago

@osrf-jenkins run tests please

scpeters commented 2 years ago

there's one more change needed in release-tools: https://github.com/ignition-tooling/release-tools/pull/540

I had already restarted a CI job and saw its failure

scpeters commented 2 years ago

there's one more change needed in release-tools: ignition-tooling/release-tools#540

I had already restarted a CI job and saw its failure

this has been merged and CI is now working

scpeters commented 2 years ago

@osrf-jenkins run tests please

Blast545 commented 2 years ago

FYI @azeey, the error is still appearing in the buildfarm: build.osrfoundation.org/sdformat-ci-sdformat9-homebrew-amd64#81/. Not sure if I'm missing something.

azeey commented 2 years ago

We need to make a patch release for macOS to fix it there. The plan is to make a release for all platforms today pending https://github.com/ignition-release/ign-tools-release/pull/2. If that doesn't get merged today, we'll do a patch release just for macOS. \cc @j-rivero