Skip to content

scripts: Add support to checkout specific commits for alsa repos#10869

Open
dbaluta wants to merge 2 commits into
mainfrom
add_commits
Open

scripts: Add support to checkout specific commits for alsa repos#10869
dbaluta wants to merge 2 commits into
mainfrom
add_commits

Conversation

@dbaluta

@dbaluta dbaluta commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Replace the hardcoded COMMIT_ID array with runtime command line parameters.

Each repo can now be checked out to a specific commit or tag by passing ---commit= on the command line, e.g.:

--alsa-lib-commit=v1.2.3
--alsa-utils-commit=abc1234

If no commit is provided for a specific repo then the code will be updated with latest code on remote. This is a slight modification from the original code where a list of hardcoded commits was used.

But that behavior was already broken, e.g the hardcoded commit for alsa-utils cannot be found in git tree anyway.

Also, remove check_commit() as it's no longer needed.

Copilot AI review requested due to automatic review settings June 10, 2026 14:59
Replace the hardcoded COMMIT_ID array with runtime command line
parameters.

Each repo can now be checked out to a specific commit or tag by passing
--<repo-name>-commit=<ref> on the command line, e.g.:

  --alsa-lib-commit=v1.2.3
  --alsa-utils-commit=abc1234

If no commit is provided for a specific repo then the code will
be updated with latest code on remote. This is a slight modification
from the original code where a list of hardcoded commits was used.

But that behavior was already broken, e.g the hardcoded commit
for alsa-utils cannot be found in git tree anyway.

Also, remove check_commit() as it's no longer needed.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates scripts/build-alsa-tools.sh to remove the hardcoded per-repo commit list and instead allow selecting a specific commit/tag at runtime via --<repo-name>-commit=<ref> command-line options, defaulting to updating from the remote when not provided.

Changes:

  • Replaces the COMMIT_ID array with a REPO_COMMIT associative array populated from --*-commit=* CLI args.
  • Removes the check_commit() helper and adds logic to fetch + checkout a user-specified ref per repo.
  • Falls back to the existing update behavior (fetch + pull) when no per-repo ref override is provided.

update_repo "$repo_dir"
fi

build_and_install "$repo_dir" "${CONFIGURE_ARGS[i]}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is a pre-existing issue and I think it doesnt really matter. there is no TARGET_ARGS passed and that's fine.

Comment on lines 56 to 58
# Function to update the repository
update_repo() {
local repo_dir="$1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Implemented in cd0c6b4. update_repo() now detects detached HEAD, resolves the remote default branch from origin/HEAD, switches back to that branch (creating a tracking branch if needed), and then runs git pull.

key="${arg#--}"
REPO_COMMIT["${key%-commit=*}"]="${key#*-commit=}" ;;
esac
done

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@lgirdwood @singalsu Can you check? This is now against the idea Liam had with hard coded git commits. The idea would be that SOF would be always built with a specific version of alsa-lib/tools that is known to work. If e.g. topology PR needs a newer ALSA, the same PR could modify the ALSA reqs.

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.

I'm generally fine if they override, but maybe west is the best way forward here @dbaluta ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants