I think we can improve Mac OS X local file equality testing. Specifically, I don't see why we need to compare FSRef structures which we always need to construct and are never going to use as a path basis in local files. We should just compare CFURLs or native paths.
This just compares native paths. That is almost certainly always going to be a valid path format for comparison, meaning whether we continue to have CFURL or something else (like a native path?) as the basis for our local file store we can always convert to a native path pretty easily.
Attachment #374828 - Flags: review?(mstange)
Attachment #374828 - Flags: review?(mstange) → review+
Attachment #374828 - Flags: superreview?(roc) → superreview+
pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/cb8262ea787f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
backed out, unit test orange http://hg.mozilla.org/mozilla-central/rev/9170bcabf752 I don't understand this error: TestRegular FAILED - cannot create core service TestDeferred FAILED - cannot create core service Finished running RegistrationOrder tests. make: *** [check] Error 1 make: *** [check] Error 2 make: *** [check] Error 2 program finished with exit code 2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The test is failing because my patch fails to report that the following two paths are equal: /Users/josh/src/mozilla/ff_192_debug/objdir-debug/dist/bin/components /Users/josh/src/mozilla/ff_192_debug/objdir-debug/xpcom/tests/../../dist/bin/components Without my patch these are reported as equal because both directories actually exist and we create an FSRef for each and compare, essentially comparing inodes and not paths. The files shouldn't have to exist in order for us to compare as far as I can tell, and the UNIX implementation actually does what my first (failed) patch here does and gets away with it. I can only assume that when using the UNIX implementation the paths it is asked to compare aren't the same as the ones I pasted above - they are either both normalized or both non-normalized. I'll test when I have access to Linux again.
Fixed by the patch in bug 491245.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Depends on: 491245
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.