Skip to content

Adding msgpack and zstandard to requirements-test.txt#5111

Open
ajanicijamd wants to merge 2 commits intomainfrom
users/ajanicijamd/update-requirements-test
Open

Adding msgpack and zstandard to requirements-test.txt#5111
ajanicijamd wants to merge 2 commits intomainfrom
users/ajanicijamd/update-requirements-test

Conversation

@ajanicijamd
Copy link
Copy Markdown
Contributor

Motivation

Adding missing pip modules.

Technical Details

The cmake --build /therock/output/build step fails because modules msgpack and zstandard are not installed.

Test Plan

  • Go through the steps of building TheRock.
  • Before executing cmake --build /therock/output/build, run pip install -r /therock/src/requirements-test.txt.
  • cmake --build /therock/output/build must succeed.

Submission Checklist

Copy link
Copy Markdown
Member

@ScottTodd ScottTodd left a comment

Choose a reason for hiding this comment

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

This shouldn't be needed, unless you can show a test that needs these requirements, along with a specific error message.

Please also title your PRs, instead of using the branch name "Users/ajanicijamd/update requirements test" that github defaults to.

Comment thread requirements-test.txt
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The requirements file for building is https://github.com/ROCm/TheRock/blob/main/requirements.txt, and that already includes these packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but outside the container. After the step
./build_tools/linux_portable_build.py --interactive
the command is in the container, where the configuration and build are done. And the packages are not there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here's the failure for msgpack:

FAILED: artifacts/rccl_run_generic/artifact_manifest.txt /therock/output/build/artifacts/rccl_run_generic/artifact_manifest.txt
cd /therock/output/build/comm-libs && /usr/local/therock-tools/bin/cmake -E env PYTHONPATH=/therock/src/rocm-systems/shared/kpack/python /opt/python/cp312-cp312/bin/python3.12 /therock/src/rocm-systems/shared/kpack/python/rocm_kpack/tools/split_artifacts.py --artifact-dir /therock/output/build/artifacts-unsplit/rccl_run_generic --output-dir /therock/output/build/artifacts/ --artifact-prefix rccl_run --clang-offload-bundler /therock/output/build/compiler/amd-llvm/dist/lib/llvm/bin/clang-offload-bundler --gpu-targets gfx1100 gfx1101 gfx1102 gfx1103
Traceback (most recent call last):
  File "/therock/src/rocm-systems/shared/kpack/python/rocm_kpack/tools/split_artifacts.py", line 22, in <module>
    from rocm_kpack.artifact_splitter import ArtifactSplitter
  File "/therock/src/rocm-systems/shared/kpack/python/rocm_kpack/artifact_splitter.py", line 18, in <module>
    from rocm_kpack.artifact_utils import (
  File "/therock/src/rocm-systems/shared/kpack/python/rocm_kpack/artifact_utils.py", line 12, in <module>
    from rocm_kpack.binutils import Toolchain
  File "/therock/src/rocm-systems/shared/kpack/python/rocm_kpack/binutils.py", line 9, in <module>
    import msgpack
ModuleNotFoundError: No module named 'msgpack'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

linux_portable_build.py is not in active use (on public github here) outside of documentation. It could be deleted.

It should already be installing requirements.txt though, not requirements-test.txt:

pip install -r /therock/src/requirements.txt

@ajanicijamd ajanicijamd changed the title Users/ajanicijamd/update requirements test Adding msgpack and zstandard to requirements-test.txt May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants