feat: add workdir option to run commands#1604
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new feature that allows users to specify the working directory inside the application container when running commands using Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| this->printer.printErr(LINGLONG_ERRV(msg)); | ||
| return -1; | ||
| } | ||
| process.cwd = workdir; |
There was a problem hiding this comment.
为了提高代码的明确性和可移植性，建议在将 std::filesystem::path 类型的 workdir 赋给 std::string 类型的 process.cwd 时，显式调用 .string() 方法。虽然隐式转换在许多情况下有效，但显式转换可以避免在处理非 ASCII 路径时可能出现的编码问题，使代码意图更清晰。
process.cwd = workdir.string();
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a --workdir option to the ll-builder run and ll-cli run commands, allowing users to specify the working directory inside the application container. The feature validates that the provided path is absolute and gracefully falls back to default behavior when the option is not specified.
Changes:
- Added
workdirfield toRunCommandOptions,RunOptions, andBuilder::runmethod signature with appropriate defaults for backward compatibility - Implemented workdir validation (must be absolute path) and assignment to
process.cwdin bothCli::runandBuilder::runmethods - Registered
--workdirCLI option in bothll-cliandll-builderrun subcommands
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
libs/linglong/src/linglong/cli/cli.h |
Added workdir field to RunOptions struct |
libs/linglong/src/linglong/cli/cli.cpp |
Implemented workdir validation and process.cwd assignment in Cli::run |
libs/linglong/src/linglong/builder/linglong_builder.h |
Added workdir parameter to Builder::run signature |
libs/linglong/src/linglong/builder/linglong_builder.cpp |
Implemented workdir validation and process.cwd assignment in Builder::run |
apps/ll-cli/src/main.cpp |
Added --workdir CLI option to run subcommand |
apps/ll-builder/src/main.cpp |
Added --workdir CLI option and passes it to Builder::run |
apps/ll-builder/src/command_options.h |
Added workdir field to RunCommandOptions struct |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cliRun->add_option("--workdir", runOptions.workdir, _("Working directory inside the app")) | ||
| ->type_name("PATH"); |
There was a problem hiding this comment.
The CLI flag --workdir is inconsistent with the existing --working-directory flag used in the enter subcommand (see apps/ll-cli/src/main.cpp:227). For a consistent user experience across ll-cli subcommands, consider using --working-directory here as well (or at least adding it as an alias).
| this->printer.printErr(LINGLONG_ERRV(msg)); | ||
| return -1; | ||
| } | ||
| process.cwd = workdir; |
There was a problem hiding this comment.
process.cwd = workdir; relies on implicit conversion from std::filesystem::path to std::string. While this works on POSIX, the rest of the codebase consistently uses explicit .string() calls when converting std::filesystem::path to std::string (e.g., cli.cpp:679, cli.cpp:807). Use workdir.string() for consistency and clarity.
| utils::error::Result<void> Builder::run(std::vector<std::string> modules, | ||
| std::vector<std::string> args, | ||
| bool debug, | ||
| std::string workdir, |
There was a problem hiding this comment.
const std::string &workdir
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: myml The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这段代码主要是为 以下是详细的审查意见和改进建议： 1. 代码逻辑与安全性意见 1：路径验证逻辑存在重复
意见 2：路径有效性验证不充分
意见 3：默认值处理
2. 代码质量意见 1：字符串拷贝性能 std::string workdir = "",这里按值传递参数。虽然对于短字符串（如路径）来说，现代 C++ 的 SSO (Small String Optimization) 优化使得拷贝开销很小，但作为最佳实践，参数传递应考虑使用
意见 2：类型转换与格式化 auto msg = fmt::format("Workdir must be an absolute path: {}", workdir);这里
3. 具体代码修改建议A. 统一路径验证逻辑 (建议在公共头文件中添加)// 假设放在 utils/path.h
#include <filesystem>
#include <string>
#include <system_error>
namespace linglong::utils {
inline bool IsValidAbsolutePath(const std::string &pathStr) {
if (pathStr.empty()) {
return false;
}
std::error_code ec;
std::filesystem::path p(pathStr);
// 检查是否为绝对路径
if (!p.is_absolute()) {
return false;
}
// 可选：检查路径是否包含非法字符（如 null）
if (pathStr.find('\0') != std::string::npos) {
return false;
}
return true;
}
} // namespace linglong::utilsB. 修改
|
Add --workdir option to ll-builder and ll-cli run commands to specify working directory inside the application container 1. Add workdir field to RunCommandOptions and RunOptions structs 2. Update Builder::run method to accept workdir parameter and set process.cwd 3. Update Cli::run method to handle workdir option and validate absolute path 4. Add CLI option parsing for --workdir in both ll-builder and ll-cli 5. Validate that workdir path is absolute before setting in container process 6. Maintain backward compatibility with empty workdir (default behavior) Log: Added --workdir option to specify working directory inside container when running applications Influence: 1. Test ll-builder run --workdir=/path/to/dir with absolute paths 2. Test ll-cli run <app> --workdir=/path/to/dir with absolute paths 3. Verify error handling when providing relative paths 4. Test empty workdir to ensure default behavior unchanged 5. Test with various commands and modules to ensure workdir is properly set 6. Verify workdir persists through command execution within container feat: 为运行命令添加工作目录选项 为 ll-builder 和 ll-cli 的 run 命令添加 --workdir 选项，用于指定应用程序 容器内的工作目录 1. 在 RunCommandOptions 和 RunOptions 结构体中添加 workdir 字段 2. 更新 Builder::run 方法以接受 workdir 参数并设置 process.cwd 3. 更新 Cli::run 方法以处理 workdir 选项并验证绝对路径 4. 在 ll-builder 和 ll-cli 中添加 CLI 选项解析支持 --workdir 5. 在容器进程中设置工作目录前验证路径是否为绝对路径 6. 保持向后兼容性，空工作目录使用默认行为 Log: 新增 --workdir 选项用于在运行应用程序时指定容器内的工作目录 Influence: 1. 测试 ll-builder run --workdir=/path/to/dir 使用绝对路径 2. 测试 ll-cli run <app> --workdir=/path/to/dir 使用绝对路径 3. 验证提供相对路径时的错误处理 4. 测试空工作目录以确保默认行为不变 5. 使用各种命令和模块测试工作目录是否正确设置 6. 验证工作目录在容器内的命令执行过程中保持不变
Add --workdir option to ll-builder and ll-cli run commands to specify working directory inside the application container
Log: Added --workdir option to specify working directory inside container when running applications
Influence:
feat: 为运行命令添加工作目录选项
为 ll-builder 和 ll-cli 的 run 命令添加 --workdir 选项，用于指定应用程序 容器内的工作目录
Log: 新增 --workdir 选项用于在运行应用程序时指定容器内的工作目录
Influence: