おがさわらなるひこのオープンソースとかプログラミングとか印刷技術とか

おがさわらなるひこ @naru0ga が技術系で興味を持ったりなんだりしたことをたまーに書くブログです。最近はてなダイアリー放置しすぎて記事書くたびにはてな記法忘れるのではてなブログに移行しました。

クリエイティブ・コモンズ・ライセンス
特に断りがない場合は、本ブログの筆者によるコンテンツは クリエイティブ・コモンズ 表示 - 継承 4.0 国際 ライセンスの下に提供されています。

はじめてのLibreOfficeソースコミット

LibreOffice Development OpenSource Programming

2014.11.05 追記：とても重要な参考資料を紹介するの忘れていたので後ろにくっつけます。見てくださいね。


私は口先野郎なので、「LibreOfficeの日本コミュニティに一番必要なのはコード書く人。オープンソースはコード書く人が正義*1」とか言っていながら、自分はぜんぜんコード書いてなかったんですよね。これでも元はプログラミングでおまんまを食べてた人間として、これはいかんよなーってずっと考えてた。

開発に挑戦することに対してはいろんなハードルがあるだろうけど、LibreOfficeの場合はコードベースも巨大だしコミュニティもいろんな人が関わってるのでそれなりのお作法もある。正直、どこから手を付けていいかわかんないところがあります。で、私的には、もちろん開発そのものの難しさもあるけれども、それ以前に、とにかく自分の修正のパッチを送ってレビュープロセス通してって、人が介在するところに心理的に抵抗があったわけです。英語では丁寧にドキュメント化されてるのだけど、私英語得意じゃないしね。

で、これは誇っていいことだと思うんですが、LibreOfficeにはEasy Hacksという「開発入門者のための誰でも直せそうなバグ」をタグ付けしたものがあって、もうほんっとに簡単なバグ（で、緊急性が高くない奴）とかを元に、修正してコンパイルしてレビューシステムにpushしてレビュー受けて、ってことをできるようになってる。よし、これにチャレンジしようと。

