-
Notifications
You must be signed in to change notification settings - Fork 71
fix: resolve executable path when run via PATH #359
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -87,17 +87,126 @@ struct CliOptions { | |
| DECO_CFG_END(); | ||
| }; | ||
|
|
||
| auto search_in_path(std::string_view name) -> std::string { | ||
| const char* path_env = std::getenv("PATH"); | ||
| if(!path_env || name.empty()) { | ||
| return std::string(name); | ||
| } | ||
|
|
||
| #ifdef _WIN32 | ||
| constexpr char path_sep = ';'; | ||
|
|
||
| // Get PATHEXT or use default extensions | ||
| std::vector<std::string> extensions; | ||
| const char* pathext_env = std::getenv("PATHEXT"); | ||
| if(pathext_env) { | ||
| std::string_view pathext_view(pathext_env); | ||
| size_t ext_start = 0; | ||
| while(ext_start < pathext_view.size()) { | ||
| size_t ext_end = pathext_view.find(';', ext_start); | ||
| if(ext_end == std::string_view::npos) { | ||
| ext_end = pathext_view.size(); | ||
| } | ||
| std::string_view ext = pathext_view.substr(ext_start, ext_end - ext_start); | ||
| if(!ext.empty()) { | ||
| extensions.emplace_back(ext); | ||
| } | ||
| ext_start = ext_end + 1; | ||
| } | ||
| } else { | ||
| extensions = {".exe", ".cmd", ".bat", ".com"}; | ||
| } | ||
|
|
||
| // Check if name already has an extension | ||
| bool has_extension = name.find('.') != std::string_view::npos; | ||
|
|
||
| auto is_executable = [](const std::filesystem::path& p) { | ||
| std::error_code ec; | ||
| auto status = std::filesystem::status(p, ec); | ||
| return !ec && std::filesystem::exists(status) && !std::filesystem::is_directory(status); | ||
| }; | ||
| #else | ||
| constexpr char path_sep = ':'; | ||
| auto is_executable = [](const std::filesystem::path& p) { | ||
| std::error_code ec; | ||
| auto status = std::filesystem::status(p, ec); | ||
| if(ec || !std::filesystem::exists(status) || std::filesystem::is_directory(status)) { | ||
| return false; | ||
| } | ||
| return (status.permissions() & (std::filesystem::perms::owner_exec | | ||
| std::filesystem::perms::group_exec | | ||
| std::filesystem::perms::others_exec)) != std::filesystem::perms::none; | ||
| }; | ||
| #endif | ||
|
|
||
| std::string_view path_view(path_env); | ||
| size_t start = 0; | ||
| while(start < path_view.size()) { | ||
| size_t end = path_view.find(path_sep, start); | ||
| if(end == std::string_view::npos) { | ||
| end = path_view.size(); | ||
| } | ||
|
|
||
| std::string_view dir = path_view.substr(start, end - start); | ||
| if(!dir.empty()) { | ||
| #ifdef _WIN32 | ||
| // Try the name as-is first | ||
| std::filesystem::path full_path = std::filesystem::path(dir) / name; | ||
| std::error_code ec; | ||
| auto canonical = std::filesystem::canonical(full_path, ec); | ||
| if(!ec && is_executable(canonical)) { | ||
| return canonical.string(); | ||
| } | ||
|
|
||
| // If name doesn't have an extension, try each PATHEXT extension | ||
| if(!has_extension) { | ||
| for(const auto& ext : extensions) { | ||
| std::string name_with_ext = std::string(name) + ext; | ||
| full_path = std::filesystem::path(dir) / name_with_ext; | ||
| canonical = std::filesystem::canonical(full_path, ec); | ||
| if(!ec && is_executable(canonical)) { | ||
| return canonical.string(); | ||
| } | ||
| } | ||
| } | ||
| #else | ||
| std::filesystem::path full_path = std::filesystem::path(dir) / name; | ||
| std::error_code ec; | ||
| auto canonical = std::filesystem::canonical(full_path, ec); | ||
| if(!ec && is_executable(canonical)) { | ||
| return canonical.string(); | ||
| } | ||
| #endif | ||
| } | ||
|
Comment on lines
+144
to
+180
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result:
Sources: [1] SearchPathA (Microsoft Learn) • [2] POSIX environment variable PATH (The Open Group) 🏁 Script executed: # Check if the function has any current directory handling
cd src && grep -n "current_path\|current_directory\|\"\.\"\|dir\.empty()" clice.cc | head -30Repository: clice-io/clice Length of output: 88 🏁 Script executed: # Find the search_in_path function and display it with context
cd src && grep -n "search_in_path" clice.cc | head -5Repository: clice-io/clice Length of output: 157 🏁 Script executed: # Display the complete search_in_path function (approximately lines 90-210)
cd src && sed -n '90,210p' clice.ccRepository: clice-io/clice Length of output: 4406 Ensure current directory is checked as part of executable search. The loop skips empty PATH entries ( 🤖 Prompt for AI Agents |
||
|
|
||
| start = end + 1; | ||
| } | ||
|
|
||
| return std::string(name); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| auto resolve_self_path(int argc, const char** argv) -> std::string { | ||
| if(argc <= 0 || argv == nullptr || argv[0] == nullptr) { | ||
| return "clice"; | ||
| } | ||
|
|
||
| std::string_view arg0(argv[0]); | ||
| std::error_code ec; | ||
| auto absolute = std::filesystem::absolute(argv[0], ec); | ||
| if(ec) { | ||
| return std::string(argv[0]); | ||
|
|
||
| // If arg0 contains a path separator, treat it as a path (relative or absolute) | ||
| if(arg0.find('/') != std::string_view::npos || arg0.find('\\') != std::string_view::npos) { | ||
| auto absolute = std::filesystem::absolute(arg0, ec); | ||
| if(!ec) { | ||
| auto canonical = std::filesystem::weakly_canonical(absolute, ec); | ||
| if(!ec) { | ||
| return canonical.string(); | ||
| } | ||
| } | ||
| return std::string(arg0); | ||
| } | ||
| return absolute.string(); | ||
|
|
||
| // No path separator - search in PATH | ||
| return search_in_path(arg0); | ||
| } | ||
|
|
||
| auto build_options(const CliOptions& cli_options, int argc, const char** argv) | ||
|
|
||
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.
🧩 Analysis chain
🌐 Web query:
In standard C++, which header declaresstd::getenv, and is usingstd::getenvwithout includingguaranteed to be portable?💡 Result:
std::getenvis declared in<cstdlib>. [1][2]std::getenvwithout including<cstdlib>is not guaranteed portable—it may “work” only because some other header (or your implementation) happens to include/declare it, but the standard doesn’t require that. You should include<cstdlib>yourself. [2][3]Sources:
[1] cppreference:
<cstdlib>header contents (listsgetenv) (en.cppreference.com)[2] cppreference: C++ headers / requirements about what headers provide (en.cppreference.com)
[3] Stack Overflow discussion noting non-portability of relying on indirect declarations (stackoverflow.com)
Citations:
Add
<cstdlib>forstd::getenvportability.The code uses
std::getenvwithout including<cstdlib>, which is not guaranteed to be portable across standard-compliant implementations.Suggested fix
+#include <cstdlib> `#include` <expected> `#include` <filesystem>🤖 Prompt for AI Agents