Skip to content

fix: correct split-repo-post-process.sh regex replacement#8305

Open
shivanee-p wants to merge 1 commit into
mainfrom
shivaneep-owlbot-fixes
Open

fix: correct split-repo-post-process.sh regex replacement#8305
shivanee-p wants to merge 1 commit into
mainfrom
shivaneep-owlbot-fixes

Conversation

@shivanee-p
Copy link
Copy Markdown
Contributor

@shivanee-p shivanee-p commented May 18, 2026

This PR fixes a bug in the monorepo's split-repo post-processing script bin/split-repo-post-process.sh which caused newly migrated/split packages to generate duplicate code files inside a literal folder named $1.

Because the capture group in source was removed, the trailing /$1 reference in dest has no capture group to bind to. The regular expression replacement logic was tested under both standard and scoped package scenarios to verify the fix.

@shivanee-p shivanee-p requested a review from a team as a code owner May 18, 2026 21:42
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the bin/split-repo-post-process.sh script to update the destination path in the .OwlBot.yaml configuration file, specifically removing a trailing variable from the destination string. A review comment identifies a potential issue where the gsed command might fail if the package name contains slashes (e.g., scoped packages) and suggests using a different delimiter like | to improve robustness.

echo "OwlBot config is copying each folder"
gsed -i 's/\.\*-nodejs\/(.*)/.*-nodejs/' "${PACKAGE_PATH}/.OwlBot.yaml"
gsed -i "s/dest: \/owl-bot-staging\/\$1\/\$2/dest: \/owl-bot-staging\/${PACKAGE_NAME}\/\$1/" "${PACKAGE_PATH}/.OwlBot.yaml"
gsed -i "s/dest: \/owl-bot-staging\/\$1\/\$2/dest: \/owl-bot-staging\/${PACKAGE_NAME}/" "${PACKAGE_PATH}/.OwlBot.yaml"
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.

high

The gsed command uses / as a delimiter, which will cause the command to fail if ${PACKAGE_NAME} contains a slash (e.g., for scoped packages like @google-cloud/storage). Using a different delimiter such as | is safer when dealing with paths or package names that may contain slashes.

Suggested change
gsed -i "s/dest: \/owl-bot-staging\/\$1\/\$2/dest: \/owl-bot-staging\/${PACKAGE_NAME}/" "${PACKAGE_PATH}/.OwlBot.yaml"
gsed -i "s|dest: /owl-bot-staging/\$1/\$2|dest: /owl-bot-staging/${PACKAGE_NAME}|" "${PACKAGE_PATH}/.OwlBot.yaml"

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.

1 participant