-
Notifications
You must be signed in to change notification settings - Fork 167
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
Implement support for options with homebrew. #304
Implement support for options with homebrew. #304
Conversation
CI fails as the unit tests are not updated. Moreover the rosdep rules in https://github.com/ros/rosdistro/blob/master/rosdep/osx-homebrew.yaml should be updated if this is merged / released, since the --universal options are probably unwanted on newer os x versions and the options for vtk should also be checked. |
Apart from However, in this case it is maybe more of an "install hint". I.e. if PCL is installed, we don't care what version it is (i.e. we don't enforce "HEAD"), but when it is not installed, we install |
So apart from updating the tests this makes sense to me. @NikolausDemmel do you think it is worth the effort to get this pushed through or no? |
- they are ignored at the moment and therefore have not been used in a long time - on recent OS X these options are certainly wrong, so this is a prerequisite for merging ros-infrastructure/rosdep#304
- as long as ros-infrastructure/rosdep#304 is not merged these will be ignored. If we merge this before rosdep is updated it will make testing that rosdep change slightly more easy. - install_flags will be used for `brew install`, but unlike options they are not checked for already installed packages
- as long as ros-infrastructure/rosdep#304 is not merged these will be ignored. If we merge this before rosdep is updated it will make testing that rosdep change slightly more easy. - install_flags will be used for `brew install`, but unlike options they are not checked for already installed packages - for boost --with-python see ros-infrastructure/rosdep#330 - for pcl --HEAD on Mavericks see - http:https://answers.ros.org/question/173654/cant-install-pcl-with-homebrew-on-osx-109-during-ros-hydro-installation/ - ros/homebrew-hydro#3
- as long as ros-infrastructure/rosdep#304 is not merged these will be ignored. If we merge this before rosdep is updated it will make testing that rosdep change slightly more easy. - install_flags will be used for `brew install`, but unlike options they are not checked for already installed packages - for boost --with-python see ros-infrastructure/rosdep#330 - for pcl --HEAD on Mavericks see - http:https://answers.ros.org/question/173654/cant-install-pcl-with-homebrew-on-osx-109-during-ros-hydro-installation/ - ros/homebrew-hydro#3
- as long as ros-infrastructure/rosdep#304 is not merged these will be ignored. If we merge this before rosdep is updated it will make testing that rosdep change slightly more easy. - install_flags will be used for `brew install`, but unlike options they are not checked for already installed packages - for boost --with-python see ros-infrastructure/rosdep#330 - for pcl --HEAD on Mavericks see - http:https://answers.ros.org/question/173654/cant-install-pcl-with-homebrew-on-osx-109-during-ros-hydro-installation/ - ros/homebrew-hydro#3
- The support for homebrew options seems to have been lost in fa4d0b6. - This commit supports installing options and checking installed options. - Options are accepted if the installed ones are a super set of the ones declared in rosdep. - We also add support for `install_flags`, which are similar to options and get passed to `brew install`, but do not get checked for already installed packages. - Checking installed packages can be slow, as we have to call the `brew` command for each potential package to check if it is really linked and if the options are ok. - There are some limitations to what we can detect, for example we do not check for formulae from wrong taps, we do not preserve already installed options for reinstall of packages (see TODOs in source). - Some scenarios cannot be resolved automatically, e.g. installing the correct options for a package that is also in the dependency list of another homebrew package to be installed (see TODOs in source). - See ros-infrastructure#299 for a discussion of what the semantics should/could be.
@wjwwood: I think we can reuse some of this for xylem, so yes. Latest commit has updated/passing tests. I also added Please review. |
- Change from our mongodb-dev to homebrew maintained mongodb - homebrew maintained mongodb builds and installs libs since Homebrew/legacy-homebrew#25253 - --with-boost option is needed, so depends on ros-infrastructure/rosdep#304 Is there a way to list all released pacakges depending on the `mongodb-dev` rosdep key?
return False | ||
for spec in pkg_info['installed']: | ||
if spec['version'] == linked_version: | ||
# print(r.package, spec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably drop this debugging line.
Other than my notes the code seems to be OK. I still need to test it locally. |
This seems to work for me locally, +1 @tfoote or @dirk-thomas can we get a review from one of you too? |
Sorry @NikolausDemmel we are really behind over here, I'm doing my best to get another review on this, and then I'll try to get it into a release. I appreciate your help tracking and pushing this along. |
Ok, I'm going to merge this, I'll plan to do a release today if I have time, otherwise it may be Monday before I get to it. |
Implement support for options with homebrew.
Thanks! |
We were waiting on the farm to get sync'ed up before I did a release, @tfoote can I release now? |
I guess I need to release rosdep again with this pull request. I was reinstalling everything with rosdep and boost didn't get the |
|
thanks |
declared in rosdep.
brew
command for each potential package to check if it is really linked and if the
options are ok.
for formulae from wrong taps, we do not preserve already installed options
for reinstall of packages (see TODOs in source).
options for a package that is also in the dependency list of another homebrew
package to be installed (see TODOs in source).
of what the semantics should/could be.