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

[WiP] Fix upstream downstream package logic #137

Closed

Conversation

mathias-luedtke
Copy link
Member

@mathias-luedtke mathias-luedtke commented Mar 6, 2017

addresses #106

some unrelated minor fixes are bundled as well:

The dependency helper covers most of the designed logic, but uses a consolidated rosdep install step for now.

It detects the target and downsteam packages automatically, if these are not provided.
The detection can be filtered by BUILD_PKGS_WHITELIST and BUILD_PKGS_BLACKLIST (new).
The upstream packages cannot be black- or whitelisted.

Based on all upstream, target and downstream packages the list of rosdep src paths gets compiled.
In addtion the list of tests gets generated and can be filtered with TEST_BLACKLIST (new).

The resulting package list gets printed outside of the folds, including the ignores packages.
As unrelated packages get irgnored now, the travis jobs had to be extended to enforce the downstream packages.

All steps are run in separated folds, so failures can be attributed more easily.
I took the chance to fix #118

(The list of commits is a little bit mixed up - sorted by date instead of relation - in the PR display because of the rebases)

TODO:

  • documentation
  • setting DOWNSTREAM _PKGS should prevent the auto-detection
  • run rostest with env loader (removed source setup.bash by accident)
  • make deps.py a proper module
  • add unit tests for deps.py

@mathias-luedtke
Copy link
Member Author

For some reasons the actionlib tests fail.
This raised the question: Should downstream tests be exercised? Do we want an opt-out flag?

@fmessmer
Copy link
Contributor

fmessmer commented Mar 7, 2017

Tested this together with @ipa-mdl on branch ipa320/cob_robots#626
Showed that transitive dependencies are not handled correctly as they eventually end up in the Ignored packages set

@mathias-luedtke
Copy link
Member Author

I have rebased to current master and updated the logic.

catkin's topological_order does not respect exec dependencies in the ordering.So some dependencies were skpipped. This okay for the sake of the builds and tests, but rosdep installs all dependencies and got into trouble.

@mathias-luedtke mathias-luedtke force-pushed the enhancement/106 branch 2 times, most recently from a4a6d6e to a1caec5 Compare March 8, 2017 03:20
@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Mar 8, 2017

The latest update improved the handling of export and test depends.
exec_depends are now treated as transitive test_depends.
In addtion uses exec_depends get skipped in rosdep install if applicable.

@ipa-fxm: please test again. On my laptop the full run of the kinetic build/test takes 20 minutes.

@fmessmer
Copy link
Contributor

fmessmer commented Mar 8, 2017

See my ipa320/cob_robots#626 (comment)...

@mathias-luedtke mathias-luedtke force-pushed the enhancement/106 branch 3 times, most recently from 5c7cc0d to b8676af Compare March 8, 2017 19:24
@mathias-luedtke mathias-luedtke mentioned this pull request Mar 9, 2017
26 tasks
@mathias-luedtke mathias-luedtke force-pushed the enhancement/106 branch 2 times, most recently from f6baab0 to ef29214 Compare March 11, 2017 13:42
@mathias-luedtke
Copy link
Member Author

The same technique could be used for prerelease tests (#145)

@mathias-luedtke
Copy link
Member Author

rebased on top of #150

@mathias-luedtke
Copy link
Member Author

rebased to current master, works with ROSDEP_SKIP_KEYS

opt-out with STRICT_TEST_DEPENDS=true
@mathias-luedtke
Copy link
Member Author

relaxed mode is default, strict mode can be enabled with STRICT_TEST_DEPENDS=true.

@mathias-luedtke
Copy link
Member Author

as discussed with @ipa-fxm downstream packages should not get detected automatically

@fmessmer
Copy link
Contributor

fmessmer commented Feb 2, 2018

having fixed our issue with the travis log length in #267, I would also like to see our travis build time reduced

what is the status of this feature/pr?
I'm willing to help getting this ready to merge

@ipa-mdl
can you summarize what still needs to be done or what is currently preventing this from being merged (despite needing to rebase)?

@130s
Copy link
Member

130s commented Mar 15, 2018

I'm also interested in this feature.
Do you have any plan to come back to this @ipa-mdl?

@mathias-luedtke
Copy link
Member Author

Do you have any plan to come back to this

Not in this form. I think we can migrate some of it to our current codebase. For the actual upstream/downstream detection I would rather use some interfaces of catkin-tools directly instead of my custom code.

@mathias-luedtke
Copy link
Member Author

Proper handling of upstream and downstream workspaces is implemented in #361

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.

test_results block never reaches upon failure in catkin_run_tests
3 participants