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

appveyor.yml の構成変更 #72

Merged

Conversation

m-tmatma
Copy link
Member

@m-tmatma m-tmatma commented Jun 8, 2018

概略

現状の appveyor.yml は一度のブートで Debug/Release の構成のビルドを行います。

修正の背景

しかしながら、#40 で試しに x64 対応したソリューションでは
x86 の構成をビルドした後に、x64 をビルドするとコンパイルが通りますが、
クリーン環境で x64 をビルドするとコンパイルエラーになります。

これは、x86 版では HeaderMake.exe がプロジェクトファイルと同じディレクトリに生成されて
それを前提に preBuild.bat が実行されるが、
x64 では $(SolutionDir)$(Platform)$(Configuration) に HeaderMake.exe が生成されるので
preBuild.bat を実行実行しても必要なヘッダファイルが生成されないためです。

修正の目的

別のビルドの生成物によってビルドに成功したり、失敗する問題を検出するために
各ビルドがクリーン環境でビルドされるように変更します。

修正のデメリット

各構成をビルドするたびに、ターゲット環境の再起動等が走るので
トータルのビルド時間が長くなります。

@berryzplus
Copy link
Contributor

headermake,makefilemakeをx64ビルドしないといけない理由ってなんかあるんでしたっけ?

この2つの依存プロジェクトはリンクするわけではないのでx64である必然がないと思っています。
visual studio側で設定したらいいです。

sakura.slnで「構成マネージャ」を開いたときの設定値 (x64 Release構成)
headermake Release Win32
makefilemake Release Win32
sakura Release x64
sakura_lang_en_US Release x64

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

x64 のビルドの必要はないです。

そこはポイントではないです。
この修正のきっかけですが、
同様の問題が将来起きた場合に検出できるようにすることが目的です。

@berryzplus
Copy link
Contributor

そこはポイントではないです。

おっしゃる意味はたぶん、理解できてる気がします。
似たような問題が起きないように手を打ちたいね、ってとこにも同意です。

違っているのは、何を修正すべきかの判断だと思います。
appveyor.ymlには別の動機で修正が入るんだと思います。
コンフィグとプラットフォームを変数で指定できるようにする修正ですよね?

特定のコンフィグ(プラットフォーム)で何をビルドすべきか定義するのはslnです。
なので sakura.sln でビルド対象を定義するのが良いと思います。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

コンフィグとプラットフォームを変数で指定できるようにする修正ですよね?

それは手段です。目的ではないです。

特定のコンフィグ(プラットフォーム)で何をビルドすべきか定義するのはslnです。
なので sakura.sln でビルド対象を定義するのが良いと思います。

x64 のソリューションでのビルドが通らない問題を修正したい場合、
HeaderMake.exe 等の Win32 版をビルドするようにsakura.sln を修正すればいいですが
(クロスコンパイルできるためにもそうするべきですが)

この PR はあるビルド構成の生成物があるために、別のビルド構成で
本来失敗するはずのものが成功してしまう問題を検出するために
各ビルド構成でクリーンな環境でビルドするための修正です。

x64 のビルドに関する話は、上記の問題が発生する場合の具体例であって
この PR は同様の問題が発生してもすぐに appveyor で検出できるように
するためのものです。

論点が違います。

kobake
kobake previously approved these changes Jun 9, 2018
Copy link
Member

@kobake kobake left a comment

Choose a reason for hiding this comment

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

.yml がすっきりしましたね。実際に走っているビルドログも問題なさそうに見えました。
https://ci.appveyor.com/project/sakuraeditor/sakura/build/1.0.100

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

@berryzplus さん
問題なければマージお願いします。

@berryzplus
Copy link
Contributor

この PR はあるビルド構成の生成物があるために、別のビルド構成で
本来失敗するはずのものが成功してしまう問題を検出するために
各ビルド構成でクリーンな環境でビルドするための修正です。

各ビルド構成ごとにクリーンな環境でビルドを行うのは、appveyorの元々の仕様である認識です。
あえて独自動作を定義しているのは他プロジェクトでの実績や経験からきたものと思っていました。
修正内容は、独自定義をやめてappveyorの標準動作に任せる変更である認識です。

標準動作に戻すなら、artifactsの定義にも標準が使える気がします。
artifactsで標準が使えるなら build_script も標準の指定方法に戻せる気がします。

認識が間違っていたら教えてください。

なお、内容に問題があるとは考えていないのでマージしてもいいと思います。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

あえて独自動作を定義しているのは他プロジェクトでの実績や経験からきたものと思っていました。

他のプロジェクトでは「各ビルド構成ごとにクリーンな環境でビルド」はせずに
自分でビルド用のバッチファイルを書いて対応してました。

