Skip to content

fix(github): strip OpenGrep platform suffixes#10166

Merged
jdx merged 1 commit into
jdx:mainfrom
risu729:fix/opengrep-platform-suffix-cleanup
May 31, 2026
Merged

fix(github): strip OpenGrep platform suffixes#10166
jdx merged 1 commit into
jdx:mainfrom
risu729:fix/opengrep-platform-suffix-cleanup

Conversation

@risu729
Copy link
Copy Markdown
Contributor

@risu729 risu729 commented May 31, 2026

Summary

  • extract the OpenGrep install-name cleanup from fix(github): handle x86 release assets as x64 fallback #10103
  • strip raw binary suffixes like osx, manylinux, and musllinux through the shared install helper
  • move platform/version token classification into one backend helper so preferred-name matching and binary cleanup stay in sync

Notes

Tests

  • cargo fmt --all
  • git diff --check
  • cargo test test_clean_binary_name (passed before the final token-scope split; rerun hit an unrelated compile error in untouched src/backend/aqua.rs)
  • cargo test test_platform_or_version_tokens_include_linux_variants (passed before the final token-scope split)
  • cargo test test_preferred_name_handles_tar_and_split_platform_tokens (passed before the final token-scope split)

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 refactors platform token detection by centralizing OS and architecture tokens into a new platform_tokens module, replacing duplicate definitions and adding corresponding tests. Feedback suggests optimizing the binary name cleaning loop to avoid unnecessary heap allocations and using strip_prefix instead of manual slicing to prevent potential UTF-8 boundary panics.

Comment thread src/backend/static_helpers.rs
Comment thread src/backend/platform_tokens.rs Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 31, 2026

Greptile Summary

This PR extracts platform/version token classification into a new shared platform_tokens module, consolidating the previously duplicated is_platform_or_version_token function and OS/arch token lists from asset_matcher.rs and static_helpers.rs. The primary functional addition is appending osx, manylinux, and musllinux to BINARY_OS_TOKENS so clean_binary_name can strip OpenGrep-style suffixes.

  • New platform_tokens.rs module: separates tokens into BINARY_* (used by clean_binary_name) and PREFERRED_NAME_* buckets while preserving every token from the original is_platform_or_version_token match list, plus gaining manylinux2014/musllinux_1_2 versioned-prefix coverage via starts_with.
  • clean_binary_name early-exit guards: the new contains(os) / contains(arch) short-circuits before building format strings, a minor performance improvement with no correctness impact since the actual removal still uses exact rfind patterns.
  • Three new tests in static_helpers.rs verify the osx, manylinux, and musllinux cases that motivated the change.

Confidence Score: 5/5

Safe to merge — the refactoring is a clean extraction with no regressions and the new tokens fix the targeted OpenGrep binary naming issue.

All tokens from the original is_platform_or_version_token match block are accounted for in the new module; the three new OS tokens (osx, manylinux, musllinux) are additive and narrowly scoped; early-exit contains() guards are purely a performance optimisation that cannot produce false strips because the actual removal still uses exact rfind patterns.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/platform_tokens.rs New shared module extracted from asset_matcher.rs; adds osx/manylinux/musllinux to BINARY_OS_TOKENS and separates BINARY vs PREFERRED_NAME token lists with a unit test.
src/backend/static_helpers.rs Replaces local OS_PATTERNS/ARCH_PATTERNS with the new shared BINARY_OS_TOKENS/BINARY_ARCH_TOKENS; adds early-exit contains() guards and three new clean_binary_name test cases for osx/manylinux/musllinux.
src/backend/asset_matcher.rs Removes the local is_platform_or_version_token definition and imports it from the new platform_tokens module instead.
src/backend/mod.rs Registers the new platform_tokens module as a private (mod) submodule.

Reviews (2): Last reviewed commit: "fix(github): strip OpenGrep platform suf..." | Re-trigger Greptile

Comment thread src/backend/platform_tokens.rs
@risu729 risu729 force-pushed the fix/opengrep-platform-suffix-cleanup branch from ebf8723 to 62617b5 Compare May 31, 2026 09:15
@risu729 risu729 marked this pull request as ready for review May 31, 2026 14:42
@jdx jdx merged commit 0d255d8 into jdx:main May 31, 2026
32 checks passed
@risu729 risu729 deleted the fix/opengrep-platform-suffix-cleanup branch May 31, 2026 15:55
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.

2 participants