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

MYDEFINESに-DNDEBUGが指定されてない対策の改良 #1390

Conversation

berryzplus
Copy link
Contributor

PR の目的

言語DLLのMakefileのビルドでMYDEFINESを指定しなかった場合に-DNDEBUGを追加するための条件式を分かりやすくします。
同時に、本体ビルドのMakefileに同様の記述を挿入して記述を揃えます。

カテゴリ

  • ビルド関連
    • MinGWビルド

PR の背景

よく考えたら、-D_DEBUG-DNDEBUG も指定されていなければ -DNDEBUG を指定するということは、こういう書き方の方が分かりやすいのではと思ったり。

ifeq (,$(findstring -D_DEBUG,$(MYDEFINES)))
ifeq (,$(findstring -DNDEBUG,$(MYDEFINES)))
MYDEFINES += -DNDEBUG
endif
endif

んではそれで修正しときます。別PRになるので本体側も合わせて変更したいと思います。

Originally posted by @berryzplus in #1389 (comment)

PR のメリット

  • 言語DLLのMakefileに関して、MYDEFINESに -D_DEBUG も -DNDEBUG も指定されていない場合の条件式が読みやすくなります。また、何気に未指定の場合に -DNDEBUG が定義されない問題があったのを修正します。
  • 本体ビルドのMakefileに関して、言語DLLと記述が揃います。

PR のデメリット (トレードオフとかあれば)

たぶん、ないと思います。

仕様・動作説明

テスト内容

MYDEFINESの指定を変えてMinGWビルドして、rcとcppのビルドコマンドラインに -D_DEBUG-DNDEBUG のいずれかが指定されていることを確認しました。また、生成された実行可能ファイルを visual studio で開き、埋め込まれたアイコンリソースに問題ないことを確認しました。

  • mingw32-make -C sakura_core MYDEFINES= -j8
    • MYDEFINES 指定なし、Release版になるのでサクラアイコン(ID=210)がピンク。
  • mingw32-make -C sakura_core MYDEFINES=-D_DEBUG -j8
    • MYDEFINES=-D_DEBUG Debug版になるのでサクラアイコン(ID=210)が青。
  • mingw32-make -C sakura_core MYDEFINES=-DNDEBUG -j8
    • MYDEFINES=-DNDEBUG Release版になるのでサクラアイコン(ID=210)がピンク。
  • mingw32-make -C sakura_lang_en_US MYDEFINES= -j8
    • MYDEFINES 指定なし、Release版になるのでサクラアイコン(ID=210)がピンク。
  • mingw32-make -C sakura_lang_en_US MYDEFINES=-D_DEBUG -j8
    • MYDEFINES=-D_DEBUG Debug版になるのでサクラアイコン(ID=210)が青。
  • mingw32-make -C sakura_lang_en_US MYDEFINES=-DNDEBUG -j8
    • MYDEFINES=-DNDEBUG Release版になるのでサクラアイコン(ID=210)がピンク。

PR の影響範囲

MinGWビルド(本体ビルドと言語DLLビルド)

関連 issue, PR

#1389

参考資料

@AppVeyorBot
Copy link

@k-takata k-takata added environment 環境関連全般【ChangeLog除外】 MinGW MinGW labels Sep 5, 2020
@k-takata
Copy link
Member

k-takata commented Sep 5, 2020

また、何気に未指定の場合に -DNDEBUG が定義されない問題があったのを修正します。

解説をお願いします。
MYDEFINES をチェックするのではなく DEFINES をチェックするように変えたのと関係がありますか?

@berryzplus
Copy link
Contributor Author

また、何気に未指定の場合に -DNDEBUG が定義されない問題があったのを修正します。

解説をお願いします。
MYDEFINES をチェックするのではなく DEFINES をチェックするように変えたのと関係がありますか?

Makefileはあまり詳しくないので挙動からの推測になりますが、MYDEFINESが空定義の場合のMYDEFINES+=-DNDEBUGが機能していないようです。

DEFINESには必ず何かしら定義されているのでDEFINES+=-DNDEBUGならば定義追加が機能するようです。

チェック対象は MYDEFINES のままでも構わないんですが、追加対象を変えるから変えたほうが分かりやすいかと思ってチェック対象も変更しています。

@berryzplus
Copy link
Contributor Author

空定義に対する += が機能しないようなので、
左辺側が空定義にならないように別の変数を使うようにした、ということです。

@k-takata
Copy link
Member

k-takata commented Sep 5, 2020

なるほど。
コマンドラインから変数をセットすると、Makefiles 内での式は += であっても無視されます。
https://www.ecoop.net/coop/translated/GNUMake3.77/make_6.jp.html#SEC65
override を使えばコマンドラインの設定を上書きできるようです。

Copy link
Member

@k-takata k-takata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

@berryzplus
Copy link
Contributor Author

コマンドラインから変数をセットすると、Makefiles 内での式は += であっても無視されます。

う~む、つまり・・・

誤) 空定義だから += が機能しない
正) コマンドラインで指定してるから += が機能しない

@berryzplus berryzplus merged commit e34c4aa into sakura-editor:master Sep 5, 2020
@berryzplus berryzplus deleted the feature/add_ndebug_to_mydefines_if_missing branch September 5, 2020 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
environment 環境関連全般【ChangeLog除外】 MinGW MinGW
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants