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.

Sign up for GitHub

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

Jump to bottom

chore: Add missing peer dependency for nsfwjs #9206

Closed
wants to merge 1 commit into from
Closed

chore: Add missing peer dependency for nsfwjs #9206

wants to merge 1 commit into from

Conversation

grainrigi
Copy link

@grainrigi grainrigi commented Nov 20, 2022 •
edited
Loading

What

Add @tensorflow/tfjs, a peer dependency of nsfwjs, as a dependency of packages/backend.

Why

As described in #9202, optional support for tensorflow is incomplete and the server won't work without @tensorflow/tfjs-node.
In platforms which don't support libtensorflow, this would be a problem because @tensorflow/tfjs-node cannot be installed due to missing prebuilt binaries.

@grainrigi
chore: Add missing peer dependency for nsfwjs
bc57706 
Fixes misskey-dev#9202

Since '@tensorflow/tfjs-node' is moved into optionalDependencies in misskey-dev#8986,
'@tensorflow/tfjs', a peer dependency of 'nsfwjs',
won't be installed when optionalDeps are skipped.
This was causing missing module error during the server boot.

To fix this, we add '@tensorflow/tfjs' as a normal dependency.
This won't break non tensorflow capable platform
since this package does not require any prebuilt binaries to be installed.
@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Nov 20, 2022
@tamaina
Copy link
Contributor

tamaina commented Dec 2, 2022 •
edited
Loading

Is this change necessary in yarn v3 (develop branch)?

@grainrigi
Copy link
Author

grainrigi commented Dec 3, 2022 •
edited
Loading

Sorry, I misunderstood about optionalDependencies.
I didn't need to use --ignore-optional because installation error for optionalDependencies does not affect the exit code of yarn install itself.
Without using --ignore-optional, yarn install completes successfully, and the server worked without any problem!
(Why this works is that trying to install @tensorflow/tfjs-node causes the installation of all its dependency (including @tensorflow/tfjs), and installation failure of @tensorflow/tfjs-node does not remove the dependency installed at that time.)
It might also work well in yarn v3.

Finally, thanks for reviewing this!

@grainrigi grainrigi closed this Dec 3, 2022
@grainrigi
Copy link
Author

FYI, I tried to test with yarn v3, but yarn install ran out all virtual memory and could not confirm if it really works.
(yarn v3 uses really much virtual memory(about 256g on my x86_64 machine), which i386 cannot afford (max. 4GB virtual memory))

@tamaina
Copy link
Contributor

tamaina commented Dec 3, 2022

yarn install --immutable may not consume that much memory. (However, it consumes a lot of memory to change yarn.lock.)

@grainrigi
Copy link
Author

Unfortunately, --immutable didn't really help. (still causes OOM)
For now I'll stick to yarn v1 on my i386 server.
Thank you!

@grainrigi grainrigi mentioned this pull request Dec 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@grainrigi @tamaina