nsIFile::moveTo not implemented consistently across platforms. WIN32 and mach-o implementations modify the nsLocalFile object to point at the new file location instead of the old file location. UNIX implementation preserves what the nsLocalFile was previously point at. IMO, the UNIX implementation is much more consistent with what you'd expect from something that represents a file path. of course, changing the behavior of this code is probably going to cause bugs :-( bug 198947 is a symptom of this problem. the bigger problem being that nsIFile and nsILocalFile are poorly documented.

198947 can be fixed independently of this bug and edt would like to avoid the risk this one injects; minusing.

Um.. This is topembed because it makes life hell for embeddors, not because it breaks our installer. The fact that the installer can hack around it is irrelevant to whether this bug is topembed or not! Not like that's going to change the EDT's mind, of course....

> the UNIX implementation is much more consistent with what you'd expect from > something that represents a file path. I thought that it represented a file, not a file path. After all, the interface is called nsIFile, not nsIFilePath.

ccarlen: well, SetLeafName does not rename the file, right? there are two sets of functions.. those that affect the file path and those that affect the underlying file itself. where we draw the line is not well defined.

so, i tried this little code snipet to test out the behavior of Java's File.renameTo method (nsIFile is based on Java's File class). import java.io.* ; class FileTest { public static void main(String[] args) { File f = new File("a"); System.out.println(f.getPath()); f.renameTo(new File("b")); System.out.println(f.getPath()); } } if i had a file named "a" in the current directory, then this program would rename the file as "b". the program prints out "a" twice, indicating that Java's renameTo method does not change the file path referenced by the File object even though renameTo clearly makes |f| reference a file that no longer exists.

The nsIFile iface doesn't mention Java's File class, so if I'm using nsIFile, how would I know to look to Java as the model? I didn't know there was any connection until you pointed it out. Win32, Mach-O, and OS/2 all update the file on moving it - making Unix the sole exception. Not just to say it's easier to change 1 impl instead of 3, but independent programmers made those 4 implementations and 3 out of 4 expected the file to be updated on the move. If 3/4 people who use the iface expect that as well, seems natural to me.

> but independent programmers made those 4 implementations OS/2 was a copy/paste of the Win32 impl to start with. Hardly independent. No sure how exactly OS/X was done, but based on some other things I've seen in that code, it also borrowed heavily from the win32 code.

conrad: doug said he modeled nsIFile after Java's File class. while true, it doesn't mean that we are in any way bound to following Java's implementation, i just thought it was interesting to see how they interpreted virtually the same problem.

The nsIFile is an abstraction for the logical file -- not an abstaction of the path. If we go down the road of making nsIFile just represent the path, then we had better stop caching the stat info. (More over, why not just use a const char* and NSPR?) The nsIFile is a stateful object representing the file on disk. If I move the file on disk, i would expect that nsIFile object to continue to reference the same file. i am with conrad. update the file abstraction after a move fix the IDL to reflect this soon-to-be consistent behavior.

doug: what about comment #4? nsIFile represents exactly a file path in some cases and exactly a file in others. if nsIFile always represents a file, then why doesn't SetLeafName move the underlying file?

Darin, as you described in comment #4, we need a set of methods for constructing an nsIFile object, and a separate set of methods for operating on the object. We obviously need more clarity between the two. Can we do it with more documentation and fixing the various implementations to be more consistent, or do we need another interface?

so, i've surveyed consumers of MoveTo/MoveToNative. most don't care about the nsIFile object after MoveTo is called. some do, however. here's an example of one that does: nsCOMPtr<nsIFile> parentDir; rv = mCacheDirectory->GetParent(getter_AddRefs(parentDir)); if (NS_FAILED(rv)) return rv; rv = mCacheDirectory->MoveToNative(uniqueDir, nsCString()); if (NS_FAILED(rv)) return rv; // set mCacheDirectory to point to parentDir/Cache/ again rv = parentDir->AppendNative(NS_LITERAL_CSTRING("Cache")); if (NS_FAILED(rv)) return rv; mCacheDirectory = do_QueryInterface(parentDir); notice that if MoveToNative didn't change the state of mCacheDirectory, then this could have been move compactly written like so: rv = mCacheDirectory->MoveToNative(uniqueDir, nsCString()); if (NS_FAILED(rv)) return rv; i also found examples in nsAbLDAPReplicationData.cpp, in which it calls SetLeafName after calling MoveToNative in order to ensure that the state of the file object is indeed changed to point to MoveToNative. in that case, it is one extra line of code: rv = mReplicationData->MoveTo(nsnull, newName); if (NS_SUCCEEDED(rv)) rv = mReplicationData->SetLeafName(newName); currently, on some platforms this is redundant. if we make the change i'm proposing than this code would be required. notice that it is not a lot of code either compared to the cache example. if you think about it, MoveTo currently does two things A) move the underlying file, and B) change what the nsIFile points at. some consumers are happy with MoveTo doing A+B, but other consumers only want step A. so, they must manually do A+B-B. so, i'm saying that i think the WIN32/OSX impls have factored things in a less favorable way.

dougt: also, is it really valid for MoveTo to preserve the cached stat result? if MoveTo crosses filesystem boundaries, then the result of stat'ing the file could be different, right? i guess, in the majority of cases, MoveTo could rightly preserve the cached stat result. but, given that few places in the code care maybe this isn't a valuable optimization? gordon: i don't think major interfaces changes are really an option. this is a frozen interface, and i think it is sufficient modulo implementation inconsistencies. sure, it would be nice to have a clearer separation between methods which affect only the path and methods which affect only (or also) the underlying file.

Yes, I realize nsIFile is frozen, which is why (as a future direction) I was wondering if we needed to transition to a *new* interface. My hope though is we can deal with the current problem by making the implementations consistent and clarifying the documentation. I think this discussion is a good start on clarification :-)

not another interface! The problem that darin describes is minor and can be resolved by properly documenting what our behavior is. >if nsIFile always represents a file, then why doesn't SetLeafName move the underlying file? apples and oranges. on the nsIFile exists methods for pointing to a file on disk (setLeafName, append) and methods for modification/acces of the file (moveTo). (at a couple points in history we proposed to break this interface into to pieces) Since implementation are expected to delay stat, methods like append, setLeafName, InitWithPath, simply do path construction. plus we have MoveTo(nsnull, new-name). I think that the developers that need to hold onto the origanal file location can simply make a clone of the nsIFile before the MoveTo call.

> also, is it really valid for MoveTo to preserve the cached stat result? no, of course not. >if MoveTo crosses filesystem boundaries, then the result of stat'ing the file could be different, right? yup >i guess, in the majority of cases, MoveTo could rightly preserve the cached stat result. but, given that few places in the code care maybe this isn't a valuable optimization? I don't think anyone is asking for that -- we don't really are about the optimization, but rather that the nsIFile object which is being moved continues to point at the resulting file which was just moved.

One question. Say I have an nsIFile object called |file|, ok? What is the expected result of the following three sequences of calls? (Each time |file| starts out with the leafName "file"). 1) file.moveTo(nsnull, "newName"); 2) file.copyTo(nsnull, "newName"); 3) file.copyTo(nsnull, "newName"); file.remove();

1) file.moveTo(nsnull, "newName"); file points to a "newName" in the same directory as the inital directory. 2) file.copyTo(nsnull, "newName"); failure - file already exists. 3) file.copyTo(nsnull, "newName"); file.remove(); Failure - file already exists. newName is deleted from the file system; file now doesn't have any valid stat info, but still points to the location of where "newName" was.

Wow! I just had two collision in a row on this bug. >I think that the developers that need to hold onto the origanal file location can simply make a clone of the nsIFile before the MoveTo call. That doesn't currently work on Mac because of the way FSSpecs/FSRefs/File IDs work, which the existing implementation depends on. Cloning an nsIFile and moving the original results on the clone pointing to the new location on Mac.

re: comment #18, huh? I don't think that's what Boris meant. I think he meant each of those three examples as separate test cases, not that they would be executed in order. He states that 'Each time |file| starts out with the leafName "file"', so #3 should result in "file" being deleted, not "newFile", because CopyTo() doesn't change what the nsIFile is pointing at...right?

sorry about that. Assuming no errors: 1) file.moveTo(nsnull, "newName"); file points to "newFile". The file "file" has been moved in the file system to "newName" 2) file.copyTo(nsnull, "newName"); file still points to "file" and there exists a new file in the same directory as "file" named "newName" 3) file.copyTo(nsnull, "newName"); file.remove(); file still points to "file" and there exists a new file in the same directory as "file" named "newName". Lastly, "file" is removed from the file system. This is the same as (2) although not as safe.

OK. So just checking that we're ok with moveTo and copyTo having different semantics as far as whether the nsIFile points to the previous or new location after the operation....

bz: that was my other point. i don't think they should differ. it just seems awkward for those to differ.

CopyTo -- meaning: make a copy of this file and put it there. MoveTo -- meaning: move this file there.

CopyTo: make a copy of the file named by this path and put it "there". MoveTo: move the file named by this path to "there". If nsIFile really represents a path -- and you can't do all the usual "file" things on it, like write or read, but you can do all the stuff you usually do with a path, including point to non-existent file -- then I don't think it should change when we MoveTo.

> Cloning an nsIFile and moving the original results on the clone pointing to the new location on Mac. Gordon: That's a bug, and there's a patch which fixes it - bug 164396. It's a characteristic of FSRef, not FSSpec. Because of that, bug 164396 stops using an FSRef to represent the file. But, assuming that nasty bug will be behind us, it shouldn't affect this argument.

shaver - the problem is that nsIFile is both a path and a logical file. some methods, we treat the nsIFile like a path (setLeafName), others we treat it like a file object (GetFileSize) At the end of the day, we should do what is easier for the developer. I would like to hear from Pete Colins since he did alot of work to support nsIFile ease-of-use in js (with his jslib)

All the places where you would pass a string to a POSIX function ("/tmp/foo"), we use nsIFile. stat(2) returns file size, permissions, etc., which we've broken out into a handful of methods. I think a lot of developer comfort is "principle of least surprise", and having moveTo mutate the path would certainly surprise me. (What happens if you call GetFileSize and your path points to a directory?)

I think what this bug demonstrates is that the principle of least surprise is a myth, at least in this case.. what it should be called is "the principle of what I personally would expect" :) it seems like we need to go with what is going to cause the least pain for the rest of the codebase, and then notify embeddors of the clarification to the API.

i believe that the nsLocalFileUnix implementation will give us just that -- "the least pain for the rest of the codebase." i think my examples demonstrate this.

re: comment #26: Conrad, directories can be specified in FSSpecs with just the vRefNum, dirID, and no leaf name, though technically it's not canonical. That could result in the pathname/(FSSpec|FSRef) incoherence problem for directories, but perhaps our code avoids that type of use. Anyway, I'm glad to see bug 164396 getting fixed. I was recently bitten by it in my disk cache cleaning code.