BurntSushi / walkdir

Rust library for walking directories recursively.
The Unlicense
1.22k stars 107 forks source link

`DirEntry`s `FileType` can have different equality properties depending on code path #93

Closed epage closed 6 years ago

epage commented 6 years ago

I'm trying to create a form of zipper walkdir iterators in my revision to dir-diff. In the original code path, we just did quality checks on the FileType and everything worked. When I manually look up the FileType, my tests fail.

---- easy_good stdout ----
FileType(FileType { mode: 32768 }) != FileType(FileType { mode: 33206 })
true != true
false != false
false != false

32768 -> 0x8000 33206 -> 0x81B6 (seems like Debug for FileType should output hex)

It looks like the extra information is S_IMODE

>>> hex(stat.S_IMODE(a.st_mode))
'0x1b6'

I've narrowed it down to read_dir vs metadata.

The reason I bring this up here is that DirEntry seems to have a way to populate its ty from either fs::DirEntry or metadata, I'm just not hitting that case in my code. This could cause some surprising behavior for walkdir users.

BurntSushi commented 6 years ago

@epage This is interesting. Could you say more about what problem you're trying to solve by comparing FileType for equality? The way in which things might differ here are symbolic links, which might point to real differences rather than artificial ones. For example, for walkdir::DirEntry, if follow_links was enabled on the originating iterator, then all operations are performed on the link target. Otherwise, they are performed on the link itself.

epage commented 6 years ago

The way in which things might differ here are symbolic links, which might point to real differences rather than artificial ones.

Are you referring to the nature of the symlinks vs not or is from_link only being used for symbolic links and from_entry for everything else? (Hey, that makes the name make more sense).

I'm implementing a diffing iterator for dir-diff. I'm implementing this in a naive way.

I've modified my code to not use the file_type returned by WalkDir. I just find this surprising that hidden data is impacting the behavior of FileType but I doubt there is a good solution to this.

BurntSushi commented 6 years ago

Yeah I'm not sure what is to be done here. If anything, I'm surprised that Eq is implemented at all for FileType, and I don't think I would use it for directory diffing (other than doing comparisons based on the results of FileType's method). I'm going to close this for now, but if you learn more we can re-open! Thanks for the report!

epage commented 6 years ago

Raising awareness was my main concern.

Interesting idea to not implement Eq (deprecation being the only option at this point). The annoying thing is its just valuable enough. It works alright if you use it from a consistent source but not if you mix the sources.