Project

General

Profile

Actions

バグ #1161

open

--confrootオプション付きで起動した時Gemfileを含むプラグインが存在すると落ちる

Added by cob odo over 6 years ago. Updated about 5 years ago.

Status:
レビュー待ち
Priority:
通常
Assignee:
Target version:
プラグイン名:
クラッシュする:
Yes

Description

再現手順の通り。


再現手順

  • Gemfileを含むプラグインが1つでも存在する
  • MIKUTTER_CONFROOT環境変数なし
  • 事前に env MIKUTTER_CONFROOT=/path/to/conf bundle install 済

……の状態で、

ruby mikutter.rb --confroot=/path/to/conf

で起動すると、プラグイン内部でgemパッケージをrequireする行で落ちます。


Related issues

Related to 機能 #1070: bundlerを使ってGemをインストールする時に、プラグインディレクトリを環境変数で設定する終了toshi_a 初音2017-10-08

Actions
Actions #1

Updated by toshi_a 初音 over 6 years ago

confrootオプションは、MIKUTTER_CONFROOTより前からあって、MIKUTTER_CONFROOT環境変数はconfrootオプションが指定されたときと同じ振る舞いをすべきですね( #1070 )。

Actions #2

Updated by toshi_a 初音 over 6 years ago

  • Related to 機能 #1070: bundlerを使ってGemをインストールする時に、プラグインディレクトリを環境変数で設定する added
Actions #3

Updated by cob odo about 6 years ago

  • Status changed from 新規 to 実装待ち
  • Target version changed from 3.6 to 3.8
  • クラッシュする changed from No to Yes
Actions #4

Updated by cob odo about 6 years ago

  • Status changed from 実装待ち to レビュー待ち
  • ブランチ set to topic/1161-gemfile-with-confroot-option

修正して topic/1161-gemfile-with-confroot-option ブランチにpushしました。どなたか確認をお願いします。

修正にあたっては以下のプラグインを作成して使用しました。修正前の状態ではこれでクラッシュさせることができます。

$MIKUTTER_CONFROOT/plugin/websocket/websocket.rb

require 'websocket'

$MIKUTTER_CONFROOT/plugin/websocket/Gemfile

# frozen_string_literal: true

source "https://rubygems.org" 

git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem 'websocket'

Actions #5

Updated by Izumi Tsutsui about 5 years ago

  • Assignee set to Izumi Tsutsui

OSC京都後のミーティングで この修正方法はどうなのだろう、という会話をとしぁさんとしたりしましたが、とりあえずこちらで再現確認してみます

Actions #6

Updated by Izumi Tsutsui about 5 years ago

cob odo さんは書きました:

修正にあたっては以下のプラグインを作成して使用しました。修正前の状態ではこれでクラッシュさせることができます。

とりあえず(?) Windows 10 Pro の WSL な ubuntu 18.04 LTS で修正前のクラッシュは再現しました。

tsutsui@optiplex:~/mikutter$ LANG=ja_JP.UTF-8 DISPLAY=:0 ruby mikutter.rb --confroot=/home/tsutsui/.mikutter-test
Gtk-WARNING **: Locale not supported by C library.
        Using the fallback 'C' locale.
        from /var/lib/gems/2.5.0/gems/gtk2-3.3.6/lib/gtk2.rb:13:in `<top (required)>'
        from /home/tsutsui/mikutter/core/plugin/gtk/gtk.rb:5:in `require'
        from /home/tsutsui/mikutter/core/plugin/gtk/gtk.rb:5:in `<top (required)>'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:158:in `load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:158:in `block in load'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `block in atomic'
        from /usr/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `atomic'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:146:in `load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:149:in `block (2 levels) in load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:147:in `each'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:147:in `block in load'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `block in atomic'
        from /usr/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `atomic'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:146:in `load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:149:in `block (2 levels) in load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:147:in `each'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:147:in `block in load'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `block in atomic'
        from /usr/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
        from /home/tsutsui/mikutter/core/utils.rb:299:in `atomic'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:146:in `load'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:98:in `block in load_all'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:38:in `block in each_spec'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:33:in `each'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:33:in `each'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:36:in `each_spec'
        from /home/tsutsui/mikutter/core/miquire_plugin.rb:96:in `load_all'
        from /home/tsutsui/mikutter/core/boot/load_plugin.rb:10:in `<top (required)>'
        from /home/tsutsui/mikutter/core/miquire.rb:98:in `require'
        from /home/tsutsui/mikutter/core/miquire.rb:98:in `miquire_original_require'
        from /home/tsutsui/mikutter/core/miquire.rb:95:in `file_or_directory_require'
        from /home/tsutsui/mikutter/core/miquire.rb:76:in `block in miquire'
        from /home/tsutsui/mikutter/core/miquire.rb:75:in `each'
        from /home/tsutsui/mikutter/core/miquire.rb:75:in `miquire'
        from /home/tsutsui/mikutter/core/miquire.rb:18:in `miquire'
        from mikutter.rb:43:in `<main>'
Traceback (most recent call last):
        22: from mikutter.rb:43:in `<main>'
        21: from /home/tsutsui/mikutter/core/miquire.rb:18:in `miquire'
        20: from /home/tsutsui/mikutter/core/miquire.rb:75:in `miquire'
        19: from /home/tsutsui/mikutter/core/miquire.rb:75:in `each'
        18: from /home/tsutsui/mikutter/core/miquire.rb:76:in `block in miquire'
        17: from /home/tsutsui/mikutter/core/miquire.rb:95:in `file_or_directory_require'
        16: from /home/tsutsui/mikutter/core/miquire.rb:98:in `miquire_original_require'
        15: from /home/tsutsui/mikutter/core/miquire.rb:98:in `require'
        14: from /home/tsutsui/mikutter/core/boot/load_plugin.rb:10:in `<top (required)>'
        13: from /home/tsutsui/mikutter/core/miquire_plugin.rb:96:in `load_all'
        12: from /home/tsutsui/mikutter/core/miquire_plugin.rb:36:in `each_spec'
        11: from /home/tsutsui/mikutter/core/miquire_plugin.rb:33:in `each'
        10: from /home/tsutsui/mikutter/core/miquire_plugin.rb:33:in `each'
         9: from /home/tsutsui/mikutter/core/miquire_plugin.rb:38:in `block in each_spec'
         8: from /home/tsutsui/mikutter/core/miquire_plugin.rb:98:in `block in load_all'
         7: from /home/tsutsui/mikutter/core/miquire_plugin.rb:146:in `load'
         6: from /home/tsutsui/mikutter/core/utils.rb:299:in `atomic'
         5: from /usr/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
         4: from /home/tsutsui/mikutter/core/utils.rb:299:in `block in atomic'
         3: from /home/tsutsui/mikutter/core/miquire_plugin.rb:158:in `block in load'
         2: from /home/tsutsui/mikutter/core/miquire_plugin.rb:158:in `load'
         1: from /home/tsutsui/.mikutter-test/plugin/websocket.rb:1:in `<top (required)>'
/home/tsutsui/.mikutter-test/plugin/websocket.rb:1:in `require': cannot load such file -- websocket (LoadError)
tsutsui@optiplex:~/mikutter$

WSL ubuntu 18.04 LTS メモ

WSL用の Xサーバーを別途設定して(詳細はググれば出てくたはずなので略)、

sudo apt install make gcc libx11-dev git ruby ruby-dev ruby-bundler libidn11-dev fonts-vlgothic

くらいを入れておけばとりあえず bundler で設定できて mikutter 起動までいけるはず?
(素の状態から検証しなおすのがめんどい)

Actions #7

Updated by Izumi Tsutsui about 5 years ago

OSC京都後のミーティングで この修正方法はどうなのだろう、という会話をとしぁさんとしたりしましたが

65a33333 の「コマンドライン引数を環境変数に設定する」という方法に根拠のない違和感がありました。

しかるべき規格に記載があるわけではないのですが、感覚的には
「コマンドラインで指定された設定が優先、コマンドライン指定がなかったら次に環境変数を見る」
というのが一般的という気がしているせいかなと思います。

つまり、コマンドラインと環境変数が異なっていたらコマンドライン設定が優先されるだけで
環境変数はそのままであるべきで、コマンドラインで指定された情報を bundler 関連に
引き渡す方法として環境変数経由ではない然るべき方法を用いるべきという気がする、
というのが違和感の正体かなと思っています。

が、コードをまったく読んでいないので、どういう構成なのかぼちぼち拾ってみます。

Actions #8

Updated by cob odo about 5 years ago

環境変数に手を加えずに解決するアプローチとしては以下のようなものが考えられます。Gemfileの方の手を入れて、bundle install時と --confroot 指定実行時で一貫性を保てばいいというわけです。

diff --git a/Gemfile b/Gemfile
index 17be4c90..ad0f5262 100644
--- a/Gemfile
+++ b/Gemfile
@@ -35,7 +35,7 @@ group :plugin do
   Dir.glob(File.expand_path(File.join(__dir__, 'core/plugin/*/Gemfile'))){ |path|
     eval File.open(path).read
   }
-  Dir.glob(File.join(File.expand_path(ENV['MIKUTTER_CONFROOT'] || '~/.mikutter'), 'plugin/*/Gemfile')){ |path|
+  Dir.glob(File.join(File.expand_path(Module.const_defined?('::Mopt') && ::Mopt.confroot || ENV['MIKUTTER_CONFROOT'] || '~/.mikutter'), 'plugin/*/Gemfile')){ |path|
     eval File.open(path).read
   }
 end

ここからはIMOです。
mikutterの子プロセスとしてbundleを実行するようなプラグインをもし作ろうとすると、この辺のことを意識して十全に対策する必要が出てきます。mikutter側で Mopt.confrootMIKUTTER_CONFROOT 環境変数を一貫した状態に変更しておくほうが、そういったサードパーティープラグインに対しては親切かなと個人的には感じます。
環境変数はあくまでも親プロセスから渡された環境情報で、そのアプリケーションのみが使用する限りにおいて変更することには何ら問題ないと思います。もちろん USERHOME のような、子プロセスが使用する蓋然性が高い変数を変更することには大きなリスクを伴いますが、 MIKUTTER_CONFROOT をそれらと同等に扱うべきとはあまり思えないです。

Actions

Also available in: Atom PDF