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

GitHub Actions の環境変数に対応する #1664

Closed
wants to merge 4 commits into from
Closed

GitHub Actions の環境変数に対応する #1664

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 9, 2021

PR の目的

githash.bat を GitHub Actions に対応させます。

カテゴリ

  • 機能追加
    • GitHub Actions ビルド版
  • ビルド関連
    • GitHub Actions

PR の背景

#1630 でバッチファイルを確認していた際、 githash.bat が GitHub Actions に対応していないことに気が付きました。

経緯を調査したところ、 #1259 にて GitHub Actions を導入した際の残件であり、現在に至るまで対応が行われていません。
(以下、 #1259 の本文より引用)

未実装なところ

この PR で上記のうち「各種環境変数への対応」を行い、 GitHub Actions でのビルドにおいても必要な環境変数が設定されるようにします。

PR のメリット

  • GHA でビルドを行うときも必要な環境変数が定義されるようになります。
    • PR の成果物におけるバージョン情報ダイアログにて、
      • PR 番号が表示されるようになります。
      • PR ページへのリンクが設定されるようになります。
  • 成果物をローカルビルドと区別可能になります。

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

  • 特にありません。

仕様・動作説明

GitHub Actions の環境変数では、プルリクエストの番号とコミットハッシュが提供されません。
対策として、 PR によるワークフローの実行時に Webhook ペイロードオブジェクトからこれらの情報を取得し、次の環境変数にセットして代替とします。
なお、 PR 以外の Action 実行時は空文字列がセットされます。

オブジェクトのキー 説明 セットされる環境変数
github.event.pull_request.number PR 番号 GITHUB_PULL_REQUEST_NUMBER
github.event.pull_request.head.sha PR のコミットハッシュ GITHUB_PULL_REQUEST_HEAD_SHA

各ジョブの MSBuild ステップにて実行される githash.bat では、 GitHub Actions が提供する環境変数と上記の環境変数からビルドに必要な環境変数を生成し、その値を githash.h として出力します。

参照する CI 環境変数 説明 利用している環境変数
GITHUB_REPOSITORY リポジトリ名 CI_REPO_NAME
GIT_COMMIT_URL
GITHUB_PR_HEAD_URL
CI_BUILD_URL
GITHUB_ACTOR ワークフローの実行ユーザー名 CI_ACCOUNT_NAME
GITHUB_RUN_NUMBER ワークフローの一意な実行番号 CI_BUILD_NUMBER
GITHUB_RUN_ID ワークフローの一意な識別番号 CI_BUILD_VERSION
CI_BUILD_URL
GITHUB_ACTIONS ( Action の実行中は常にtrue GIT_COMMIT_URL
GITHUB_PR_HEAD_URL
CI_BUILD_URL
GITHUB_SERVER_URL GitHub サーバーのURL GIT_COMMIT_URL
GITHUB_PR_HEAD_URL
CI_BUILD_URL
GITHUB_PULL_REQUEST_NUMBER PR 番号 GITHUB_PR_NUMBER
GITHUB_PR_HEAD_URL
GITHUB_PULL_REQUEST_HEAD_SHA PR のコミットハッシュ GITHUB_PR_HEAD_COMMIT
GITHUB_PR_HEAD_SHORT_COMMIT
GITHUB_PR_HEAD_URL

また、プロジェクト名に相当する情報( APPVEYOR_PROJECT_SLUG に相当する情報)も提供されませんが、 CI_BUILD_URL の生成にはこの情報がなくても支障がないことから代替策は用意していません。

githash.bat が生成するヘッダファイルには依然として AppVeyor の環境変数が出力されるようになっていましたが、今回コンソールに出力される内容を調整する一環として廃止しています(今後本体側コードで使用する予定があればお申し出ください)。

なお、この PR では #1190 で言及された次の変更は行っていません。リクエストがあれば別途対応しますのでお申し出ください。

  • 環境変数の名称変更
    • GITHUB_PR_HEAD_SHORT_COMMIT → GITHUB_PR_SHORT_COMMIT_HASH
    • GITHUB_PR_HEAD_COMMIT → GITHUB_PR_COMMIT_HASH
    • GITHUB_PR_HEAD_URL → GITHUB_PR_URL
  • GITHUB_TAG_URL の追加
    定義:set GIT_TAG_URL=%PREFIX_GITHUB%/%CI_REPO_NAME%/tree/%GIT_TAG_NAME%

PR の影響範囲

( CI ビルドにおける) githash.h の生成プロセス

テスト内容

githash.bat のコンソール出力内容、生成されたヘッダファイル、成果物のファイル名を確認します。

関連 issue, PR

#1259 GitHub Actions を導入
#1271 残件対応①:ヘルプとインストーラのビルド
#1285 残件対応②:成果物を個別に Zip ファイル化

参考資料

@sonarcloud
Copy link

sonarcloud bot commented May 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@berryzplus
Copy link
Contributor

githash.bat を GitHub Actions に対応させます。

根本的なツッコミなんですが、githash.batをGitHub Actionsに対応させる必要は「ない」と思います。

githash.batの役割は、appveyorの環境変数を独自環境変数にマップし、
独自環境変数をdefineするヘッダーファイルを生成することです。

このため、GitHub Actions側で独自環境変数を定義するようにしてやれば、
githash.batにGitHub Actionsの環境変数と独自環境変数のマップを定義してやらなくても良いはずです。

@AppVeyorBot
Copy link

Build sakura 1.0.3749 completed (commit 24dfeea104 by @kazasaku)

@ghost
Copy link
Author

ghost commented May 9, 2021

@berryzplus says: #1664 (comment)
CI の設定ファイルでできることをあえてバッチファイルで行っている現在の構成を無視しても良いのであれば、この PR を閉じて build-sakura.yml にてすべて定義するようにしたものを出しますが、どうしましょうか?

この PR は「導入当時の残件」を、「環境変数は githash.bat で設定するようになっている」ことから githash.bat の変更で対応するものなので、後者の前提条件を外せるなら別の対応が取れます。
(なお、自分は Azure Pipelines の環境変数を扱っている以上、 githash.bat は AppVeyor 専用ではないと思っています。)

@berryzplus
Copy link
Contributor

@berryzplus says: #1664 (comment)
CI の設定ファイルでできることをあえてバッチファイルで行っている現在の構成を無視しても良いのであれば、この PR を閉じて build-sakura.yml にてすべて定義するようにしたものを出しますが、どうしましょうか?

この PR は「導入当時の残件」を、「環境変数は githash.bat で設定するようになっている」ことから githash.bat の変更で対応するものなので、後者の前提条件を外せるなら別の対応が取れます。
(なお、自分は Azure Pipelines の環境変数を扱っている以上、 githash.bat は AppVeyor 専用ではないと思っています。)

ぼくが言ってるのは「CIの設定でできることにわざわざバッチ書くのはアフォだよね?」です。
#1665 で書いている通り、Appveyorは他のCIに比べて機能が足りないです。
Appveyorに対応させるためには、バッチを書く必要がありました。
機能的に差があるのに、Appveyorに合わせる必要はないと思います。
Azure Pipelinesには固有の持ち味があるはずで、GitHub Actionsにも固有の持ち味があります。
無理やりAppveyorに合わせるってことは、CIごとの持ち味を殺す結果になります。

このまま進めても良いです。

自分はgithash.bat は原則「Appveyor の環境変数をヘッダーに取り込むバッチ」だと捉えています。

Azure Pipelinesの対応も含まれていますが、内容が不完全だと思います。
※ビルドバージョンで、azpビルド品を正規品(=appveyorビルド品)と区別できません。

GitHub Actionsでも同じ問題が発生すると思っています。
とりあえず、修正が現状のAzure Pipelines相当に達したと判断したらOK出してしまうと思いますが、
まだDraftだったので内容のレビューはしていません。

ただ、環境情報はソースではなく構成ファイルに記述するのがよいと思います。
(ので、そのうちそういうPRで上書きしようとする感じになると思います)

@ghost
Copy link
Author

ghost commented May 14, 2021

(Duplicate of #1669 ) ワークフロー設定ファイルの変更で対応します。

@ghost ghost marked this as a duplicate of #1669 May 14, 2021
@ghost ghost closed this May 14, 2021
@ghost ghost deleted the feature/enhance_githash_support_gha branch May 14, 2021 02:47
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants