-
Notifications
You must be signed in to change notification settings - Fork 85
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
fix: filename too long when use pnpm #1421
Conversation
Walkthrough本次更新引入了对文件名过长的处理,通过使用 Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- crates/mako/src/generate/generate_chunks.rs (3 hunks)
- e2e/fixtures/os.file_name_too_long/expect.js (1 hunks)
- e2e/fixtures/os.file_name_too_long/mako.config.json (1 hunks)
- e2e/fixtures/os.file_name_too_long/src/abcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujs.js (1 hunks)
- e2e/fixtures/os.file_name_too_long/src/index.ts (1 hunks)
Additional comments not posted (5)
e2e/fixtures/os.file_name_too_long/mako.config.json (1)
1-4
: 配置文件审查通过配置文件中的设置符合项目标准,没有发现问题。
e2e/fixtures/os.file_name_too_long/expect.js (1)
1-7
: 测试代码审查通过,但建议增加更全面的测试测试逻辑正确,但建议确保覆盖所有相关的测试场景,以防止未来的回归。
e2e/fixtures/os.file_name_too_long/src/index.ts (1)
1-5
: 动态导入语法正确,但请注意潜在的文件名长度问题动态导入的语法没有问题,但由于文件名过长,建议在实际项目中避免使用如此长的文件名,以免引起兼容性或性能问题。
crates/mako/src/generate/generate_chunks.rs (1)
2-2
: 导入语句审查通过。已正确添加
std::hash::Hasher
和twox_hash::XxHash64
,用于处理文件名过长的情况。Also applies to: 14-14
e2e/fixtures/os.file_name_too_long/src/abcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujsdfjsdifjsdfjsdfabcdefghigklmnupqlmfdfsdfjsdfijsdfiujs.js (1)
1-1
: 代码审查文件中的代码非常简单,仅包含一个导出常量的语句。这个文件的主要目的可能是用于测试系统处理超长文件名的能力。
// fixed os error 63 file name too long, reserve 16 bytes for .map and others | ||
let reserve_file_name_length = 239; | ||
if let Some(hash) = &self.hash { | ||
hash_file_name(&self.file_name, hash) | ||
} else { | ||
if self.file_name.len() > reserve_file_name_length { | ||
let mut hasher: XxHash64 = Default::default(); | ||
hasher.write_str(self.file_name.as_str()); | ||
return format!( | ||
"{}.{}", | ||
&hasher.finish().to_string()[11..], | ||
&self.file_name[self.file_name.len() - reserve_file_name_length..] | ||
); | ||
} |
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.
长文件名处理逻辑审查。
此方法通过 XxHash64
对过长的文件名进行哈希处理,确保文件名长度不会超出操作系统限制。请注意,哈希后的文件名与原始文件名的后缀组合可能需要进一步验证以确保唯一性。
- "{}.{}",
- &hasher.finish().to_string()[11..],
- &self.file_name[self.file_name.len() - reserve_file_name_length..]
+ "{}-{}.{}",
+ &hasher.finish().to_string()[..8],
+ &self.file_name[self.file_name.len() - reserve_file_name_length..],
+ &self.file_name[self.file_name.len() - 3..]
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// fixed os error 63 file name too long, reserve 16 bytes for .map and others | |
let reserve_file_name_length = 239; | |
if let Some(hash) = &self.hash { | |
hash_file_name(&self.file_name, hash) | |
} else { | |
if self.file_name.len() > reserve_file_name_length { | |
let mut hasher: XxHash64 = Default::default(); | |
hasher.write_str(self.file_name.as_str()); | |
return format!( | |
"{}.{}", | |
&hasher.finish().to_string()[11..], | |
&self.file_name[self.file_name.len() - reserve_file_name_length..] | |
); | |
} | |
// fixed os error 63 file name too long, reserve 16 bytes for .map and others | |
let reserve_file_name_length = 239; | |
if let Some(hash) = &self.hash { | |
hash_file_name(&self.file_name, hash) | |
} else { | |
if self.file_name.len() > reserve_file_name_length { | |
let mut hasher: XxHash64 = Default::default(); | |
hasher.write_str(self.file_name.as_str()); | |
return format!( | |
"{}-{}.{}", | |
&hasher.finish().to_string()[..8], | |
&self.file_name[self.file_name.len() - reserve_file_name_length..], | |
&self.file_name[self.file_name.len() - 3..] | |
); | |
} |
"{}.{}", | ||
&hasher.finish().to_string()[11..], | ||
&self.file_name[self.file_name.len() - reserve_file_name_length..] |
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.
1、为啥是 hash.part_file_name
?很少看到 hash 在前面的。建议用 part_file_name.hash.ext
2、为啥是 11?
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/generate/generate_chunks.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/generate/generate_chunks.rs
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/generate/generate_chunks.rs (3 hunks)
Additional context used
GitHub Check: Spell Check
crates/mako/src/generate/generate_chunks.rs
[warning] 43-43:
"extention" should be "extension".
[warning] 52-52:
"extention" should be "extension".
[warning] 60-60:
"extention" should be "extension".
Additional comments not posted (4)
crates/mako/src/generate/generate_chunks.rs (4)
Line range hint
2-14
:
导入XxHash64
看起来不错!导入
XxHash64
是为了新的哈希逻辑,符合预期。
43-44
: 请确认reserve_file_name_length
的值。
reserve_file_name_length
的值为 207,请确认这个值是否合适。Tools
GitHub Check: Spell Check
[warning] 43-43:
"extention" should be "extension".
50-51
: 哈希逻辑看起来不错!使用
XxHash64
对文件名进行哈希处理是合理的。
55-61
: 字符串格式化逻辑看起来不错,但请验证文件名的唯一性。字符串格式化逻辑将哈希后的文件名与原始文件扩展名结合,确保生成的文件名不会过长。请验证文件名的唯一性。
Tools
GitHub Check: Spell Check
[warning] 60-60:
"extention" should be "extension".
let file_extention = file_path.extension().unwrap(); | ||
let file_stem = file_path.file_stem().unwrap().to_string_lossy().to_string(); | ||
let (_, reserve_file_path) = | ||
file_stem.split_at(file_stem.len() - reserve_file_name_length); | ||
return format!( | ||
"{}.{}.{}", | ||
reserve_file_path, | ||
&hasher.finish().to_string()[0..8], | ||
file_extention.to_str().unwrap() |
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.
修正拼写错误。
单词 "extention" 应该拼写为 "extension"。
- let file_extention = file_path.extension().unwrap();
+ let file_extension = file_path.extension().unwrap();
- "{}.{}.{}",
+ "{}-{}.{}",
- &file_extention.to_str().unwrap()
+ &file_extension.to_str().unwrap()
Tools
GitHub Check: Spell Check
[warning] 52-52:
"extention" should be "extension".
[warning] 60-60:
"extention" should be "extension".
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/generate/generate_chunks.rs (3 hunks)
Additional comments not posted (3)
crates/mako/src/generate/generate_chunks.rs (3)
14-14
: 引入XxHash64
哈希算法。引入
twox_hash
crate 以支持新的哈希功能。
43-44
: 添加常量reserve_file_name_length
。该常量用于确定文件名需要截断的长度,确保最终文件名不超过207个字符。
49-62
: 处理文件名过长的逻辑。当文件名长度超过
reserve_file_name_length
时,使用XxHash64
哈希算法生成文件名哈希,并将其与截断后的文件名和文件扩展名组合。
@@ -38,9 +40,26 @@ pub struct ChunkFile { | |||
|
|||
impl ChunkFile { | |||
pub fn disk_name(&self) -> String { | |||
// fixed os error 63 file name too long, reserve 48 bytes for _js-async、extension、.map and others | |||
let reserve_file_name_length = 207; | |||
let file_path = Path::new(&self.file_name); | |||
if let Some(hash) = &self.hash { | |||
hash_file_name(&self.file_name, hash) |
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.
hash 开启的情况下 源文件路径也可能会超的
判断长度的逻辑要提到最外层
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: 2
Outside diff range, codebase verification and nitpick comments (7)
crates/mako/src/generate/generate_chunks.rs (7)
2-2
: 建议:优化导入顺序。将
use std::hash::Hasher;
移动到标准库导入部分的末尾,以保持导入顺序的一致性。可以考虑如下调整:
use std::collections::HashMap; +use std::hash::Hasher; use std::path::Path; use std::sync::Arc;
14-14
: 建议:优化导入顺序。将
use twox_hash::XxHash64;
移动到外部库导入部分的末尾,以保持导入顺序的一致性。可以考虑如下调整:
use tracing::warn; +use twox_hash::XxHash64;
43-44
: 建议:添加注释说明。在
reserve_file_name_length
变量声明处添加注释,解释为什么选择 207 作为保留长度。可以考虑如下调整:
// fixed os error 63 file name too long, reserve 48 bytes for _js-async、extension、.map and others +// 207 是为了确保文件名长度不会超过操作系统限制 let reserve_file_name_length = 207;
46-46
: 建议:优化变量命名。
format_file_name
可以改为formatted_file_name
,以更好地表达其含义。可以考虑如下调整:
let mut format_file_name = self.file_name.clone(); +let mut formatted_file_name = self.file_name.clone();
48-49
: 建议:优化哈希处理逻辑。在
hasher.write_str(self.file_name.as_str());
之前添加注释,解释哈希处理的目的。可以考虑如下调整:
if self.file_name.len() > reserve_file_name_length { + // 使用 XxHash64 对文件名进行哈希处理,确保唯一性 let mut hasher: XxHash64 = Default::default(); hasher.write_str(self.file_name.as_str());
53-53
: 建议:优化变量命名。
reserve_file_path
可以改为reserved_file_path
,以更好地表达其含义。可以考虑如下调整:
let (_, reserve_file_path) = +let (_, reserved_file_path) = file_stem.split_at(file_stem.len() - reserve_file_name_length);
55-58
: 建议:优化文件名格式。将文件名格式从
"{}.{}.{}"
改为"{}-{}.{}"
,以更好地分隔文件名和哈希值。可以考虑如下调整:
format_file_name = format!( - "{}.{}.{}", + "{}-{}.{}", reserve_file_path, &hasher.finish().to_string()[0..8], file_extension.to_str().unwrap() );
let file_extension = file_path.extension().unwrap(); | ||
let file_stem = file_path.file_stem().unwrap().to_string_lossy().to_string(); |
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.
建议:处理文件扩展名为空的情况。
在 file_path.extension().unwrap();
之前添加判断,处理文件扩展名为空的情况。
可以考虑如下调整:
let file_extension = file_path.extension().unwrap();
+let file_extension = file_path.extension().unwrap_or_default();
Committable suggestion was skipped due to low confidence.
|
||
if let Some(hash) = &self.hash { | ||
hash_file_name(&self.file_name, hash) | ||
hash_file_name(&format_file_name, hash) | ||
} else { | ||
self.file_name.clone() | ||
format_file_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.
建议:优化文件名生成逻辑。
在 disk_name
方法中,考虑将 hash_file_name
和 format_file_name
的逻辑分开,以提高代码的可读性和维护性。
可以考虑如下调整:
if let Some(hash) = &self.hash {
- hash_file_name(&format_file_name, hash)
+ return hash_file_name(&formatted_file_name, hash);
}
- format_file_name
+formatted_file_name
Committable suggestion was skipped due to low confidence.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1421 +/- ##
==========================================
- Coverage 61.57% 60.75% -0.82%
==========================================
Files 29 126 +97
Lines 2363 15042 +12679
==========================================
+ Hits 1455 9139 +7684
- Misses 908 5903 +4995 ☔ View full report in Codecov by Sentry. |
when using pnpm
filename length will oversize of os limit 255
this will make os error 63
Summary by CodeRabbit
Summary by CodeRabbit
XxHash64
进行哈希并截断文件名,确保文件名格式正确,最长保留207个字符。mako.config.json
用于设置模块ID策略和开发工具。