標準動作に戻すなら、artifactsの定義にも標準が使える気がします。
artifactsで標準が使えるなら build_script も標準の指定方法に戻せる気がします。

独自定義しているのは #12 を将来的に対応可能なように
ビルド設定を書くようにしているためです。

「artifactsの定義にも標準」というのはワイルドカードを使う方法ですか?

@berryzplus
Copy link
Contributor

「artifactsの定義にも標準」というのはワイルドカードを使う方法ですか?

いや、マニュアルを見ると appveyor が定義する変数があるっぽいです。
ワイルドカードと同じところで解説されてます。

https://www.appveyor.com/docs/packaging-artifacts/

The artifact path must be relative to the root of repository.
For example, to upload the myproject.dll assembly from the bin folder of a project enter:

bin\debug\myproject.dll

You can use wildcards and environment variables in the artifact path.
Let’s say the “configuration” variable contains the current build configuration.
Then to upload all assemblies in the bin directory:

bin\$(configuration)\*.dll

記述を斜め読みしてこんな書き方もできるはず、と思いました。

artifacts: 
 - path: sakura\$(configuration)\sakura.exe  

であるなら…と1つ前に書いたコメントにつながります。

@berryzplus
Copy link
Contributor

だめでしたね。少なくとも build_script 内では無理そう。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

だめでしたね。少なくとも build_script 内では無理そう。

そうですね。

ビルド失敗のコミットを残すために revert のほうがいいですか?
それとも rebase してきれいにしたほうがいいですか?

@kobake
Copy link
Member

kobake commented Jun 9, 2018

rebase が良いです。

情報を残したいのであれば、以下のようにコメント書いておくくらいで良いかと。

失敗記録

https://ci.appveyor.com/project/sakuraeditor/sakura/build/1.0.102

appveyor.yml

version: 1.0.{build}
image: Visual Studio 2017
clone_folder: C:\projects\$(APPVEYOR_ACCOUNT_NAME)\$(APPVEYOR_PROJECT_NAME)_$(APPVEYOR_BUILD_VERSION)
init:
- ps: Set-WinSystemLocale ja-JP
- ps: Start-Sleep -s 5
- ps: Restart-Computer

configuration:
  - Release
  - Debug

platform:
  - x86

build_script:
- cmd: >-
    set SLN_FILE=sakura\sakura.sln
    echo PLATFORM      $(platform)
    echo CONFIGURATION $(configuration)
    set EXTRA_CMD=/verbosity:minimal /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
    set MSBUILD_EXE="C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe"
    echo %MSBUILD_EXE% %SLN_FILE% /p:Platform=$(platform) /p:Configuration=$(configuration)      /t:"Clean","Rebuild"  %EXTRA_CMD%
         %MSBUILD_EXE% %SLN_FILE% /p:Platform=$(platform) /p:Configuration=$(configuration)      /t:"Clean","Rebuild"  %EXTRA_CMD%
    echo appveyor_yml
artifacts:
- path: sakura\$(configuration)\sakura.exe
- path: sakura\$(configuration)\sakura.pdb
- path: sakura_lang_en_US\$(configuration)\sakura_lang_en_US.dll

ビルドログ

Build started
Set-WinSystemLocale ja-JP
Start-Sleep -s 5
git clone -q https://github.com/sakura-editor/sakura.git C:\projects\sakuraeditor\sakura_1.0.102
git fetch -q origin +refs/pull/72/merge:
git checkout -qf FETCH_HEAD
set SLN_FILE=sakura\sakura.sln
echo PLATFORM      $(platform)
PLATFORM      $(platform)
echo CONFIGURATION $(configuration)
CONFIGURATION $(configuration)
set EXTRA_CMD=/verbosity:minimal /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
set MSBUILD_EXE="C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe"
echo %MSBUILD_EXE% %SLN_FILE% /p:Platform=$(platform) /p:Configuration=$(configuration)      /t:"Clean","Rebuild"  %EXTRA_CMD%
"C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe" sakura\sakura.sln /p:Platform=$(platform) /p:Configuration=$(configuration)      /t:"Clean","Rebuild"  /verbosity:minimal /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
     %MSBUILD_EXE% %SLN_FILE% /p:Platform=$(platform) /p:Configuration=$(configuration)      /t:"Clean","Rebuild"  %EXTRA_CMD%
Microsoft (R) Build Engine version 15.7.179.6572 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
C:\projects\sakuraeditor\sakura_1.0.102\sakura\sakura.sln.metaproj : error MSB4126: The specified solution configuration "$(configuration)|$(platform)" is invalid. Please specify a valid solution configuration using the Configuration and Platform properties (e.g. MSBuild.exe Solution.sln /p:Configuration=Debug /p:Platform="Any CPU") or leave those properties blank to use the default solution configuration. [C:\projects\sakuraeditor\sakura_1.0.102\sakura\sakura.sln]
Command exited with code 1

