-
-
Notifications
You must be signed in to change notification settings - Fork 769
feat(github): add rename_exe option and switch elm, opam, yt-dlp from ubi #7140
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -345,6 +345,13 @@ pub fn install_artifact( | |||||||||||||
|
|
||||||||||||||
| // Extract with determined strip_components | ||||||||||||||
| file::untar(file_path, &install_path, &tar_opts)?; | ||||||||||||||
|
|
||||||||||||||
| // Handle rename_exe option for archives | ||||||||||||||
| if let Some(rename_to) = | ||||||||||||||
| lookup_platform_key(opts, "rename_exe").or_else(|| opts.get("rename_exe").cloned()) | ||||||||||||||
| { | ||||||||||||||
| rename_executable_in_dir(&install_path, &rename_to)?; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Ok(()) | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -393,6 +400,35 @@ pub fn verify_checksum_str( | |||||||||||||
| Ok(()) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Renames the first executable file found in a directory to a new name. | ||||||||||||||
| /// Used by the `rename_exe` option to rename binaries after archive extraction. | ||||||||||||||
| fn rename_executable_in_dir(dir: &Path, new_name: &str) -> eyre::Result<()> { | ||||||||||||||
| // Find executables in the directory (non-recursive for top level) | ||||||||||||||
| for entry in std::fs::read_dir(dir)?.flatten() { | ||||||||||||||
| let path = entry.path(); | ||||||||||||||
| if path.is_file() && crate::file::is_executable(&path) { | ||||||||||||||
| let file_name = path.file_name().unwrap().to_string_lossy(); | ||||||||||||||
|
||||||||||||||
| let file_name = path.file_name().unwrap().to_string_lossy(); | |
| let file_name = path | |
| .file_name() | |
| .ok_or_else(|| eyre::eyre!("Path '{}' has no file name", path.display()))? | |
| .to_string_lossy(); |
Copilot
AI
Nov 28, 2025
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.
The function returns Ok(()) when no executable is found, which silently ignores the case where rename_exe was specified but no suitable executable exists. This should return an error to alert users that the rename operation failed.
| Ok(()) | |
| bail!( | |
| "No executable file found in directory '{}' to rename to '{}'", | |
| dir.display(), | |
| new_name | |
| ); |
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.
Using
flatten()silently ignores read errors for individual directory entries. Consider handling these errors explicitly or logging them to avoid silently skipping files that fail to read.