Closed Bug 40750 Opened 25 years ago Closed 24 years ago

add flag to nsIFile for "no symbolic links allowed"

Categories

(Core :: XPCOM, defect, P3)

All
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: warrensomebody, Assigned: dougt)

Details

(Keywords: perf, Whiteboard: patch reviewed!)

Attachments

(2 files)

It has been reported to me that a large amount of time during startup (and in general?) goes to resolving symbolic links (aliases, shortcuts) for nsIFile, or at least checking to see if we need to resolve (usually non-existent) symbolic links. Perhaps we should consider adding a flag to nsILocalFile init routine(s) to say whether symbolic links are allowed or not. Then when resolving resources (e.g. chrome urls) we would just say "these are never allowed to be symbolic links" and bypass the need to check for links. I don't think allowing symbolic links in our resource hierarchy is necessary. (Note this optimization might be unnecessary when we switch most resources over to jar files, but the flag in general might help us in other places.)
Keywords: perf
I know that we have hashed this once or twice before but: nsIFile - the symlink autoresolution. we should just make resolution an explicit call. This is what is expected on windows and the mac from the developers point of view anyway and it will reduce complexity (probably improve performance). On unix, it could just noop. The more I think about this, the more I think it is the right thing to do. thoughts?
Are you sure that would yield a usable interface? It would be bad if code had to test for symlinks and dereference them at every step. That's why I thought that just declaring "this path is allowed to contain symlinks" vs. "this one doesn't" might be better. But I'm open to suggestion.
what would it do on unix?
also, if we added a flag, I would rather have an attribute instead changing the initWithPath(). This is so that someone can set it after it has been inited. Maybe it should return an error if there were any symlinks in what the nsIFile was initilized to.
Looking at this.
Status: NEW → ASSIGNED
I added this attribute and got everything building on windows. I have tested directory enumeration, stating every file as we go. Windows was implement so that shortcut resolution would only happen if there was a file-not-found condition when we try to use the nsIFile. Hence, there is no performance improvement on windows while interation over the path c:\winnt\. Unix is always going to symlink resolve so this is a no brainer and no improvement. Mac is the one that may have improve performance. We are resolving alot.
Dougt, got a patch to attach to this bug yet? /be
Attached patch first cut (deleted) — Splinter Review
Not fully tested on linux/mac. Windows looks okay.
heck, i don't mean testing I mean implementing...
Attached patch second cut - unix diff. (deleted) — Splinter Review
I have a r=blizzard on the patches. Need approval. note this does note fix mac yet. there is another bug for that.
Whiteboard: patch reviewed!
fix is in. windows and unix work fine. Mac still does nothing with any values set. I will fix that with bug 38481.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
dougt, how can I verify this? testcase? Thanks!
It's only useful to do this if you're going to fix the places in the code that can't have links, and pass the flag in there, e.g. resource urls. Have you done that? If not, we should reopen this.
warren, resource url/file transport need to resolve because the mac uses alias in dist. I would love to disable this, but component registration, ect. would stop working.
Why does the mac use aliases in dist? Seems unnecessary since other platforms don't. Let's remove it.
Why are symlinks (aliases, whatever) in dist bad? /be
No way, dude. Using aliase in dist allows Mac to track file dependencies properly, and also allows developers to edit files via aliases in chrome/ etc, without having to worry about the extra make step that windows developers need. It also means that relinking a DLL is all that is required to update the one in components or whatever. The short: aliases make our lives easier. Please don't take that away.
This conversation should be put out on the mac/build ngs.
Aliases make your life much much slower (at least as measured on windows -- I assume mac is similar). When we complete the jar file step, you won't have them anyway, so both our arguments are moot. I would like to identify places in the code that can never allow aliases, and set the flag for them so that we get the performance benefit.
warren, acutally, I want to know where in the code we require them. i would love to be able to just say to hell will alias resolution all together, but are some small places that need them... more so on the mac than any platform.
Well, for one thing, we never use folder aliases. So it would be safe, I think, to only permit leaf aliases for resource URLs. I think clients will want to use aliases for other uses of nsILocalFile, e.g. I should be able to replace my profile folder with an alias to a different folder.
we don't have that kind of gradularity with the current api. either it resolves automatically or not. we should change this to take flags, but I don't like the complexity it will create.
Sounds like still an open issue. Reopening...move along the bug train if you disagree.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
that would be a different bug. This bug is fixed.
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
please verify
warren, please mark verified is all is well.
QA Contact: leger → warren
asdf
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: