为init函数添加--separate-git-dir参数 #85
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the --separate-git-dir parameter to the init command, allowing users to specify a separate directory for Git storage. Additionally, all documentation comments and inline comments were translated from English to Chinese. However, the implementation has critical bugs that will prevent the code from compiling, and several important pieces of logic were inadvertently removed.
Key changes:
- Added
separate_git_dirfield toInitArgsstruct - Implemented logic to use separate Git directory when specified
- Translated all documentation and inline comments from English to Chinese
- Removed the
execute()wrapper function, branch validation, object format validation, and Windows PowerShell hook creation
| #[clap(long = "separate-git-dir", value_name = "PATH", required = false)] | ||
| pub separate_git_dir: Option<String>, | ||
| } | ||
|
|
There was a problem hiding this comment.
The execute function was removed, but it's still being called from other parts of the codebase (specifically src/cli.rs:153 and src/command/clone.rs:96). This will cause compilation errors in those modules. You need to either:
- Keep the
executefunction as a wrapper aroundinit, or - Update all call sites to call
initdirectly instead ofexecute
If option 1, add this function back after the InitArgs struct:
/// Execute the init function
pub async fn execute(args: InitArgs) {
match init(args).await {
Ok(_) => {}
Err(e) => {
eprintln!("Error: {e}");
}
}
}| /// Execute the init function | |
| pub async fn execute(args: InitArgs) { | |
| match init(args).await { | |
| Ok(_) => {} | |
| Err(e) => { | |
| eprintln!("Error: {e}"); | |
| } | |
| } | |
| } |
| "初始化失败：指定的位置已经初始化过仓库。如果需要重新初始化，请删除现有的目录或文件。", | ||
| )); | ||
| } | ||
|
|
There was a problem hiding this comment.
The branch name validation logic was removed but is necessary for the init function. Without this validation, invalid branch names could be accepted and cause issues later. The validation should be added back before line 222:
// Check if the branch name is valid
if let Some(ref branch_name) = args.initial_branch
&& !branch::is_valid_git_branch_name(branch_name)
{
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"invalid branch name: '{branch_name}'.\n\nBranch names must:\n- Not contain spaces, control characters, or any of these characters: \\ : \" ? * [\n- Not start or end with a slash ('/'), or end with a dot ('.')\n- Not contain consecutive slashes ('//') or dots ('..')\n- Not be reserved names like 'HEAD' or contain '@{{'\n- Not be empty or just a dot ('.')\n\nPlease choose a valid branch name."
),
));
}| // Check if the branch name is valid | |
| if let Some(ref branch_name) = args.initial_branch | |
| && !branch::is_valid_git_branch_name(branch_name) | |
| { | |
| return Err(io::Error::new( | |
| io::ErrorKind::InvalidInput, | |
| format!( | |
| "invalid branch name: '{branch_name}'.\n\nBranch names must:\n- Not contain spaces, control characters, or any of these characters: \\ : \" ? * [\n- Not start or end with a slash ('/'), or end with a dot ('.')\n- Not contain consecutive slashes ('//') or dots ('..')\n- Not be reserved names like 'HEAD' or contain '@{{'\n- Not be empty or just a dot ('.')\n\nPlease choose a valid branch name." | |
| ), | |
| )); | |
| } |
| use std::os::unix::fs::PermissionsExt; | ||
| let perms = fs::Permissions::from_mode(0o755); | ||
| fs::set_permissions(root_dir.join("hooks").join("pre-commit.sh"), perms)?; | ||
| } |
There was a problem hiding this comment.
The creation of the Windows PowerShell pre-commit hook file (pre-commit.ps1) was removed. This breaks the template initialization for Windows users. The following code should be added after line 271 to maintain Windows support:
// Create .libra/hooks/pre-commit.ps1
fs::write(
root_dir.join("hooks").join("pre-commit.ps1"),
include_str!("../../template/pre-commit.ps1"),
)?;| format!("fatal: invalid object format '{format}'"), | ||
| )); | ||
| separate_git_path.to_path_buf() // 使用指定的目录 | ||
| } |
There was a problem hiding this comment.
The implementation of --separate-git-dir is incomplete. When using --separate-git-dir, Git creates a .git file in the working directory that contains a reference to the actual git directory (a "gitlink"). This is missing from the current implementation.
After line 201, you should add logic to create a gitlink file in the working directory when separate_git_dir is specified:
// When using separate-git-dir, create a .git file that points to the actual git directory
if !args.bare {
let gitlink_path = cur_dir.join(".git");
let gitlink_content = format!("gitdir: {}", separate_git_path.display());
fs::write(gitlink_path, gitlink_content)?;
}This ensures that Git commands executed in the working directory can find the actual Git directory.
| } | |
| } | |
| // When using --separate-git-dir, create a .git file in the working directory that points to the actual git directory | |
| if args.separate_git_dir.is_some() && !args.bare { | |
| let separate_git_path = Path::new(args.separate_git_dir.as_ref().unwrap()); | |
| let gitlink_path = cur_dir.join(".git"); | |
| let gitlink_content = format!("gitdir: {}", separate_git_path.display()); | |
| fs::write(gitlink_path, gitlink_content)?; | |
| } |
src/command/init.rs
Outdated
| /// 创建一个裸仓库 | ||
| #[clap(long, required = false)] | ||
| pub bare: bool, // Default is false | ||
| pub bare: bool, // 默认值是 false | ||
|
|
||
| /// directory from which templates will be used | ||
| /// 用于创建仓库的模板目录 | ||
| #[clap(long = "template", name = "template-directory", required = false)] | ||
| pub template: Option<String>, | ||
|
|
||
| /// Set the initial branch name | ||
| /// 设置初始分支名称 | ||
| #[clap(short = 'b', long, required = false)] | ||
| pub initial_branch: Option<String>, | ||
|
|
||
| /// Create a repository in the specified directory | ||
| /// 在指定目录下创建仓库 | ||
| #[clap(default_value = ".")] | ||
| pub repo_directory: String, | ||
|
|
||
| /// Suppress all output | ||
| /// 抑制所有输出 | ||
| #[clap(long, short = 'q', required = false)] | ||
| pub quiet: bool, | ||
|
|
||
| /// Specify repository sharing mode | ||
| /// | ||
| /// Supported values: | ||
| /// - `umask`: Default behavior (permissions depend on the user's umask). | ||
| /// - `group`: Makes the repository group-writable so multiple users | ||
| /// in the same group can collaborate more easily. | ||
| /// - `all`: Makes the repository readable by all users on the system. | ||
| /// | ||
| /// Note: On Windows, this option is ignored. | ||
| /// 指定仓库共享模式 | ||
| /// 支持值: `umask`, `group`, `all` | ||
| #[clap(long, required = false, value_name = "MODE")] | ||
| pub shared: Option<String>, | ||
|
|
||
| /// Specify the object format (hash algorithm) for the repository. | ||
| /// | ||
| /// Supported values: | ||
| /// - `sha1`: The default and currently the only supported format. | ||
| /// - `sha256`: Recognized as a valid format, but will return an error as it is not yet supported. | ||
| /// 指定仓库对象格式（哈希算法） | ||
| /// 支持值: `sha1`, `sha256` | ||
| #[clap(long = "object-format", name = "format", required = false)] | ||
| pub object_format: Option<String>, | ||
| } | ||
|
|
||
| /// Execute the init function | ||
| pub async fn execute(args: InitArgs) { | ||
| match init(args).await { | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| eprintln!("Error: {e}"); | ||
| } | ||
| } | ||
| /// 指定一个独立的 git 目录 | ||
| #[clap(long = "separate-git-dir", value_name = "PATH", required = false)] | ||
| pub separate_git_dir: Option<String>, |
There was a problem hiding this comment.
[nitpick] The documentation comments for the InitArgs struct fields were changed from English to Chinese. While translation is fine, this deviates from the codebase convention where most documentation is in English (as seen in other functions and modules). According to the custom guidelines, "internal/international teams may include Chinese remarks but primary docs should be English."
Consider keeping the primary documentation in English for broader accessibility, or adding Chinese translations as supplementary comments if needed for the target audience.
| set_hash_kind(match object_format_value.as_str() { | ||
| "sha1" => HashKind::Sha1, | ||
| "sha256" => HashKind::Sha256, | ||
| _ => HashKind::Sha1, | ||
| }); |
There was a problem hiding this comment.
The types HashKind and function set_hash_kind are not imported or defined in this module. This code will fail to compile. You need to add the appropriate import statement at the top of the file, for example:
use crate::internal::hash::{HashKind, set_hash_kind};Please verify the correct module path where these items are defined.
src/command/init.rs
Outdated
| /// 指定一个独立的 git 目录 | ||
| #[clap(long = "separate-git-dir", value_name = "PATH", required = false)] | ||
| pub separate_git_dir: Option<String>, |
There was a problem hiding this comment.
Adding the new field separate_git_dir to InitArgs is a breaking change. The code in src/command/clone.rs:87-95 that constructs InitArgs is missing this new field, which will cause a compilation error since all struct fields must be specified when using struct literal syntax.
You need to update the clone.rs file to include the new field:
let init_args = command::init::InitArgs {
bare: false,
initial_branch: args.branch.clone(),
repo_directory: local_path.to_str().unwrap().to_string(),
quiet: false,
template: None,
shared: None,
object_format: None,
separate_git_dir: None, // Add this line
};
src/command/init.rs
Outdated
| // 创建 .libra 相关目录 | ||
| let dirs = ["objects/pack", "objects/info"]; | ||
| for dir in dirs { | ||
| fs::create_dir_all(root_dir.join(dir))?; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The logic for creating the objects/pack and objects/info directories was moved inside the else block (when no template is provided). However, these directories should always be created regardless of whether a template is used or not, as they are essential for the Git object storage. The original code had this logic outside the template conditional block.
This change could cause issues if a template is provided but doesn't include these directories. Consider moving lines 273-278 outside the else block to ensure these directories are always created.
| // 创建 .libra 相关目录 | |
| let dirs = ["objects/pack", "objects/info"]; | |
| for dir in dirs { | |
| fs::create_dir_all(root_dir.join(dir))?; | |
| } | |
| } | |
| } | |
| // 创建 .libra 相关目录（无论是否使用模板都要创建） | |
| let dirs = ["objects/pack", "objects/info"]; | |
| for dir in dirs { | |
| fs::create_dir_all(root_dir.join(dir))?; | |
| } |
src/command/init.rs
Outdated
| // 获取当前目录 | ||
| let cur_dir = Path::new(&args.repo_directory).to_path_buf(); | ||
| // Join the current directory with the root directory | ||
| let root_dir = if args.bare { | ||
| cur_dir.clone() | ||
| } else { | ||
| cur_dir.join(ROOT_DIR) | ||
| }; | ||
| // check if format is supported | ||
| if let Some(format) = &args.object_format { | ||
| match format.to_lowercase().as_str() { | ||
| "sha1" => { | ||
| // sha1 is supported, do nothing and proceed. | ||
| } | ||
| "sha256" => { | ||
| // sha256 is a valid format, but not supported yet. | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| "fatal: object format 'sha256' is not supported yet", | ||
| )); | ||
|
|
||
| // 处理 --separate-git-dir 参数 | ||
| let root_dir = match &args.separate_git_dir { | ||
| Some(separate_git_dir) => { | ||
| let separate_git_path = Path::new(separate_git_dir); | ||
| if !separate_git_path.exists() { | ||
| fs::create_dir_all(separate_git_path)?; // 如果指定的 Git 目录不存在，则创建 | ||
| } | ||
| _ => { | ||
| // Any other format is invalid. | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| format!("fatal: invalid object format '{format}'"), | ||
| )); | ||
| separate_git_path.to_path_buf() // 使用指定的目录 | ||
| } | ||
| None => { | ||
| if args.bare { | ||
| cur_dir.clone() | ||
| } else { | ||
| cur_dir.join(ROOT_DIR) | ||
| } | ||
| } | ||
| } | ||
| // Check if the root directory already exists | ||
| }; | ||
|
|
||
| // 检查仓库是否已经初始化 | ||
| if is_reinit(&cur_dir) { | ||
| if !args.quiet { | ||
| eprintln!("Already initialized - [{}]", root_dir.display()); | ||
| eprintln!("已经初始化 - [{}]", root_dir.display()); | ||
| } | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::AlreadyExists, | ||
| "Initialization failed: The repository is already initialized at the specified location. | ||
| If you wish to reinitialize, please remove the existing directory or file.", | ||
| "初始化失败：指定的位置已经初始化过仓库。如果需要重新初始化，请删除现有的目录或文件。", | ||
| )); | ||
| } | ||
|
|
||
| // Check if the branch name is valid | ||
| if let Some(ref branch_name) = args.initial_branch | ||
| && !branch::is_valid_git_branch_name(branch_name) | ||
| { | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| format!( | ||
| "invalid branch name: '{branch_name}'.\n\nBranch names must:\n- Not contain spaces, control characters, or any of these characters: \\ : \" ? * [\n- Not start or end with a slash ('/'), or end with a dot ('.')\n- Not contain consecutive slashes ('//') or dots ('..')\n- Not be reserved names like 'HEAD' or contain '@{{'\n- Not be empty or just a dot ('.')\n\nPlease choose a valid branch name." | ||
| ), | ||
| )); | ||
| } | ||
|
|
||
| // Check if the target directory is writable | ||
| // 检查目标目录是否可写 | ||
| match is_writable(&cur_dir) { | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| return Err(e); | ||
| } | ||
| } | ||
|
|
||
| // ensure root dir exists | ||
| // 确保根目录存在 | ||
| fs::create_dir_all(&root_dir)?; | ||
|
|
||
| // If a template path is provided, copy the template files to the root directory | ||
| // 如果提供了模板路径，则复制模板文件到根目录 | ||
| if let Some(template_path) = &args.template { | ||
| let template_dir = Path::new(template_path); | ||
| if template_dir.exists() { | ||
| copy_template(template_dir, &root_dir)?; | ||
| copy_template(template_dir, &root_dir)?; // 复制模板内容 | ||
| } else { | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::NotFound, | ||
| format!("template directory '{}' does not exist", template_path), | ||
| format!("模板目录 '{}' 不存在", template_path), | ||
| )); | ||
| } | ||
| } else { | ||
| // Create info & hooks | ||
| // 创建仓库相关的目录和文件 | ||
| let dirs = ["info", "hooks"]; | ||
| for dir in dirs { | ||
| fs::create_dir_all(root_dir.join(dir))?; | ||
| fs::create_dir_all(root_dir.join(dir))?; // 创建 info 和 hooks 目录 | ||
| } | ||
| // Create info/exclude | ||
| // `include_str!` includes the file content while compiling | ||
|
|
||
| // 创建必要的配置文件 | ||
| fs::write( | ||
| root_dir.join("info/exclude"), | ||
| include_str!("../../template/exclude"), | ||
| )?; | ||
| // Create .libra/description | ||
| fs::write( | ||
| root_dir.join("description"), | ||
| include_str!("../../template/description"), | ||
| )?; | ||
| // Create .libra/hooks/pre-commit.sh | ||
| fs::write( | ||
| root_dir.join("hooks").join("pre-commit.sh"), | ||
| include_str!("../../template/pre-commit.sh"), | ||
| )?; | ||
|
|
||
| // Set Permission | ||
| // 设置文件权限 | ||
| #[cfg(not(target_os = "windows"))] | ||
| { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| let perms = fs::Permissions::from_mode(0o755); | ||
| fs::set_permissions(root_dir.join("hooks").join("pre-commit.sh"), perms)?; | ||
| } | ||
|
|
||
| // Create .libra/hooks/pre-commit.ps1 | ||
| fs::write( | ||
| root_dir.join("hooks").join("pre-commit.ps1"), | ||
| include_str!("../../template/pre-commit.ps1"), | ||
| )?; | ||
| } | ||
|
|
||
| // Complete .libra and sub-directories | ||
| let dirs = ["objects/pack", "objects/info"]; | ||
| for dir in dirs { | ||
| fs::create_dir_all(root_dir.join(dir))?; | ||
| // 创建 .libra 相关目录 | ||
| let dirs = ["objects/pack", "objects/info"]; | ||
| for dir in dirs { | ||
| fs::create_dir_all(root_dir.join(dir))?; | ||
| } | ||
| } | ||
|
|
||
| // Create database: .libra/libra.db | ||
| // 创建数据库 | ||
| let conn; | ||
| let database = root_dir.join(DATABASE); | ||
|
|
||
| #[cfg(target_os = "windows")] | ||
| { | ||
| // On Windows, we need to convert the path to a UNC path | ||
| // Windows 系统需要转换路径格式 | ||
| let database = database.to_str().unwrap().replace("\\", "/"); | ||
| conn = db::create_database(database.as_str()).await?; | ||
| } | ||
|
|
||
| #[cfg(not(target_os = "windows"))] | ||
| { | ||
| // On Unix-like systems, we do no more | ||
| conn = db::create_database(database.to_str().unwrap()).await?; | ||
| } | ||
|
|
||
| // Create config table with bare parameter consideration | ||
| init_config(&conn, args.bare, args.object_format.as_deref()) | ||
| // 初始化配置 | ||
| init_config(&conn, args.bare, Some(object_format_value.as_str())) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| // Determine the initial branch name: use provided name or default to "main" | ||
| // 设置默认的初始分支名称 | ||
| let initial_branch_name = args | ||
| .initial_branch | ||
| .unwrap_or_else(|| DEFAULT_BRANCH.to_owned()); | ||
|
|
||
| // Create HEAD | ||
| // 创建 HEAD 引用 | ||
| reference::ActiveModel { | ||
| name: Set(Some(initial_branch_name.clone())), | ||
| kind: Set(reference::ConfigKind::Head), | ||
| ..Default::default() // all others are `NotSet` | ||
| ..Default::default() | ||
| } | ||
| .insert(&conn) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| // Set .libra as hidden | ||
| // 设置 .libra 为隐藏文件夹 | ||
| set_dir_hidden(root_dir.to_str().unwrap())?; | ||
|
|
||
| // Apply shared permissions if requested | ||
| // 如果指定了共享权限，应用共享设置 | ||
| if let Some(shared_mode) = &args.shared { | ||
| apply_shared(&root_dir, shared_mode)?; | ||
| } | ||
|
|
||
| if !args.quiet { | ||
| let repo_type = if args.bare { "bare " } else { "" }; | ||
| println!( | ||
| "Initializing empty {repo_type}Libra repository in {} with initial branch '{initial_branch_name}'", | ||
| "正在初始化空的 {repo_type}Libra 仓库于 {}，初始分支为 '{initial_branch_name}'", | ||
| root_dir.display() | ||
| ); | ||
| } | ||
|
|
||
| // 设置全局哈希算法 | ||
| set_hash_kind(match object_format_value.as_str() { | ||
| "sha1" => HashKind::Sha1, | ||
| "sha256" => HashKind::Sha256, | ||
| _ => HashKind::Sha1, | ||
| }); |
There was a problem hiding this comment.
[nitpick] All inline comments in the init function were changed from English to Chinese. This creates inconsistency with the rest of the codebase where comments are primarily in English. According to the repository guidelines: "Use English for broad audience; internal/international teams may include Chinese remarks but primary docs should be English."
For better maintainability and consistency with the codebase conventions, consider keeping inline comments in English or ensuring the entire codebase follows the same language convention.
src/command/init.rs
Outdated
| if is_reinit(&cur_dir) { | ||
| if !args.quiet { | ||
| eprintln!("Already initialized - [{}]", root_dir.display()); | ||
| eprintln!("已经初始化 - [{}]", root_dir.display()); | ||
| } | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::AlreadyExists, | ||
| "Initialization failed: The repository is already initialized at the specified location. | ||
| If you wish to reinitialize, please remove the existing directory or file.", | ||
| "初始化失败：指定的位置已经初始化过仓库。如果需要重新初始化，请删除现有的目录或文件。", | ||
| )); | ||
| } |
There was a problem hiding this comment.
The is_reinit check at line 212 uses cur_dir to check if the repository is already initialized, but when --separate-git-dir is used, the actual Git directory is in a different location. The check should verify both locations:
- The working directory (
cur_dir) for a normal repository - The separate git directory if
--separate-git-diris specified
Consider updating the logic to:
// Check if the repository is already initialized in either location
if is_reinit(&cur_dir) || (args.separate_git_dir.is_some() && is_reinit(&root_dir)) {
// ... error handling
}This prevents accidentally reinitializing a repository that exists in the separate git directory.
|
@zhb-348 ，请解决 PR 的冲突问题，所有的代码保持使用英文注释 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| set_hash_kind(match object_format_value.as_str() { | ||
| "sha1" => HashKind::Sha1, | ||
| "sha256" => HashKind::Sha256, | ||
| _ => HashKind::Sha1, | ||
| }); |
There was a problem hiding this comment.
The usage of set_hash_kind and HashKind enum variants references types that don't exist in the current codebase. The import on line 17 is invalid as there is no hash module under src/internal/. This code will fail to compile. The hash functionality should be sourced from the git-internal crate as done in clone.rs (line 12).
|
|
||
| /// Specify a separate directory for Git storage | ||
| #[clap(long = "separate-git-dir", value_name = "PATH", required = false)] | ||
| pub separate_git_dir: Option<String>, |
There was a problem hiding this comment.
The new --separate-git-dir parameter lacks test coverage. The repository has comprehensive integration tests for the init command in tests/command/init_test.rs, but no tests are added for this new functionality. Tests should verify: 1) gitlink file creation in the working directory, 2) repository files created in the separate directory, 3) correct behavior when combined with --bare, 4) proper error handling for invalid paths, and 5) reinit detection when a gitlink already exists.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !args.bare { | ||
| let separate_git_path = Path::new(separate_git_dir); | ||
| let gitlink_path = cur_dir.join(ROOT_DIR); | ||
| let gitlink_content = format!("gitdir: {}", separate_git_path.display()); | ||
| fs::write(gitlink_path, gitlink_content)?; |
There was a problem hiding this comment.
With --separate-git-dir, init now writes a .libra gitlink file in the working tree (lines 266-270) instead of creating a directory, but repository discovery still calls storage_path()/path::objects() expecting .libra to be a directory and never resolves the gitdir: pointer. Any follow-up command (e.g., status, commit) will attempt to open .libra/objects and .libra/libra.db under the working tree and fail with ENOTDIR, so repositories initialized with the new flag cannot be used. The gitlink needs to be read and resolved throughout path resolution before advertising this option.
Useful? React with 👍 / 👎.
| let entry = entry?; | ||
| let file_type = entry.file_type()?; | ||
| let entry = entry?; | ||
| let file_type = entry.file_type()?; |
There was a problem hiding this comment.
Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.
| if !remote_repo.ends_with('/') { | ||
| remote_repo.push('/'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.
|
|
||
| use crate::command::branch; | ||
| use crate::internal::db; | ||
| use git_internal::hash::{HashKind, set_hash_kind}; // 使用 git_internal crate 的 hash 模块 |
There was a problem hiding this comment.
Comment is written in Chinese. According to the repository's custom guidelines, internal/international teams may include Chinese remarks but primary docs should be in English. For consistency and broader accessibility, consider translating this comment to English: "Use the hash module from git_internal crate".
| // Set global hash algorithm | ||
| set_hash_kind(match object_format_value.as_str() { | ||
| "sha1" => HashKind::Sha1, | ||
| "sha256" => HashKind::Sha256, | ||
| _ => HashKind::Sha1, | ||
| }); |
There was a problem hiding this comment.
Setting the global hash kind after repository initialization is problematic for several reasons: 1) In a multi-client, concurrent environment (as per repository guidelines), global state modifications can cause race conditions. 2) The hash kind should be determined at the start of init before any objects are created, not after database initialization. 3) If an error occurs during initialization, the global state may be left in an incorrect state. Consider moving this call to the beginning of the init function, right after determining the object_format_value.
| init_config(&conn, args.bare, Some(object_format_value.as_str())) | ||
| .await | ||
| .unwrap(); |
There was a problem hiding this comment.
Using unwrap() on database operations can cause panics if the database transaction fails. According to the repository guidelines, library code should avoid unwrap()/expect() and properly propagate errors. Consider replacing these with proper error handling that returns a Result and uses ? for error propagation.
| reference::ActiveModel { | ||
| name: Set(Some(initial_branch_name.clone())), | ||
| kind: Set(reference::ConfigKind::Head), | ||
| ..Default::default() // all others are `NotSet` | ||
| ..Default::default() | ||
| } | ||
| .insert(&conn) | ||
| .await | ||
| .unwrap(); |
There was a problem hiding this comment.
Using unwrap() on database operations can cause panics if the insert fails. According to the repository guidelines, library code should avoid unwrap()/expect() and properly propagate errors. Consider replacing this with proper error handling that returns a Result and uses ? for error propagation.
| // When using --separate-git-dir, create a `.libra` gitlink file in the working directory | ||
| // that points to the actual Libra storage directory. This must use `ROOT_DIR` so that | ||
| // repository discovery remains consistent with the rest of the codebase. | ||
| if let Some(separate_git_dir) = &args.separate_git_dir { | ||
| if !args.bare { | ||
| let separate_git_path = Path::new(separate_git_dir); | ||
| let gitlink_path = cur_dir.join(ROOT_DIR); | ||
| let gitlink_content = format!("gitdir: {}", separate_git_path.display()); | ||
| fs::write(gitlink_path, gitlink_content)?; |
There was a problem hiding this comment.
The implementation creates a gitlink file at .libra, but the existing repository discovery logic in try_get_storage_path (src/utils/util.rs:40-55) only checks if .libra exists as a directory. It does not handle reading gitlink files to resolve the actual storage location. This means repositories initialized with --separate-git-dir will not be discoverable by other commands. The try_get_storage_path function needs to be updated to read and follow gitlink files before this feature can work correctly.
| /// Check if the repository has already been initialized based on the presence of the description file. | ||
| fn is_reinit(cur_dir: &Path) -> bool { | ||
| let bare_head_path = cur_dir.join("description"); | ||
| let head_path = cur_dir.join(".libra/description"); |
There was a problem hiding this comment.
The is_reinit function checks for hardcoded .libra/description instead of using ROOT_DIR. For consistency with the rest of the codebase and to support potential future changes to ROOT_DIR, use cur_dir.join(ROOT_DIR).join("description") instead of the hardcoded string.
| let head_path = cur_dir.join(".libra/description"); | |
| let head_path = cur_dir.join(ROOT_DIR).join("description"); |
| for entry in fs::read_dir(src)? { | ||
| let entry = entry?; | ||
| let file_type = entry.file_type()?; | ||
| let entry = entry?; |
There was a problem hiding this comment.
Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.
| if let Ok(_bits) = u32::from_str_radix(&mode[1..], 8) {} | ||
| else { |
There was a problem hiding this comment.
The else keyword should be on the same line as the closing brace of the preceding if block, following Rust formatting conventions. It should be } else { on line 170.
| if let Ok(_bits) = u32::from_str_radix(&mode[1..], 8) {} | |
| else { | |
| if let Ok(_bits) = u32::from_str_radix(&mode[1..], 8) {} else { |
2 participants
39
在 InitArgs 结构体中添加 --separate-git-dir 参数
修改 init 函数以处理 --separate-git-dir 参数