-
Notifications
You must be signed in to change notification settings - Fork 245
GHA workflow PoC to configure submodule remote, branch #594
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
Changes from all commits
5bc74b9
5d1bc89
2deb92e
8977331
02f5fa2
439fe21
76b7471
afb9a31
c89f985
4871fdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,16 @@ on: | |
| default: ADHOCBUILD | ||
| amdgpu_families: | ||
| type: string | ||
| description: "(etc. gfx110X-dgpu)" | ||
| project_name: | ||
| type: string | ||
| description: "name of subproject (etc. hipBLASLt) to configure remote url and branch to fetch. (disable w/ null name and url)" | ||
| project_seturl: | ||
| type: string | ||
| description: "url of subproject (etc. https://github.com/ROCm/hipBLASLt) (disable w/ null name and url)" | ||
| project_branch: | ||
| type: string | ||
| description: "name of branch (etc. develop) to configure remote url and branch to fetch" | ||
|
Comment on lines
+12
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's try to keep each workflow focused on one specific task. If there was one primary way to interface with the project then having a monolithic workflow could make sense, but we have a complex development model here with several ways to build/test/use artifacts. This new code could go in a workflow that composes with the existing workflows, to have:
With the "create a branch" approach mentioned in my other comment, we could have
|
||
|
|
||
| workflow_call: | ||
| inputs: | ||
|
|
@@ -16,6 +26,16 @@ on: | |
| default: ADHOCBUILD | ||
| amdgpu_families: | ||
| type: string | ||
| description: "(etc. gfx110X-dgpu)" | ||
| project_name: | ||
| type: string | ||
| description: "name of subproject (etc. hipBLASLt) to configure remote url and branch to fetch. (disable w/ null name and url)" | ||
| project_seturl: | ||
| type: string | ||
| description: "url of subproject (etc. https://github.com/ROCm/hipBLASLt) (disable w/ null name and url)" | ||
| project_branch: | ||
| type: string | ||
| description: "name of branch (etc. develop) to configure remote url and branch to fetch" | ||
|
|
||
| jobs: | ||
| build_linux_packages: | ||
|
|
@@ -59,8 +79,65 @@ jobs: | |
| restore-keys: | | ||
| linux-build-packages-manylinux-v2- | ||
|
|
||
| - name: Fetch submodule from remote | ||
| run: | | ||
| proj_name="${{ inputs.project_name }}" | ||
| proj_branch="${{ inputs.project_branch }}" | ||
| proj_seturl="${{ inputs.project_seturl }}" | ||
|
Comment on lines
+82
to
+86
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's move all this bash to a Python script so we can
|
||
|
|
||
| echo "[*] Fetch submodule from remote using GHA inputs:" | ||
| echo " proj_name: [$proj_name]" | ||
| echo " proj_path: [$proj_submodule_path]" | ||
| echo " proj_seturl: [$proj_seturl]" | ||
| echo " proj_branch: [$proj_branch]" | ||
|
|
||
| #Only update submodule if set in github action inputs | ||
| if [ "$proj_name" ] || [ "$proj_seturl" ]; then | ||
| if [ ! "$proj_name" ]; then | ||
| proj_name=$(basename "$proj_seturl") | ||
| echo "[*] proj_name GHA Input not set, deriving from proj_seturl: [$proj_name]" | ||
| else | ||
| echo "[*] proj_name GHA Input set to: [$proj_name]" | ||
| fi | ||
|
|
||
| proj_submodule_path=$(git config --file .gitmodules --get submodule.$proj_name.path) | ||
|
|
||
| if [ "$proj_submodule_path" ]; then | ||
| echo "[*] Existing submodule config for [$proj_name]:" | ||
| git config --file .gitmodules --get-regexp ^submodule.$proj_name\. | ||
|
|
||
| echo "[*] Submodule status [$proj_name]:$(git submodule status -- $proj_submodule_path)" | ||
|
|
||
| echo "[*] Setting remote url and branch for submodule [$proj_name]" | ||
|
|
||
| [ "$proj_seturl" ] && git submodule set-url -- $proj_submodule_path $proj_seturl | ||
| [ "$proj_branch" ] && git submodule set-branch --branch $proj_branch -- $proj_submodule_path | ||
|
|
||
| echo "[*] Modified submodule config for [$proj_name]:" | ||
| git config --file .gitmodules --get-regexp ^submodule.$proj_name\. | ||
|
|
||
|
|
||
| echo "[*] Fetching latest submodule commit from remote" | ||
| git submodule sync -- $proj_submodule_path | ||
| git submodule update --init --remote --force -- $proj_submodule_path | ||
| if [ $? -gt 0 ]; then | ||
| echo "[-] Could not update submodule from remote. Aborting..." | ||
| else | ||
| echo "[+] Bump submodule [$proj_name]:$(git submodule status -- $proj_submodule_path)" | ||
| git \ | ||
| -c user.name=therockbot \ | ||
| -c user.email=therockbot@amd.com \ | ||
| commit -a -m "Bump submodule $proj_name with its remote for parent project" | ||
| fi | ||
| else | ||
| echo "[-] Could not find submodule path. Aborting..." | ||
| fi | ||
| else | ||
| echo [*] No project name or url specicfied. Skipping submodule fetch... | ||
| fi | ||
| - name: Fetch sources | ||
| run: | | ||
| #Continue with normal fetching of sources | ||
| ./build_tools/fetch_sources.py --jobs 12 | ||
|
|
||
| - name: Install python deps | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level feedback: I see two main ways to build out #559 with GitHub Actions, and this PR currently sits a bit awkwardly between them, while I think we should lean strongly into one or the other.
For (1), you can either edit the source tree as you have it now, with commands like
git submodule set-urlor you can runfetch_sources.pyfirst, then go into the submodule and make local changes using e.g.git fetch. Since the source tree is only used within the context of that workflow, there is no strong need to make those changes persistent (e.g. by writing patches to https://github.com/ROCm/TheRock/tree/main/patches or editing.gitmodules).For (2), we would create branches like what @marbre has done for PRs like #532 (no patches affected) and #562 (one patch dropped).
I like option 2 for how well it composes with other development activities. If we took what you have for "set one subproject to this remote url and branch" and put it in a python script, we could use that script both "try jobs" (this PR) and for routine submodule updates.
That being said, option 1 is simpler so long as our build/test/etc. workflows are also simple. A try job that runs just
change_subproject_ref.pyfetch_sources.pycmake --build ...Should be pretty easy to reason about and maintain.