PPAとROSのdebでインストールした時の違い

YoheiKakiuchi commented 9 years ago

pythonのモジュールに関してですが、 PPAのパッケージは/usr/lib/python2.7/dist-packages以下にxxx_idl.pyができます。 ROSのパッケージは/opt/ros//lib/python2.7/dist-packages/hrpsys 以下に入ります。

ROSを使う方は from hrpsys import _idl と書いてあるかと思いますが、PPAのパッケージではこれではだめで、 import _idl とかくようになっており記法に互換性が無いように思います。

また、PPAのパッケージはパッケージインストール時に _idl.py等を作成しており、removeしてもそれらのファイルが残ってしまって、完全な削除が難しいかと思います。

取り急ぎ、はまりポイントがあったので情報共有いたします。

yosuke commented 9 years ago

情報ありがとうございます。早急に改善します。

yosuke commented 9 years ago

PPA側についてもサービス系のidlからのstubは/usr/lib/python2.7/dist-packages/hrpsysに作るように変更しました。

また今までなかったprermスクリプトも作成し、パッケージをremoveするとstubも削除するように設定しました。

この形で問題ないだろうとは思っているのですが、問題出そうでしたら指摘ください。 > @fkanehiro さん

設定を変更していて気持ちが悪い部分を思い出したのでついでに治したいと思うのですが、今はOpenHRPのidlがhrpsysのパッケージの中でstub生成されています。これはOpenHRPのパッケージ側で生成するのが本来のところではないかと思います。

また、import OpenHRPという流儀とfrom hrpsys import OpenHRPという流儀が混在しており、互換性のために両方の場所にstubを生成しているのですが、これは互換性を残しつつも、いずれimport OpenHRPに変えていく方向が良いのではないかなと思います。

この2点について、昔議論したかなと思いつつも改めて確認してから進めようと思うのですが、識者の意見をお聞かせ願えないでしょうか？ > @fkanehiro さん @k-okada さん

yosuke commented 9 years ago

事前にPPAのOpenHRPのパッケージを見てみたのですが、dist-packages/OpenHRPが事後生成でなくパッケージ内に入っている（パッケージビルド時にcmakeで生成されている）ようです。これについてはパッケージインストール時の事後生成のほうが良いという議論（ @fkanehiro さんからの希望）があったと思いますし、そちらのほうがより安全なのは間違いないと思いますので、そのようにしたいと思います。

ROSパッケージに影響が出ないようにOpenHRPレポジトリのcmakeはいじらずパッケージ側の処理で省いておきます。

yosuke commented 9 years ago

これが綺麗にかけていないのも、このパッケージ化の問題っぽいですね。

ref #760

yosuke commented 9 years ago

ref fkanehiro/openhrp3#36

yosuke commented 9 years ago

OpenHRPのcmakeはstubを整理するためにdist-packages/OpenHRPフォルダに入れる設定になっているのですが、module宣言を使ったidlの場合、omniidlが配置してくれるファイルレイアウトでないとうまく機能しないので、dist-packagesフォルダが汚くなってしまうのですが、そちらにstubを配置できるとうれしいです。

とここまで考えて朝やった作業がまずいかなと思い始めたのですが、hrpsys-baseのidlもmodule宣言を使ったものがありますね、、、。これを考えると、dist-packages/hrpsys以下に生成させる今のアプローチはあまり上手くないかもしれません。

フォルダの綺麗さを取るかPythonの機能の完全性を取るかというところなのですが、omniidl的に言うと今ほとんどのhrpsysのidlはmodule OpenHRPを使ってしまっているので、それをhrpsysというフォルダに入れること自体がかなりねじれが出ていますね。本当はmodule hrpsysを使ってomniidlの出力するファイルレイアウトで素直にhrpsysフォルダに出力させるべき。

yosuke commented 9 years ago

ref #530

yosuke commented 9 years ago

@k-okada さん @fkanehiro さん

関係する各所を見て回って、問題が整理できてきたのでまとめておきます。

omniidlが出力するpython stubはOpenHRP/init.pyと_idl.pyの2つがあり、双方、絶対パスで参照しあっており、ファイルレイアウトは守った状態でコピーする必要がある。このファイルレイアウトを崩した形でコピーしても、オブジェクトの生成などは動作する。だた、 #530 にあるような定数の参照などが機能しなくなってしまう。