LibreOffic Conference 2014 Bernのときから初めて、ついついサボってたのでこれはイケナイと思い立ち、自分の尻叩きのために関東LibreOfficeHackFest(#1) with 第119回東京エリアDebian勉強会というイベントをやって*2 ようやっと目鼻がつき、こないだ送ったパッチは無事に取り込まれました。

すっごいしょぼい修正で恥ずかしいのですが一応コミッター*3 の仲間入りをしたので、記録として残しておきます。

課題の選定

Easy HacksはAll LibreOffice Easy Hacks by required Skillというページで必要なスキルごとに分類されているんですが、今回は曲がりなりにも昔はそれでご飯食べてたC++の、超簡単レベルな奴からピックアップ。

Bug 43157 - Clean up OSL_ASSERT, DBG_ASSERT, etc.

Bug Descriptionから引用すると：

The assertion/logging functionality from osl/diagnose.h (OSL_TRACE, OSL_ASSERT, OSL_ENSURE, OSL_FAIL) and tools/debug.hxx (DBG_ASSERTWARNING, DBG_ASSERT, DBG_BF_ASSERT, DBG_WARNING, DBG_WARNING1--5, DBG_WARNINGFILE, DBG_ERRORFILE) is obsolete and needs to be cleaned up:

  • To assert invariants of the code (that can only be violated if there are programming errors) use standard C/C++ assert.
  • To log warnings about unusual events (that the code nevertheless needs to handle in some way, like malformed input or I/O failures), use the SAL_WARN... macros from sal/log.h.
  • To log other information useful for debugging, use the SAL_INFO... macros.

See https://wiki.documentfoundation.org/Development#Assertions_and_Logging, the mail thread at http://lists.freedesktop.org/archives/libreoffice/2011-November/020864.html, and the documentation in the sal/log.h header for further information.

要は古くて非推奨になったアサーション・ロギングマクロを、今のスタイルに書き直しましょう、ってだけ。機械的な置換でいけそうだからこれなら自分でもできるやろと。

ビルド環境構築

まずはgitでソースコードをまるっと取って来ます。で、ビルドに必要なライブラリをがっと取ってきて、一回ビルド通します。LibreOfficeのフルビルドは環境にも寄りますけど数時間かかるので、ビルド投入して寝ちゃうのがいいでしょう。

$ git clone git://gerrit.libreoffice.org/core LibreOffice
$ sudo apt-get build-dep libreoffice
$ cd LibreOffice
$ ./autogen.sh --enable-gstreamer --disable-gstreamer-0-10 --with-lang="ALL"
$ make

ビルド終わったらちゃんと動くことを確認します。

$ instdir/programs/swriter &

ヘルプ→LibreOfficeDevについて、を確認。

大丈夫そうですね。

コードの修正

今回の修正は色んなところのソースをお掃除しましょうというネタなので、どこ直してもまあいいわけです。なので直せそうなところをgrepで探しましょう。

豆知識としては、LibreOfficeはビルド時に依存するライブラリのバージョンを固定するために、サーバーにおいてあるtar ballを落としてきてworkdir/UnpackedTarballというところに展開します。その他yaccの生成物とかもあるので、単にgrep -r するとそういうのがいっぱい引っかかってしまうので、git grep使うほうが100倍ぐらい賢いです。
ということで、git grep OSL_ したら、あるわあるわ。どれ直してももちろんよいのですが、一部のソースは特定のconfigureオプションをオンにしないとコンパイルされない（そして、そのconfigureオプションはほとんど誰も使っていない）とかある*4 ので、ちょっとだけ注意が必要です。今回は pyuno/source/module/pyuno.cxx というソースを直すことにしました。変更が2行しかないのでｗ

ターゲットが決まったらまずは作業ブランチ切りましょう。

git checkout -b work

修正はバグ票にあった他のコミットを見ながら、こんな感じに書き換えればいいのかなと*5。

--- a/pyuno/source/module/pyuno.cxx
+++ b/pyuno/source/module/pyuno.cxx
@@ -66,7 +66,7 @@ void PyUNO_del (PyObject* self)
 
 OUString val2str( const void * pVal, typelib_TypeDescriptionReference * pTypeRef , sal_Int32 mode )
 {
-    OSL_ASSERT( pVal );
+    SAL_WARN_IF( !pVal, "pyuno", "pVal != NULL" );
     if (pTypeRef->eTypeClass == typelib_TypeClass_VOID)
         return OUString("void");
 
@@ -124,7 +124,7 @@ OUString val2str( const void * pVal, typelib_TypeDescriptionReference * pTypeRef
         buf.append( "{ " );
         typelib_TypeDescription * pTypeDescr = 0;
         TYPELIB_DANGER_GET( &pTypeDescr, pTypeRef );
-        OSL_ASSERT( pTypeDescr );
+        SAL_WARN_IF( !pTypeDescr, "pyuno", "pTypeDescr != NULL" );
 
         typelib_CompoundTypeDescription * pCompType = (typelib_CompoundTypeDescription *)pTypeDescr;
         sal_Int32 nDescr = pCompType->nMembers;

で、make。フルビルド済ませてあるので、今度はそんなにかからないです。本当はmake checkでユニットテストも通すべき（そもそも、このコードにユニットテスト書くべき）なんですが、私が作業したときのmasterはなぜかmake checkすると通らない（よくあることのようです ^^;）ので、うーんと悩んだ末そのまま。

とりあえず、ローカルの作業ブランチにコミットしておきます。コミットコメントはこんな感じ。バグ修正のときはバグ番号を書くのが決まりです。

fdo#43157 - Clean up OSL_ASSERT, DBG_ASSERT
    
- Clean up OSL_ASSERT

gerritの設定

LibreOfficeでは、Googleが開発したソースコードレビューシステムGerritを使用しています。gerritの持つgitリポジトリにpushすると、その上でdiffを確認しつつ、レビューコメントがついたり質疑応答したりできるシステムです。ビルドbotも走っていて、そのコミットでビルドがぶっ壊れないかも確認してくれます。超便利。MLにパッチ投げて議論して、ってのもいいけど、gerritを使うほうが推奨されてる……はず。

ので、まずはgerritの設定するところから始めます。TDF WikiのGerritの解説ページを見ながら。

最初は https://gerrit.libreoffice.org にてアカウント作成ですが、これは大昔にやったことがあったので省略。Googleアカウントがあれば簡単だった記憶が。で、アカウント設定でgit push用のssh公開鍵を設定しておきます。もちろん専用の鍵ペア作ってもかまいません。

次に、解説ページによると、

You must set your username to match your freenode IRC nick (see 'Username' in your gerrit account settings).

とあるので、freenodeのNick登録しなきゃいけません。IRC技能が低い私はここでしばらく調べまわったのですが、自分 irssi 使ってるので、

/msg NickServ REGISTER <password> youremail@example.com

しといて、

/NETWORK ADD -autosendcmd "/^msg nickserv id <username> <password>;wait 2000" Freenode

でFreenodeのNickとGerritのアカウント名を合わせればいい……のかな。今んとこそうしてます。ただ、irssi常時上げてるわけじゃないし、irssiもnotify上げてくれるわけじゃないので実のところIRCマトモに使えてない。なんかいい方法ないすかね。ま、それはまた別の話。

で、先の解説ページに戻りまして、手動セットアップします。まずは.ssh/configに：

Host logerrit gerrit.libreoffice.org
       IdentityFile /path/to/your/private-key
       User <username>
       Port 29418
       HostName gerrit.libreoffice.org

を追記。秘密鍵のところは、さっきGerritに登録した公開鍵とペアになる鍵を指定しましょう。当たり前ですね。

で、LibreOfficeのソースディレクトリに移動しておいて、

$ ./loggerit test

とやって、"Your gerrit setup was successful!" と表示されればOK。

さてgitにpush先を指定しておきましょう。

$ git config remote.origin.pushurl ssh://logerrit/core

これで準備完了です。

いよいよpush……の前に

なんだかんだで、ソースコードを修正してからココらへんの設定をのたのたやってたので、まる2日ぐらい経ってたのかな？

LibreOfficeは進化が早いソフトウェアなので、2日も経つと当然masterには大量に変更が溜まってる。まあ、ビルド通らなくなるとは思えないけど、もし同じファイルを修正した人がいたりしたら悲しくなるので、念のためmasterを最新にして、自分の変更をその先頭にぶら下げるようにしたい。

私、git力も低いので、ホントはこういうときってrebaseでできるのかもなあって思いつつ、悩んだ末masterをpullして最新にした後新たにブランチ切って、そこに作業ブランチの変更をcherry-pickするようにしました。いいのかなあ、こんなやり方で。

$ git checkout master
$ git pull origin master
<ぞろぞろとファイルが降りてくる>
$ git checkuout work
$ git log --oneline | head
<先頭は自分のコミットなのでハッシュ値を見ておく>
$ git checkout -b fdo43157 master
$ git cherry-pick <さっき確認したハッシュ値>
$ make
<ちゃんとmake通ってできたものが動くこと確認>

Gerritにpush

ドキドキしながらpush……してもいいのですが、いきなり投げるのはちょっと怖いので、なんか稽古場があった気がする……って読んだら、

<ローカルブランチ名>:refs/for/master の代わりに <ローカルブランチ名>:refs/drafts/master に投げるとドラフトとしてコミットできるよ

って書いてある。これはドラフトに一旦おいて議論するみたいな使い方をするところなのかなってのも思うのですが、まあ、突っ込んですぐ消すなら大丈夫かな、と……。

で、今の場合は：

$ git push origin fdo43157:refs/drafts/master

すると、「このURLに登録されたよ！」って出るので、そこを見に行くと、draftsにちゃんとコミットがあることが確認できる。

ホントは、このdraftsにadd reviewerしてレビュープロセスに移動するのが正しい使い方らしいのですが、誰をレビュアーにするとかプロセスがわかんなかったので、一度abandonして再度pushすることにしました。いいのかな。不安なので、みなさんはポリシー各自確かめてね。とにかくうりゃっとpush！

$ git push origin fdo43157:refs/for/master

すると、こんな感じで登録されます。あ、これレビューコメント色々もらって修正したやつなので最初とは変わってます。古いパッチも見られますけどね。最初にpushしたのはPatch 1。

pushしてしばらく経つと「ビルドbotが起動したよ」「ビルド成功したよ」とメールが来ます。ほっと一安心。あとは人間のレビュアーの登場を待ちます。

ライセンス宣言

おっと、ライセンス宣言もしないといけません。開発者・貢献者リストのページにあるとおり、開発者メーリングリストに：

All of my past & future contributions to LibreOffice may be
licensed under the MPLv2/LGPLv3+ dual license.

というメールを投げる必要があります。著作権譲渡や契約の締結などは不要です。めっちゃカジュアルですね。
本当は先の開発者リストWikiに自分の名前書かなきゃらしいのですが、このWiki超重要なので間違って編集したら怖いな、いわれたらやろうということで一旦保留*6。

レビュー指摘反映

さて、ドキドキしながら待ってたら、コメントがつきました。まずはTakeshi Abeさん*7からの二つのコメント。

  • SAL_WARN_IFの第三引数は「こういう状態だからおかしいよ」ってものをログに残すものだから、条件逆じゃない？
  • SAL_WARN_IFの第二引数に指定するログエリアはinclude/sal/log-areas.doxに定義しないとダメだよ。

二番目の指摘は完全に見落としだったんですが、最初の指摘は……私はなにを考えていたんでしょう。まあ、勘違いとしかいいようがないですね。お恥ずかしや。

次の指摘はバグ#43157のreporterでもあるStephan Bergmannさんによるもの。

  • これ両方とも、NULLだったら後ろ死ぬからワーニングじゃなくてアサートにすべきじゃない？

……むぐ。そりゃそうだ。何を考えていたんでしょうワタクシ。

言い訳するとSAL_WARN_IFとかを定義してるinclude/sal/log.hxxにはアサートに当たるものがなかったのと、同じバグ票に対するこちらのコミットだとOSL_ASSERTをSAL_WARN_IFに書き換えているコードがあったからというのはあるんだけど、それはプログラマー的には思考停止であって言い訳にもならん。あかんわ。

で、間抜けな私はあれ、アサートのマクロってなくない？どれ使えばいいんだろ？と動揺して、日本語メーリングリストで質問したら、「いやいやC++のassert()でいいでしょう」ってCalcハッカーのKohei Yoshidaさんから教えてもらった。……あ。そうだね。そりゃそうだ。というか、バグ票にも " To assert invariants of the code (that can only be violated if there are programming errors) use standard C/C++ assert." って書いてあるじゃんかー。頭悪いな。

ということで、assert() に直した奴を再度コミット。--amend でコミットするとChange-Id:を見て、同じgerritエントリーに登録してくれるとな。素晴らしい。ので、それをpush。えいっ。これがPatch 2。

再びレビュー待ち……そして。

またビルドbotからのメールを受け取ってドキドキしながら待ってたら、「いいみたいだね、でもassert()使うなら #include を明示的にするほうがいいと思うな。この場合は他のヘッダでincludeされてるから問題ないけどね」って指摘をもらった。けど、それは別途直しておくよってことでパッチはacceptされて、無事コミットは取り込まれました。わーい！

http://cgit.freedesktop.org/libreoffice/core/commit/?id=922f2005f34589e60969be3f2bf74e4af58e2e69

感想

  • 仕組みはいろいろ良くできてる。ドキュメントが少し散らかってる感じはあるけど、ちゃんと読めばわかる。
  • Gerritすごく良くできてて感動。
  • みなさん大変親切でありがたや。お世話になりました。
  • 次はもうちょっと機能や不具合に関わる修正に挑戦したいな。


参考資料：Tomofumi Yagiさんの発表資料 (2014.11.05追記)

LibreOfficeへのコミットについては、Tomofumi Yagiさんのスライドが非常に参考になるので紹介しておきます。私が書かなかったOpenGrokなどについても説明されていて、必読です。

*1:あーもちろん、翻訳やアウトリーチ・プロモーションやその他の活動もやっぱり大事なんですよ。わかりやすいように極端な言い方をしてるだけで。

*2:タイトルの通り、東京エリアDebian勉強会の皆さんには大変お世話になりました。またコラボしましょう！

*3:LibreOfficeはgitで管理されており、誰でも1行からコミット可能なので、例えばApache OpenOfficeのように特定のコミッターという権限を持つ人がいるわけではありません。コードが取り込まれれば誰でもコミッターです。

*4:BernのHackNightで挑戦したのがうまく行かなかったのはこれが原因。

*5:あ、間違ってるってのは知ってるので、ちょっと待ってね。

*6:結局は、後述のStephenさんがやってくれました。

*7:LibreOffice日本語チームのメンバーで、アクティブにコミットされている安倍さん。