Page MenuHomePhabricator

dirstate-v2: Document flags/mode/size/mtime fields of tree nodes
ClosedPublic

Authored by SimonSapin on Mon, Oct 11, 4:31 PM.

Details

Summary

This file format modification was previously left incomplete because of
planned upcoming changes. Not all of these changes have been made yet,
but documenting what exists today will help talking more widely about it.

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

SimonSapin created this revision.Mon, Oct 11, 4:31 PM

This seems fine, but I couldn't import it without the previous commit, and I'd like someone who knows Rust to handle that.

mercurial/helptext/internals/dirstate-v2.txt
405

s/called/call/

424

Are the other bits being masked out? There's a dirstate propose on the wiki to track the u+x bit on filesystems that don't support that, but it required setting and checking a bit for a FIFO or something exotic that Mercurial wouldn't handle anyway. If we're starting fresh, it would be nice to avoid the hack.

SimonSapin added inline comments.Tue, Oct 12, 7:14 AM
mercurial/helptext/internals/dirstate-v2.txt
424

At the moment all 32 bits are whatever we got from stat_result.st_mode, same as in dirstate-v1. But the status algorithm only looks at the exec bit.

By the way maybe status should also look at the file type, in case a normal file became a symlink or vice versa. But last I tried I didn’t manage to craft a test case that does this without changing the mtime, since touch -d follows symlinks.

I’m considering removing this mode field in dirstate-v2 and replacing it with a single HAS_EXEC_PERMISSION bit (plus maybe an IS_SYMLINK bit), but that’s not for this patch which document the current format.

SimonSapin updated this revision to Diff 30711.Tue, Oct 12, 7:25 AM
SimonSapin updated this revision to Diff 30712.Tue, Oct 12, 7:49 AM

I couldn't import it without the previous commit, and I'd like someone who knows Rust to handle that.

To avoid this review dependency I’ve inverted the order of these two commits. This one now documents the previous format, and the commit that changes the format then also updates the documentation.

mharbison72 accepted this revision.Tue, Oct 12, 12:17 PM

LGTM, but I still get an import error for this. Since there are a bunch of accepted things not pushed yet, maybe this has a dependency on one of those things.

$ hg phabimport D11625
applying patch from D11625
patching file rust/hg-core/src/dirstate_tree/on_disk.rs
Hunk #2 FAILED at 93
abort: patch failed to apply
mercurial/helptext/internals/dirstate-v2.txt
424

+1 for doing that before freezing the format. (I think the wiki page also had bits to indicate if each was emulated, but I don't have time to find it now.)

456

/dependening/depending/

This revision is now accepted and ready to land.Tue, Oct 12, 12:17 PM

Is it on top of hg-committed / 38deb65d4441 that this does not apply cleanly? If so this is because because D11623 hasn’t been pushed to hg-commited yet. If you can fix that that should solve this import.

martinvonz added a subscriber: martinvonz.

I changed this one's parent to be D11623 since D11624 was abandoned. I hope that's how you meant it to be.

Yes that’s what I meant. Thanks!