-
Notifications
You must be signed in to change notification settings - Fork 122
fix(installer): move binary name to avoid conflicts #888
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
Conversation
WalkthroughThis pull request adds support for custom binary naming during installation across GitHub and source-based installation flows. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🪛 Ruff (0.14.10)secator/installer.py95-95: Unused Remove unused (RUF100) 246-246: Unused class method argument: (ARG003) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
secator/installer.py (1)
246-321:binary_nameparameter is accepted but never used - source installations won't rename binaries.The
binary_nameparameter is added to the signature but the method never uses it. When a tool likehttpxis installed from source (viago install), the binary will be installed ashttpxinGOBIN, not renamed tohttpx-toolkit. This means:
go install github.com/projectdiscovery/httpx/cmd/httpx@v1.7.0installs binary ashttpxbinary_name='httpx-toolkit'is ignoredcmd = 'httpx-toolkit -irh'will fail because nohttpx-toolkitbinary existsThe static analysis correctly flagged this as an unused argument (ARG003).
🔧 Proposed fix: Rename binary after successful source installation
@classmethod def install(cls, config, version=None, install_prereqs=True, binary_name=None): """Install from source. Args: cls: ToolInstaller class. config (dict): A dict of distros as keys and a command as value. version (str, optional): Version to install. install_prereqs (bool, optional): Install pre-requisites. binary_name (str, optional): Custom binary name to use after installation. Returns: Status: install status. """ install_cmd = None + original_binary_name = None if isinstance(config, str): install_cmd = config + # Extract original binary name from install command for renaming + if binary_name: + # Try to extract the original name from common patterns + if 'go install' in install_cmd: + # e.g., go install github.com/user/repo/cmd/toolname@version + match = re.search(r'/cmd/([^@/]+)@', install_cmd) + if match: + original_binary_name = match.group(1) else: # ... existing code ... # ... existing code until the end ... # Run command ret = Command.execute(install_cmd, cls_attributes={'shell': True}, quiet=False) if ret.return_code != 0: return InstallerStatus.INSTALL_FAILED + + # Rename binary if custom name is specified + if binary_name and original_binary_name and binary_name != original_binary_name: + # Find the installed binary and rename it + original_path = shutil.which(original_binary_name) + if original_path: + target_dir = os.path.dirname(original_path) + target_path = os.path.join(target_dir, binary_name) + console.print(Info(message=f'Renaming {original_binary_name} to {binary_name}...')) + shutil.move(original_path, target_path) + return InstallerStatus.SUCCESSAlternatively, you may need to pass the original tool name (e.g., from
tool_cls.cmd.split(' ')[0]) to properly locate and rename the binary after source installation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
secator/installer.pysecator/runners/command.pysecator/tasks/httpx.py
🧰 Additional context used
🪛 Ruff (0.14.10)
secator/installer.py
95-95: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
246-246: Unused class method argument: binary_name
(ARG003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (6)
secator/runners/command.py (1)
84-94: LGTM!The new
install_binary_nameattribute is well-placed alongside other installation-related attributes and follows the existing naming convention. The defaultNonevalue is appropriate, allowing tools to optionally override when they need a custom binary name.secator/tasks/httpx.py (1)
21-21: LGTM! Consistent binary renaming to avoid conflict.The changes are well-coordinated:
cmdupdated to use the new binary namehttpx-toolkitinstall_binary_nameset to matchinstall_cmdcorrectly retains the original Go package path (github.com/projectdiscovery/httpx/cmd/httpx)The installer will download/build the original
httpxbinary and rename it tohttpx-toolkit, avoiding the conflict with Python'shttpxlibrary.Also applies to: 79-81
secator/installer.py (4)
89-101: LGTM! Binary name extraction and propagation to GithubInstaller.The binary name is correctly retrieved from the tool class and passed to the GitHub installer path.
328-357: LGTM! GithubInstaller properly handles binary renaming.The implementation correctly:
- Accepts
binary_nameparameter- Computes
final_binary_namewith fallback to repo name- Passes both
repo_name(to find binary in archive) andbinary_name(for final destination) to_download_and_unpack
447-491: LGTM! Archive extraction correctly handles binary renaming.The separation of
repo_name(for finding the binary in the archive) andbinary_name(for the final destination) is the right approach. The binary is correctly:
- Located in the archive using the original
repo_name- Made executable
- Moved to the destination with the custom
binary_name
115-115: Note: binary_name is passed but not used by SourceInstaller.install.This call correctly passes
binary_name, but as noted above, theSourceInstaller.installmethod doesn't actually use it to rename the installed binary.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.