これらのstubは絶対パスで参照しあうので、ファイルレイアウトをそのまま/usr/lib/python2.7/dist-packages/直下にコピーする必要がある。ただ問題としては_idl.pyファイルが大量にできるので見た目が汚くなり、削除をサポートするためのdebianスクリプトも複雑になる。

上記を解決するために/usr/lib/python2.7/dist-packages/hrpsysに置くこともできる。ただ、from hrpsys import _idlとした場合、オブジェクトの生成はできるが定数の参照はできず不完全である。

上記のworkaroundとして/usr/lib/python2.7/dist-packages/hrpsysをPYTHONPATHに入れてしまうこともできる。以下のコードは期せずして上記のworkaroundを実現しており、

https://github.com/fkanehiro/hrpsys-base/blob/master/python/__init__.py#L2

例えば、以下のようにscriptを書くと定数の参照を含めた完全な機能を利用できるはずである。

import hrpsys
import OpenHRP # importの順番が重要

OpenHRP/init.pyには生成された_idl.pyが列挙されている。問題としてはOpenHRP由来のIDLとhrpsys由来のIDLが同じmodule OpenHRPを使ってしまっており、init.pyにはその両方が書かれていなければならない。パッケージインストール時にomniidlを使ってstubを生成する必要があるのはこのためである。

パッケージ削除時を考えると上記のinit.pyの管理は複雑なものになる。システムにここまでインストールされたIDLの情報をどこかに保存しておいて、削除時にも再生成をかける、、、というのが本来の実装であると思われる。

理想はhrpsys関連のIDLはmodule hrpsysを使うことである。これであれば_idl.pyの大量生成で汚くなる以外の問題は全て解決する。ただ、module OpenHRPを前提にしたコードはhrpsys全体に利用されており、この変更の影響はかなり大きい。

yosuke commented 9 years ago

stubがdist-packages以下に大量に生成されて汚くなってしまう問題については-Wbpackageオプションを使うことで指定したパッケージ名以下に配置することもできるようです。

http://omniorb.sourceforge.net/omnipy42/omniORBpy/omniORBpy005.html

yosuke commented 9 years ago

-Wbmodulesオプションというのももしかしたら使えるかもしれません。

IDL上ではmodule OpenHRPになっている場合でもPython上ではhrpsysパッケージとして生成することができるかも。

yosuke commented 9 years ago

ひとりごと状態で進行していてすいません（書くと論考が進みます）。

-Wbpackageオプションを使うとスタブとmodule両方が指定したパッケージ名以下に配置される。-Wbpackage=hrpsysとすると_idl.pyはすべてdist-package/hrpsys以下に配置されるし、from hrpsys import OpenHRPを定数の参照を含めた完全な状態で利用することができる。何個か前で書いたPYTHONPATHを使った方法はtrickyな方法で混乱が多いと思うのでこちらのほうがbetterだと思う。

hrpsysと同時にOpenHRPもあり、OpenHRPについてはimport OpenHRPとして使いたい。hrpsys側で-Wbpackageオプションを使っておけばOpenHRPとhrpsys.OpenHRPは明確に区別できるので名前空間の混乱は起きない。スタブが汚くなる問題に関しては-Wbstub=OpenHRPとすることでスタブのみをOpenHRPフォルダ以下に配置すれば綺麗になる。

yosuke commented 9 years ago

-Wbstubを使ってスタブを配置するパッケージ名は-Wbstub=OpenHRPとしてmodule名と同じにすることができない仕様のようである。

-Wbpackageについては機能するので、-Wbpackage=openhrpとしてfrom openhrp import OpenHRPとして使うか、-Wbstub=OpenHRP_stubsとして別の場所でまとめて管理しておくかのどちらかになる。

ここは利用者の好みに応じて決めたい。

yosuke commented 9 years ago

-Wbpackageオプションをつけて生成すればこの問題も解決するはず。

ref #145

yosuke commented 9 years ago

hrpsys-baseパッケージについては-Wbpackageを使った方法で下位互換性も含めて問題ないと思うのでアップデートしておきました。

以下の部分のコードがあるので、import hrpsysするとimport OpenHRPがopenhrpとhrpsysどちらからimportされるかわからない（Pythonの仕様に依存する）問題があります。

https://github.com/fkanehiro/hrpsys-base/blob/master/python/__init__.py#L2

pathにappendしているのでhrpsysのほうが優先され、hrpsysのstubは両方のstubを含んでいるので問題ないと思うのですが、できれば、この行は無くした方が良いかなと思うところです。 > @k-okada さん

OpenHRPのパッケージについては方針が決まったら変更します。

yosuke commented 9 years ago

上記のIDL生成コマンドラインは以下になります。

-Wbinlineオプションもつけたほうが、よりportableになるようなのでつけてあります。

http://bazaar.launchpad.net/~hrg/hrg-packaging/hrpsys-base-deb/view/head:/debian/postinst

yosuke commented 9 years ago

debian officialのpython-omniorb-omgパッケージを見てみると、stubはパッケージ作成時に生成＆-Wbstubsオプションなどもつけずに生成してある。hrpsysはパッケージインストール時にstubを生成する必要があるが、OpenHRPについてはこの事前生成方式のままでも問題ないかもしれない。

/var/lib/dpkg/info/python-omniorb-omg.list

ただし、今はdist-packages/OpenHRP以下にinit.pyだけでなく_idl.pyも入ってしまっているので、それは配置を戻す必要がある。

yosuke commented 9 years ago

上記の-Wbpackageを付けて生成したパッケージで

import hrpsys.rtm

するとImportError: cannot import name CdrDataというエラーが出る。

import hrpsys
import rtm

するとエラーはない。エラーが消えるのはimport hrpsysによってPYTHONPATHが設定されているからであるが、PYTHONPATHを設定すると他の副作用も出るためBad practiceだと思われる。

本来はエラーを起こしている箇所

from OpenRTM import CdrData, OutPortCdr, InPortCdr

を以下のように書き換えるべきである。

from hrpsys.OpenRTM import CdrData, OutPortCdr, InPortCdr

ただし、これはROSパッケージ側の動作を崩してしまうので、合意が必要。

openrtm-aist-pythonにdist-packages直下にOpenRTMスタブを生成させる方法も綺麗な解決法かもしれない。

yosuke commented 9 years ago

OpenRTM-aist-pythonについても自分のパッケージ下にstubを置くためにPYTHONPATHを使ってしまっており、これも同様にBad practiceである。

http://svn.openrtm.org/OpenRTM-aist-Python/branches/RELENG_1_1/OpenRTM-aist-Python/OpenRTM_aist/__init__.py

本来は-Wbpackageを使うかdist-packages直下に置くかどちらかにすべきであるが、ソースの管理がgithubでないので、開発者とのコミュニケーションが難点である。

yosuke commented 9 years ago

OpenHRPについてはマージ fkanehiro/openhrp3#69 でimport OpenHRPで完全に動くようになるはず。

OpenRTMのスタブについてはPPAのopenrtm-aist-pythonパッケージでdist-packages直下に生成するようにして、現在、ビルド待ち（launchpadが混雑しているようでビルドが遅い）。

hrpsys-baseについては-Wbpackage=hrpsysオプションを付けたのでこれもimport hrpsys.OpenHRPで完全に動くようになるはず。

すべてのビルドが通ったらapt-get update; apt-get upgradeして要確認。

yosuke commented 9 years ago

一晩たってopenrtm-aist-pythonのビルドは完了

hrpsys.OpenRTMのスタブの内容が足りなかったので補強

ざっと動かした感じではうまく機能していそう

yosuke commented 9 years ago

作業一段落したので様子見モードに入ります。自分でも使って確かめていきますが、変な動作がありましたら @yosuke にmentionして教えてください。

@YoheiKakiuchi @fkanehiro @k-okada

yosuke commented 9 years ago

Pythonはsys.path.appendするとライブラリのサーチパスの末尾にパスが追加され

import hrpsys
import OpenHRP

とした場合のOpenHRPがdist-packages直下のOpenHRPになり、hrpsys.OpenHRPが優先されてくれない。

従来のコードとの互換性を考えると、以下の部分のコードは

https://github.com/fkanehiro/hrpsys-base/blob/master/python/__init__.py#L2

以下のようにしてパスの先頭に追加されるようにしたほうが良いかもしれない。