@berryzplus
Copy link
Contributor

berryzplus commented Jun 9, 2018

自分とこでやってみた感じ、artifacts でなら使えそうでした。

https://ci.appveyor.com/project/berryzplus/sakura/build/1.0.35
https://github.com/berryzplus/sakura/blob/c28baed81de6c2ce7a32d024fcb94b009c397415/appveyor.yml#L15-L22

※sakura_lang_en_US.dllが取れてないのは出力先を変えてあるためです。

@m-tmatma m-tmatma force-pushed the feature/refactor-appveyor-yml branch from fb19156 to f7e8144 Compare June 9, 2018 21:56
@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

fb19156 を rebase で消しました。

C:\gitwork>git clone https://github.com/m-tmatma/sakura.git
Cloning into 'sakura'...
remote: Counting objects: 41957, done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 41957 (delta 0), reused 1 (delta 0), pack-reused 41954
Receiving objects: 100% (41957/41957), 15.91 MiB | 3.22 MiB/s, done.
Resolving deltas: 100% (35800/35800), done.
Checking out files: 100% (1615/1615), done.

C:\gitwork>cd sakura

C:\gitwork\sakura>git config --global core.editor sakura.exe

C:\gitwork\sakura>set PATH=%PATH%;C:\Program Files (x86)\sakura

C:\gitwork\sakura>git branch -r
  origin/HEAD -> origin/master
  origin/ansi
  origin/feature/WIP-cmake-experiement
  origin/feature/WIP-refactor-appveyor_yml
  origin/feature/WIP-x64-trial
  origin/feature/appveyor-test-utf8
  origin/feature/appveyor-test1
  origin/feature/refactor-appveyor-yml
  origin/feauture/easy-PR-review
  origin/master

C:\gitwork\sakura>git checkout -b feature/refactor-appveyor-yml  origin/feature/refactor-appveyor-yml
Switched to a new branch 'feature/refactor-appveyor-yml'
Branch 'feature/refactor-appveyor-yml' set up to track remote branch 'feature/refactor-appveyor-yml' from 'origin'.

C:\gitwork\sakura>
C:\gitwork\sakura>git log --oneline > log1.txt

log1.txt

fb19156 を消すので f7e8144 に戻したい。

fb191567 変数の参照方法を変更
f7e81444 configuration と platform を指定してクリーン環境でビルドする

git rebase -i f7e8144 を実行すると以下の内容で sakura が起動する

pick fb19156 変数の参照方法を変更

pick fb191567 変数の参照方法を変更

# Rebase f7e81444..fb191567 onto f7e81444 (1 command)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out

以下のように、pick を drop に書き換えて保存して全終了する

drop fb191567 変数の参照方法を変更

# Rebase f7e81444..fb191567 onto f7e81444 (1 command)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out

以下のように成功する

C:\gitwork\sakura>git rebase -i f7e81444
Successfully rebased and updated refs/heads/feature/refactor-appveyor-yml.

C:\gitwork\sakura>git log --oneline > log2.txt
log2.txt

C:\gitwork\sakura>git status
On branch feature/refactor-appveyor-yml
Your branch is behind 'origin/feature/refactor-appveyor-yml' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        log1.txt
        log2.txt

nothing added to commit but untracked files present (use "git add" to track)

C:\gitwork\sakura>git remote -v
origin  https://github.com/m-tmatma/sakura.git (fetch)
origin  https://github.com/m-tmatma/sakura.git (push)

C:\gitwork\sakura>git branch
* feature/refactor-appveyor-yml
  master

強制 push する

C:\gitwork\sakura>git push -f origin feature/refactor-appveyor-yml
Total 0 (delta 0), reused 0 (delta 0)
To https://github.com/m-tmatma/sakura.git
 + fb191567...f7e81444 feature/refactor-appveyor-yml -> feature/refactor-appveyor-yml (forced update)

@m-tmatma
Copy link
Member Author

m-tmatma commented Jun 9, 2018

自分とこでやってみた感じ、artifacts でなら使えそうでした。

一度 rebase で最新のコミットを消した後、54f4778 で artifacts のみ変更しました。

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

確認しました。

ログも問題ないように思います。
https://ci.appveyor.com/project/sakuraeditor/sakura/build/1.0.105

@berryzplus berryzplus merged commit e9741a7 into sakura-editor:master Jun 10, 2018
@m-tmatma m-tmatma added this to the next release milestone Jun 10, 2018
@m-tmatma m-tmatma deleted the feature/refactor-appveyor-yml branch June 10, 2018 03:50
@m-tmatma m-tmatma added the CI appveyor など CI 関連 【ChangeLog除外】 label Jun 12, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI appveyor など CI 関連 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants