fix: Correctly detect format when using md5sum with check flag #4645
Conversation
|
hi |
I'll read it as yes then :) Sure thing, added. |
|
Windows is failing with: |
|
Hmm, |
you should look at other examples :) |
|
I tried with no luck, thus my question. I'm not very familiar with the historical choices of this repository just yet. |
|
ok, just rename the file from the example. It is still dumb on linux to have * in a file name :) |
|
It is possible though, so used conditional assertions, just to be inline with gnu tests on unix |
|
We have some differences with GNU: |
|
@kamilogorek ping ? :) |
|
Ah sorry, totally missed your comment, thanks for the ping. Fixed 2nd issue (debug representation is escaped for |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
| // BSD reversed mode format is similar to the default mode, but doesn’t use a character to distinguish binary and text modes. | ||
| let mut bsd_reversed = None; | ||
|
|
||
| fn gnu_re_template( |
There was a problem hiding this comment.
could you please add a rustdoc comment here? thanks
src/uu/hashsum/src/hashsum.rs
Outdated
| caps.name("binary").unwrap().as_str() == "*", | ||
| ), | ||
| Some(caps) => { | ||
| // We set the format during first pass, as mixing of formats is disallowed. |
There was a problem hiding this comment.
maybe move this into a function like
fn handle_captures(caps: Captures, bytes_marker: &str, bsd_reversed: &mut Option<bool>) -> Result<(String, String, bool), HashsumError> {
if bsd_reversed.is_none() {
let is_bsd_reversed = caps.name("binary").is_none();
let format_marker = if is_bsd_reversed {
""
} else {
r"(?P<binary>[ \*])"
}
.to_string();
*bsd_reversed = Some(is_bsd_reversed);
gnu_re = gnu_re_template(&bytes_marker, &format_marker)?;
}
Ok((
caps.name("fileName").unwrap().as_str().to_string(),
caps.name("digest").unwrap().as_str().to_ascii_lowercase(),
if *bsd_reversed == Some(false) {
caps.name("binary").unwrap().as_str() == "*"
} else {
false
},
))
}
and call it with
Some(caps) => handle_captures(caps, &bytes_marker, &mut bsd_reversed)?,
src/uu/hashsum/src/hashsum.rs
Outdated
| @@ -629,6 +634,36 @@ where | |||
| )) | |||
| .map_err(|_| HashsumError::InvalidRegex)?; | |||
|
|
|||
| fn handle_captures( | |||
| caps: Captures, | |||
There was a problem hiding this comment.
could you please fix the clippy warnings ? (sorry)
|
Ah, forgot that warnings are failures in CI and missed that locally. Updated |
|
thanks :) |
The first contribution to this repository, so hello there!
Looking at how sparse
/tests/by-util/test_hashsum.rstests are, I wasn't sure if I should add more of them there, or do you rely solely on GNU compatibility tests in this case.Fixes #4611
The main issue wasn't not handling
checkflag, but rather regexp being too greedy and treatingband*casbinary|textmarkers, where first entry ofashould dictate that the format is BSD-reverse, which should skip reading the marker entirely for the remaining of the file.References: