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

mk depends on rospack #204

Closed
moriarty opened this issue Feb 15, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@moriarty
Copy link
Contributor

commented Feb 15, 2019

$ grep -r "shell rospack" ./core/mk/

./core/mk/hg_checkout.mk:5:#	include $(shell rospack find mk)/hg_checkout.mk
./core/mk/cmake_stack.mk:22:	$(shell rospack find rosbuild)/bin/package_source.py $(CURDIR)
./core/mk/cmake_stack.mk:34:#include $(shell rospack find mk)/buildtest.mk
./core/mk/git_checkout.mk:5:#	include $(shell rospack find mk)/git_checkout.mk
./core/mk/buildtest.mk:10:rosalldeps = $(shell rospack find rospack)/rosalldeps
./core/mk/buildtest.mk:13:rules = $(foreach d, $(deps), cd $(shell rospack find $(d)) && make &&) true;
./core/mk/buildtest.mk:17:rules_test = $(foreach d, $(deps), cd $(shell rospack find $(d)) && make test &&) true;
./core/mk/buildtest.mk:23:rules_all = $(foreach d, $(deps_all), cd $(shell rospack find $(d)) && make &&) true;
./core/mk/buildtest.mk:27:rules_all_test = $(foreach d, $(deps_all), cd $(shell rospack find $(d)) && make test &&) true;
./core/mk/cmake.mk:49:	awk -f $(shell rospack find mk)/eclipse.awk .project-cmake > .project
./core/mk/cmake.mk:51:	python $(shell rospack find mk)/make_pydev_project.py
./core/mk/cmake.mk:57:include $(shell rospack find mk)/buildtest.mk
./core/mk/bzr_checkout.mk:5:#	include $(shell rospack find mk)/bzr_checkout.mk

moriarty added a commit to fetchrobotics/fetch_robots that referenced this issue Feb 15, 2019

[fetch_driver] Makefile and mk depend on rospack
mk uses rospack internally but we don't get the rospack dependency.
ros/ros#204

We also call rospack in Makefile.tarball so we should directly depend on
it instead of assuming it is depended on by ros/core/mk.
@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

I assume you are trying to point out that the mk package lacks a dependency on rospack in its package.xml file? If yes, please consider to create a pull request to address the problem.

moriarty added a commit to moriarty/ros that referenced this issue Feb 16, 2019

@moriarty

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2019

Yes sorry I meant to ask a question if it should be a <build_depend> or <build_export_depend>

I've opened a PR with build_export_depend

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

It only needs to be a build_export_depend since it is not needed at build time of the mk package. Since the manifest uses format 1 that would be a run_depend.

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

Thanks for reporting the problem and creating a PR for it.

@dirk-thomas dirk-thomas added the bug label Feb 16, 2019

dirk-thomas added a commit that referenced this issue Feb 16, 2019

[mk] add depend on rospack (#205)
* [mk] add depend on rospack

Fixes #204

* <depend> instead of <build_export_depend>

Not using package format 2

* build_depend and run_depend

* remove build_depend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.