sys.path.insert(0, os.path.dirname(__file__))

k-okada commented 9 years ago

ようやくメールが追いつきました．

http://bazaar.launchpad.net/~hrg/hrg-packaging/hrpsys-base-deb/view/head:/debian/postinst は https://github.com/fkanehiro/hrpsys-base/blob/master/idl/CMakeLists.txt#L125 と同じでしょうか．わかっていないですが，ppaでパッケージングの時にこのidl/CMakeLists.txtでmake installははしらないのでしょうか．

私としては hrpsys_confg.py （ https://github.com/fkanehiro/hrpsys-base/blob/master/python/hrpsys_config.py ) 以外に https://github.com/start-jsk/rtmros_common/blob/master/hrpsys_tools/scripts/hrpsys_tools_config.py https://github.com/start-jsk/rtmros_hironx/blob/hydro-devel/hironx_ros_bridge/src/hironx_ros_bridge/hironx_client.py あたりが使われているところで，この辺を直して https://github.com/fkanehiro/hrpsys-base/blob/master/test/test-samplerobot.py#L20 https://github.com/start-jsk/rtmros_hironx/blob/hydro-devel/hironx_ros_bridge/test/test_hironx.py#L15 あたりのコードに手を付けなくていいのであれば，問題はないのですが，そうはかなそうでしょうか．

openrtm-pythonについてはどうしますか．こちらでは https://github.com/tork-a/openrtm_aist_python-release/blob/release/indigo/openrtm_aist_python/CMakeLists.txt でつくっています．そちらで使っているのは http://www.openrtm.org/openrtm/ja/content/ubuntudebian%E3%81%B8%E3%81%AE%E3%82%A4%E3%83%B3%E3%82%B9%E3%83%88%E3%83%BC%E3%83%AB#toc4 とは別バージョン！？直すか直さないか，ソースを直すか，パッケージだけ直すか，また，この変更で https://github.com/fkanehiro/hrpsys-base/issues/764#issuecomment-134045292 の部分を直す必要が有るというこになりますか．

とりあえずテスト作りました．

https://github.com/fkanehiro/openhrp3/pull/70 https://github.com/fkanehiro/hrpsys-base/pull/770

◉ Kei Okada

2015-08-25 9:41 GMT+09:00 Yosuke Matsusaka notifications@github.com:

作業一段落したので様子見モードに入ります。自分でも使って確かめていきますが、変な動作がありましたら @yosuke https://github.com/yosuke にmentionして教えてください。

@YoheiKakiuchi https://github.com/YoheiKakiuchi @fkanehiro https://github.com/fkanehiro @k-okada https://github.com/k-okada

— Reply to this email directly or view it on GitHub https://github.com/fkanehiro/hrpsys-base/issues/764#issuecomment-134426984 .

yosuke commented 9 years ago

大部分、手を加えないで動くように作ってはいるつもりです。

openrtm-pythonについてはsvnの1_1ブランチを使っています。ソースは未修正でdebianパッケージ作成スクリプトでdist-packages直下にスタブの生成を追加した（従来のスタブも従来と同じ場所にある）ので、この部分は変更なしで問題なく動作するはずです。

import OpenHRPのところだけがOpenHRP/init.pyの管理の複雑さ（特にパッケージ削除時の）から逃げるために、hrpsys-baseのサービスについてはimport hrpsys.OpenHRPを使わないといけない状況です。

debianパッケージで頑張って削除時にも対応できるimport OpenHRPを作れればよいのですが、、、。

yosuke commented 9 years ago

IDL生成はdebianパッケージのみに存在する部分なので、以下とは同期が取れていないですね。

https://github.com/fkanehiro/hrpsys-base/blob/master/idl/CMakeLists.txt#L125

なので、テストはこれも直してからやったほうが良いかもしれません。

気持ちとしてはhrpsysフォルダの下にファイルをまとめたい、ということで一致しているのですが、よりomniORBにフォルダ構成を理解させた状態でスタブを生成させる、というのが-Wbpackageを使った方法です。

yosuke commented 9 years ago

cmakeをdebianパッケージでやっている処理と同じにしてみました。テスト通るかしら。

https://github.com/fkanehiro/hrpsys-base/pull/771

fkanehiro / hrpsys-base

PPAとROSのdebでインストールした時の違い #764