This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10217

rust-status: fix issue6456 for the Rust implementation also
ClosedPublic

Authored by Alphare on Mar 15 2021, 10:06 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG3c9ddb1986a9: rust-status: fix issue6456 for the Rust implementation also
Summary

This implementation is being used by a few people now, so we need it fixed, even
though a big rewrite will be coming quite soon.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Mar 15 2021, 10:06 AM
Alphare added a commit: rHG3c9ddb1986a9: rust-status: fix issue6456 for the Rust implementation also.Mar 16 2021, 5:49 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG3c9ddb1986a9: rust-status: fix issue6456 for the Rust implementation also (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.Oct 1 2021, 12:04 PM
martinvonz added inline comments.
rust/hg-core/src/utils/files.rs
206

Clippy pointed out that this will never be true :) I suppose the left left value should also be 0120000, but I'll let you decide if it's worth fixing it (given the comment above)

Alphare added inline comments.Oct 1 2021, 12:09 PM
rust/hg-core/src/utils/files.rs
206

Ha, indeed. Well, this does not need to exist anymore since it isn't used since D11516, it appears this slipped through the "dead code" pass.

martinvonz added inline comments.Oct 1 2021, 12:14 PM
rust/hg-core/src/utils/files.rs
206

Hmm, I suppose the problem is actually that the numbers are meant to be octal (e.g. 0o120000, I think). Doesn't matter if it's going away anyway.

Alphare added inline comments.Oct 1 2021, 12:17 PM
rust/hg-core/src/utils/files.rs
206

See D11549

Revision Contents
Changeset List

Diff 26438

rust/hg-core/src/dirstate/status.rs

(mode ^ st_mode as i32) & 0o100 != 0o000 && options.check_exec; (mode ^ st_mode as i32) & 0o100 != 0o000 && options.check_exec;
let metadata_changed = size >= 0 && (size_changed || mode_changed); let metadata_changed = size >= 0 && (size_changed || mode_changed);
let other_parent = size == SIZE_FROM_OTHER_PARENT; let other_parent = size == SIZE_FROM_OTHER_PARENT;
if metadata_changed if metadata_changed
|| other_parent || other_parent
|| copy_map.contains_key(filename.as_ref()) || copy_map.contains_key(filename.as_ref())
{ {
if metadata.is_symlink() && size_changed {
// issue6456: Size returned may be longer due to encryption
// on EXT-4 fscrypt. TODO maybe only do it on EXT4?
Dispatch::Unsure
} else {
Dispatch::Modified Dispatch::Modified
}
} else if mod_compare(mtime, st_mtime as i32) } else if mod_compare(mtime, st_mtime as i32)
|| st_mtime == options.last_normal_time || st_mtime == options.last_normal_time
{ {
// the file may have just been marked as normal and // the file may have just been marked as normal and
// it may have changed in the same second without // it may have changed in the same second without
// changing its size. This can happen if we quickly // changing its size. This can happen if we quickly
// do multiple commits. Force lookup, so we don't // do multiple commits. Force lookup, so we don't
// miss such a racy file change. // miss such a racy file change.

rust/hg-core/src/utils/files.rs

st_dev: metadata.dev(), st_dev: metadata.dev(),
st_mode: metadata.mode(), st_mode: metadata.mode(),
st_nlink: metadata.nlink(), st_nlink: metadata.nlink(),
st_size: metadata.size(), st_size: metadata.size(),
st_mtime: metadata.mtime(), st_mtime: metadata.mtime(),
st_ctime: metadata.ctime(), st_ctime: metadata.ctime(),
} }
} }
pub fn is_symlink(&self) -> bool {
// This is way too manual, but `HgMetadata` will go away in the
// near-future dirstate rewrite anyway.
self.st_mode & 0170000 == 0120000
martinvonzUnsubmitted
Not Done

Clippy pointed out that this will never be true :) I suppose the left left value should also be 0120000, but I'll let you decide if it's worth fixing it (given the comment above)

martinvonz: Clippy pointed out that this will never be true :) I suppose the left left value should also be…
AlphareAuthorUnsubmitted
Done

Ha, indeed. Well, this does not need to exist anymore since it isn't used since D11516, it appears this slipped through the "dead code" pass.

Alphare: Ha, indeed. Well, this does not need to exist anymore since it isn't used since D11516, it…
martinvonzUnsubmitted
Not Done

Hmm, I suppose the problem is actually that the numbers are meant to be octal (e.g. 0o120000, I think). Doesn't matter if it's going away anyway.

martinvonz: Hmm, I suppose the problem is actually that the numbers are meant to be octal (e.g. `0o120000`…
AlphareAuthorUnsubmitted
Done

See D11549

Alphare: See D11549
}
} }
/// Returns the canonical path of `name`, given `cwd` and `root` /// Returns the canonical path of `name`, given `cwd` and `root`
pub fn canonical_path( pub fn canonical_path(
root: impl AsRef<Path>, root: impl AsRef<Path>,
cwd: impl AsRef<Path>, cwd: impl AsRef<Path>,
name: impl AsRef<Path>, name: impl AsRef<Path>,
) -> Result<PathBuf, HgPathError> { ) -> Result<PathBuf, HgPathError> {