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

コンパイルチェックで利用するPythonバージョンを変更する #1766

Merged
2 commits merged into from Jan 17, 2022
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jan 14, 2022

PR の目的

スクリプトのコンパイルチェックで利用するPythonバージョンを変更します

カテゴリ

  • ビルド関連
    • Azure Pipelines

PR の背景

#1752 による対応の第1弾です)

コンパイルチェックを行うジョブは現在Windows Server 2016環境で実行されていますが、当該環境は3月中に利用できなくなるため、Windows Server 2022環境へ移行する予定です。

移行にあたり、2022では2.7が含まれませんので、利用するバージョンを変更します。

PR のメリット

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

リポジトリにあるPythonスクリプトが2.7と互換性があることを検証できなくなります。

仕様・動作説明

  • すでにサポートが終了しているPython 2.7の利用を廃止します。
  • 環境によって既定のPythonバージョンが異なることから、今後はバージョンを固定するのをやめ、実行時点で利用可能な最新のバージョンを選択するようにします。
    • なお、UsePythonVersionタスクの既定動作はこれです。
    • 現時点での最新は3.10系統です。

なお、各環境で利用できるバージョンは次の通りです。

環境 既定バージョン 他に利用できるバージョン
Windows Server 2016
(20220110.1)
3.7.* 2.7.*(1), 3.5.*(1, 2), 3.6.*(1), 3.8.*, 3.9.*, 3.10.*
Windows Server 2019
(20220110.1)
3.7.* 2.7.*(1), 3.5.*(1, 2), 3.6.*(1), 3.8.*, 3.9.*, 3.10.*
Windows Server 2022
(20211219.1)
3.9.* 3.7.*, 3.8.*, 3.10.*

上表注記

  1. Python 3.7より前のバージョンはサポートが終了しました。
    (参考: https://devguide.python.org/devcycle/#end-of-life-branches
  2. [All OSs] Python version 3.5 will be removed from the images on January 24, 2022

PR の影響範囲

Azure Pipelinesの次のジョブ

  • script_check

テスト内容

script_checkジョブで、

  • Python 2.7が選択されていないこと
  • 利用可能な最新のPython(現在は3.10)が選択されること
  • リポジトリにあるすべてのスクリプトが正常にコンパイルできること

関連 issue, PR

参考資料

Use Python Version task - Azure Pipelines | Microsoft Docs

@ghost ghost added the azure pipelines label Jan 14, 2022
berryzplus
berryzplus previously approved these changes Jan 15, 2022
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.

問題ないように思います。

このPRをマージすることによって、2系準拠で記述したPythonコードは、たとえその環境で問題なく動いていても「NG」っていう空気になります。

僕自身はもともと「新設するなら最新前提で書こうぜ!」派なのでなんも変わらない、と言えなくもないですが。

@@ -22,10 +22,8 @@ jobs:
strategy:
maxParallel: 2
Copy link
Contributor

Choose a reason for hiding this comment

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

ここの記述は、ビルド対象が2つ以上あったから指定していたものです。
ビルド対象が1つになると指定する意味はなくなるんではないかと思いました。
(とくに対処不要と思いますが、ややキモいっすw

Copy link
Author

Choose a reason for hiding this comment

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

matrixで指定する今までのやり方をそのまま活用したので、将来対象が増える可能性が残ったというのもあります。
変えるとしたら「1」に変えるのが良いかな(0または無指定の時は「無制限」になります)。

Copy link
Contributor

Choose a reason for hiding this comment

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

削除で良いんじゃないかな?と思いました。

元の設定も、対象2件に対して2つまで許可ってのは「無制限」と変わらん気がします。
「対象4件に対して2つまで」って制約ならば定義する意味があると思います。

まぁ、ここを修正する必然はないと思いますけど、なんかキモいですよね・・・。

Copy link
Author

Choose a reason for hiding this comment

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

確かにそうですね。
頻繁に変更するファイルでもないですし、今のうちに消しておくことにします。

Copy link
Author

Choose a reason for hiding this comment

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

消しました。

python_3.7:
versionSpec: '3.7'
python3:
versionSpec: '3.x'
Copy link
Contributor

Choose a reason for hiding this comment

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

2系の利用は従来から「非推奨」でしたが、
3.7も「そんな感じ」になったんですね。
3系のワイルドカード指定に変えるのは問題ないと思います。

当該ジョブの実行対象となるmatrixの数と同数以下であるため。
@ghost
Copy link
Author

ghost commented Jan 17, 2022

レビューありがとうございます。マージします。
もし何か不都合ありましたらご連絡ください。

@ghost ghost merged commit 68c4dc8 into sakura-editor:master Jan 17, 2022
@ghost ghost deleted the feature/change_azp_target_python_version branch January 17, 2022 04:15
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant