Skip to content

A few more checks for usr_merged checks#2065

Open
loosebazooka wants to merge 1 commit intomainfrom
more_usr_merged
Open

A few more checks for usr_merged checks#2065
loosebazooka wants to merge 1 commit intomainfrom
more_usr_merged

Conversation

@loosebazooka
Copy link
Copy Markdown
Member

No description provided.

@loosebazooka loosebazooka requested a review from thesayyn April 17, 2026 15:19
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2026

🌳 🔧 Config Check

This pull request has not modified the root BUILD

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 updates the path normalization logic in validate_usr_symlinks.awk to handle redundant leading slashes and dot-slashes, and adds comprehensive test cases to verify these scenarios. Feedback suggests simplifying the normalization loop into a single, more idiomatic AWK sub call using a regular expression to improve efficiency.

Comment thread private/util/validate_usr_symlinks.awk Outdated
sub(/^\//, "", path)
# Normalize: strip any combination of leading ./ and /
# This ensures ./bin, /bin, bin, and even //bin are treated the same.
while (sub(/^\.\//, "", path) || sub(/^\//, "", path)) ;
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.

medium

The while loop for path normalization can be simplified into a single sub call using a regular expression. This is more idiomatic in AWK and avoids the overhead of multiple function calls in a loop. The regex ^(\.\/|\/)+ will match any sequence of leading ./ or / at the start of the string.

    sub(/^(\.\/|\/)+/, "", path)

Signed-off-by: Appu Goundan <appu@google.com>
@github-actions
Copy link
Copy Markdown
Contributor

🌳 🔄 Image Check
This pull request doesn't make any changes to the images. 👍
You can check the details in the report here

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