ci(pytorch): add optional PyTorch git origin override to wheel builds - #6782
Conversation
Add an optional `pytorch_gitrepo_origin` input to the multi-arch PyTorch wheel build workflows (Linux and Windows, kept in sync). When set, the stable checkout uses it as the `--gitrepo-origin` (for example, a fork URL for pre-merge testing); it defaults to https://github.com/ROCm/pytorch.git so existing behavior is unchanged.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
therock-pr-bot
Bot
added
the
Not ready to Review
|
🎉 All checks passed! This PR is ready for review. |
| pytorch_origin="${{ inputs.pytorch_gitrepo_origin }}" | ||
| if [ -z "${pytorch_origin}" ]; then | ||
| pytorch_origin="https://github.com/ROCm/pytorch.git" | ||
| fi | ||
| ./external-builds/pytorch/pytorch_torch_repo.py checkout \ | ||
| --gitrepo-origin https://github.com/ROCm/pytorch.git \ | ||
| --gitrepo-origin "${pytorch_origin}" \ |
There was a problem hiding this comment.
I have a slight preference for this style of providing workflow inputs and default values where you load the input into an environment variable and provide a default if the input is empty:
TheRock/.github/workflows/build_portable_linux_python_packages.yml
Lines 111 to 113 in 9289636
For the approach I linked:
- multiple steps throughout the workflow can use the value like
${{ env.ARTIFACT_RUN_ID }} - The default close to where the input is
For the approach currently on this PR:
- Multiple uses would need to duplicate the default logic (possibly diverging)
- Inline bash should be avoided, see https://github.com/ROCm/TheRock/blob/main/docs/development/style_guides/github_actions_style_guide.md#prefer-python-scripts-over-inline-bash (we could clarify/extend that to also cover yaml patterns that are preferred over bash too)
| - ccache | ||
| default: sccache | ||
|
|
||
| run-name: Build Multi-Arch Windows PyTorch Wheels (${{ inputs.release_type }}, ${{ inputs.python_version }}, ${{ inputs.pytorch_git_ref }}) |
There was a problem hiding this comment.
We could also include the gitrepo_origin in the run-name if one is provided, but I think just the ref should be sufficient. What do you think? Would you want it included in the run name? We might be short on character count since repository URLs are pretty long...
If we did make that sort of change, we'd want a multiline string so optional values can be checked, like this:
TheRock/.github/workflows/multi_arch_release.yml
Lines 104 to 107 in 9289636
therock-pr-bot
Bot
removed
the
Not ready to Review
|
All therock-pr-bot checks green (approved). Expanded PR description with summary/changes/testing. Ready for maintainer merge. |
2 participants
ISSUE ID : #6735
Summary
Add an optional
pytorch_gitrepo_originworkflow input to the portable Linux and Windows PyTorch wheel build workflows so CI can checkout PyTorch from a fork URL (for pre-merge validation) while defaulting toROCm/pytorchwhen unset.Changes
multi_arch_build_portable_linux_pytorch_wheels.yml: newpytorch_gitrepo_origininput; pass through topytorch_torch_repo.py checkout --gitrepo-origin.multi_arch_build_windows_pytorch_wheels.yml: same input and wiring for the Windows wheel pipeline.Testing
pytorch_gitrepo_originis empty (still useshttps://github.com/ROCm/pytorch.git).