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

fix(build): corepackのバグの回避 #15387

Merged
merged 10 commits into from
Feb 3, 2025

Conversation

kakkokari-gtyih
Copy link
Contributor

What

Why

Fix #15386

Additional info (optional)

これでいけるかどうかは分からない

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@kakkokari-gtyih kakkokari-gtyih added this to the v2025.2.0 milestone Feb 3, 2025
@kakkokari-gtyih kakkokari-gtyih changed the title fix(docker): pnpmのバージョンをpackageManagerに強制する fix(docker): pnpmのバージョンをpackage.jsonのpackageManagerに強制する Feb 3, 2025
@kakkokari-gtyih kakkokari-gtyih marked this pull request as draft February 3, 2025 02:42
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.38%. Comparing base (9230ee5) to head (e61dd49).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #15387      +/-   ##
===========================================
+ Coverage    38.68%   40.38%   +1.69%     
===========================================
  Files         1603     1607       +4     
  Lines       204302   210127    +5825     
  Branches      4042     4084      +42     
===========================================
+ Hits         79040    84856    +5816     
- Misses      124622   124633      +11     
+ Partials       640      638       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@anatawa12
Copy link
Member

storybookはpull_request_targetなので小直してもエラーになりますので無視しましょう

@kakkokari-gtyih
Copy link
Contributor Author

Test (federation) / Federation test が落ちてるのが謎

Comment on lines 52 to 53
cache: 'pnpm'
- run: corepack enable
- run: pnpm i --frozen-lockfile
Copy link
Member

Choose a reason for hiding this comment

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

corepack enableを戻したいです

Copy link
Contributor

@Mar0xy Mar0xy Feb 3, 2025

Choose a reason for hiding this comment

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

corepack enable is obsolete here cause Install PNPM does the same thing (https://github.com/pnpm/action-setup) which is a few lines above. so either install pnpm step should be removed or corepack enable

Copy link
Member

Choose a reason for hiding this comment

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

we'ere currently generally using corepack so I prefer corepack enable and removing duplicate is out of scope of this PR.

Copy link
Contributor

@Mar0xy Mar0xy Feb 3, 2025

Choose a reason for hiding this comment

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

I think this should also be done in a seperate PR cause I noticed you tried to already make pnpm use the version specified in package.json with ed74f7b

but it never worked on some of the workflow files cause corepack enable later on overwrote it with the latest version upon running.

Copy link
Member

Choose a reason for hiding this comment

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

but it never worked on some of the workflow files cause corepack enable later on overwrote it with the latest version upon running.

corepack won't overwrite with latest version.

Corepack checks for latest version of package manager as a part of startup (of no latest version cache are not exists), to be used as default version when version is not specified.

Regardless which version is latest version recognized by corepack, corepack will use version defined in packageManager field.

@anatawa12
Copy link
Member

federationテストはそっちでdockerの設定があるのでそれを治す必要がありそう。

misskey:
image: node:20
env_file:
- ./.config/docker.env
environment:
- NODE_ENV=production
volumes:
- type: bind
source: ../../../built
target: /misskey/built
read_only: true
- type: bind
source: ../assets
target: /misskey/packages/backend/assets
read_only: true
- type: bind
source: ../built
target: /misskey/packages/backend/built
read_only: true
- type: bind
source: ../migration
target: /misskey/packages/backend/migration
read_only: true
- type: bind
source: ../ormconfig.js
target: /misskey/packages/backend/ormconfig.js
read_only: true
- type: bind
source: ../package.json
target: /misskey/packages/backend/package.json
read_only: true
- type: bind
source: ../../misskey-js/built
target: /misskey/packages/misskey-js/built
read_only: true
- type: bind
source: ../../misskey-js/package.json
target: /misskey/packages/misskey-js/package.json
read_only: true
- type: bind
source: ../../misskey-reversi/built
target: /misskey/packages/misskey-reversi/built
read_only: true
- type: bind
source: ../../misskey-reversi/package.json
target: /misskey/packages/misskey-reversi/package.json
read_only: true
- type: bind
source: ../../../healthcheck.sh
target: /misskey/healthcheck.sh
read_only: true
- type: bind
source: ../../../package.json
target: /misskey/package.json
read_only: true
- type: bind
source: ../../../pnpm-lock.yaml
target: /misskey/pnpm-lock.yaml
read_only: true
- type: bind
source: ../../../pnpm-workspace.yaml
target: /misskey/pnpm-workspace.yaml
read_only: true
- type: bind
source: ./certificates/rootCA.crt
target: /usr/local/share/ca-certificates/rootCA.crt
read_only: true
working_dir: /misskey
command: >
bash -c "
corepack enable && corepack prepare
pnpm -F backend migrate
pnpm -F backend start
"
healthcheck:
test: bash /misskey/healthcheck.sh
interval: 5s
retries: 20

@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/backend:test labels Feb 3, 2025
@kakkokari-gtyih kakkokari-gtyih marked this pull request as ready for review February 3, 2025 06:32
@anatawa12
Copy link
Member

細かい話だけど pnpm のバージョンの強制自体はすでに行えてるので、 corepack のバグを回避する的な書き方のほうが正しいかもしれない。

@kakkokari-gtyih
Copy link
Contributor Author

細かい話だけど pnpm のバージョンの強制自体はすでに行えてるので、 corepack のバグを回避する的な書き方のほうが正しいかもしれない。

(Off-topic) corepackが治ったらrevertする必要がある?それとも仕様的にそうだったけど今まで問題とならなかったということ?

@kakkokari-gtyih kakkokari-gtyih changed the title fix(docker): pnpmのバージョンをpackage.jsonのpackageManagerに強制する fix(build): corepackでインストールされるpnpmのバージョンを固定するように Feb 3, 2025
@anatawa12
Copy link
Member

revertする必要性はないかな。COREPACK_DEFAULT_TO_LATESTはpackageManagerがpackage.jsonに指定されている限りは(本来)関係ない環境変数なので。(corepackの内部実装の都合でバグのworkaroundにできてるけど別にそれが仕様とは言いづらいと思う)

更に細かい話だけど最新(というかpackageManagerで指定されているバージョンとは別のバージョン)がインストールされることはなくて、最新版がどのバージョンかだけをcorepackが取得してて、その途中に落ちるという問題なので、"インストールされるバージョンを固定する"も部分的に誤りかもしれない。

@kakkokari-gtyih kakkokari-gtyih changed the title fix(build): corepackでインストールされるpnpmのバージョンを固定するように fix(build): corepackのバグの回避 Feb 3, 2025
anatawa12 added a commit to niri-la/misskey.niri.la that referenced this pull request Feb 3, 2025
…sskey-dev#15387) (#246)

* fix: disallow corepack from fetching latest manager version instead use specified version in package.json

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows

* test: set COREPACK_DEFAULT_TO_LATEST for federation tests

* docs(changelog): Fix: Docker のビルドに失敗する問題を修正

---------

Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: kakkokari-gtyih <67428053+kakkokari-gtyih@users.noreply.github.com>
@mi-gh-maintainer mi-gh-maintainer bot merged commit 9c70a4e into misskey-dev:develop Feb 3, 2025
32 of 33 checks passed
Copy link

Thank you 🙏

@kakkokari-gtyih kakkokari-gtyih deleted the fix-15386 branch February 3, 2025 23:34
@samunohito samunohito mentioned this pull request Feb 3, 2025
3-x-3 pushed a commit to yadokari-party/yadokari-misskey that referenced this pull request Feb 4, 2025
* fix: disallow corepack from fetching latest manager version instead use specified version in package.json

* Update Changelog

* fix?

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows

* Revert "apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows"

This reverts commit 67f0dc3.

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows (re)

* fix

* fix?

* revert: removing corepack enable

* test: set COREPACK_DEFAULT_TO_LATEST for federation tests

---------

Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: anatawa12 <anatawa12@icloud.com>
ragujp pushed a commit to ragujp/misskey that referenced this pull request Feb 4, 2025
* fix: disallow corepack from fetching latest manager version instead use specified version in package.json

* Update Changelog

* fix?

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows

* Revert "apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows"

This reverts commit 67f0dc3.

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows (re)

* fix

* fix?

* revert: removing corepack enable

* test: set COREPACK_DEFAULT_TO_LATEST for federation tests

---------

Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: anatawa12 <anatawa12@icloud.com>
kozakura913 added a commit to kozakura913/yojo-art that referenced this pull request Feb 5, 2025
Cherry-picked from misskey-dev/misskey#15387

# Conflicts:
#	.github/workflows/api-cherrypick-js.yml
#	.github/workflows/test-backend.yml
#	.github/workflows/test-cherrypick-js.yml
#	.github/workflows/test-frontend.yml
#	CHANGELOG.md

Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com>
Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: anatawa12 <anatawa12@icloud.com>
kozakura913 added a commit to kozakura913/yojo-art that referenced this pull request Feb 5, 2025
Cherry-picked from misskey-dev/misskey#15387

# Conflicts:
#	.github/workflows/api-cherrypick-js.yml
#	.github/workflows/test-backend.yml
#	.github/workflows/test-cherrypick-js.yml
#	.github/workflows/test-frontend.yml
#	CHANGELOG.md

Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com>
Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: anatawa12 <anatawa12@icloud.com>
DA-TENSHI pushed a commit to SHINANOSKEY-Projekt/SHINANOSKEY that referenced this pull request Feb 5, 2025
* fix: disallow corepack from fetching latest manager version instead use specified version in package.json

* Update Changelog

* fix?

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows

* Revert "apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows"

This reverts commit 67f0dc3.

* apply COREPACK_DEFAULT_TO_LATEST: 0 to every github workflows (re)

* fix

* fix?

* revert: removing corepack enable

* test: set COREPACK_DEFAULT_TO_LATEST for federation tests

---------

Co-authored-by: Marie <github@yuugi.dev>
Co-authored-by: anatawa12 <anatawa12@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend:test packages/backend Server side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

Signatures changed in the latest version of pnpm
3 participants