Move app support directory to Library/Application Support/Camino

RESOLVED FIXED in Camino0.9

Status

Camino Graveyard
Preferences
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: Patrick C. Beard, Assigned: Conrad Carlen (not reading bugmail))

Tracking

({crash})

unspecified
Camino0.9
PowerPC
Mac OS X
crash

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

16 years ago
I have been reorganizing my disk, and have moved my home directory from one disk
partition to another. I used CCC 2.0 to do this, and each time the home
directory was on a different non-System volume. It appears to be crashing in the
bookmark code. I will enclose a crash log.
(Reporter)

Comment 1

16 years ago
Created attachment 107290 [details]
Crash log
(Reporter)

Comment 2

16 years ago
I can't reproduce this with the tip of the Chimera branch.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 3

16 years ago
That's because bookmarks was fixed to not crash immediately on not having a
profile. But, the underlying problem is still there. The profile dir is recorded
in the profile registry as an alias. An alias is only valid on the volume on
which it was made - moving the target to another volume breaks the alias.
Chimera, on setting the profile, doesn't do any checking as to whether the
profile dir actually exists or have any recovery code. I'm still of the opinion
that Chimera should not use the profile mgr since it only ever has one profile.
It should just use a standalone nsIDirectoryServiceProvider, make the "profile"
dir relative to "~/Application Support" and not have this problem.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 4

16 years ago
But, not a bookmarks problem at this point...
Component: Bookmarks → Preferences
Keywords: crash

Comment 5

16 years ago
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.
Severity: normal → critical

Comment 6

16 years ago
*** Bug 191429 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

16 years ago
The profile directory service provider has been factored out of the profile mgr.
It's in mozilla/profile/dirserviceprovider and builds a static lib which can be
used by an application without the profile mgr. I think Chimera should use this.
It would solve this problem and we would be able to remove a component from the app.

I propose making the new "profile" folder be "~/Library/Application
Support/Chimera" instead of "~/Library/Application
Support/Chimera/Profiles/default/xxxxxxxx.slt

Problem is, copying the contents from old to new location would have to be done
by hand unless we want to keep registry code in Chimera only for this purpose.
The registry code is being moved out of xpcom into the profile mgr so, since I
want to stop using that lib, would be a problem. Or, we could find the old
profile dir by digging around in "default" for "prefs.js"

Or, since we're pre-1.0, is it acceptable to relnote copying your old profile
data to the new location?

Comment 8

16 years ago
chimera 0209
after mooving of my homefolder, chimera lost preferences and is not able to save
new one!! neither preferences, nor bookmarks. Help!!!! Everythink else works
correctly...
(Assignee)

Comment 9

15 years ago
Created attachment 126848 [details] [diff] [review]
WIP patch to not use profile mgr

The patch makes Camino no longer use the profile mgr. It's silly to use the
profile mgr since its purpose is managing a list of profiles and Camino only
supports having one. Also, the profile mgr tracks profiles using aliases,
leading to problems when moving your home directory, etc. The patch just puts
the data in ~/Application Support/Camino, which can always be found.

The reason it's "In Progress" is that it does not migrate your data over from
Chimera/Profiles/default/xxxxxxxx.slt. For now, it has to be done by hand.
Created attachment 128545 [details] [diff] [review]
updated patch

I took the WIP patch and added in "copy from old profile into new directory"
support.

I have no idea if anything beyond the PreferenceManager.mm stuff will apply
cleanly.  I applied everything manually, and to recreate this patch I just
changed the relevant part of diffs.txt.
Attachment #126848 - Attachment is obsolete: true
cc'ing myself

and as an aside - since this is just for Camino, go native.  Registry code isn't
required with NSFileManager around.
(Assignee)

Comment 12

15 years ago
+  NSString *pathString = [[NSString stringWithString:@"~/Library/Application
Support/Chimera/Profiles/default"] stringByExpandingTildeInPath];

Use the folder manager to find ~/Library/Application Support. I doubt that that
string is valid in all locales.

Other than that, looks pretty good. Without reading the actual registry using
nsIRegistry, you could run into troubles:
(1) The profile was moved somewhere else - after all, the registry contains an
alias to it.
(2) The profile dir is is not called "default" but "default-n"
The profile mgr does a CreateUnique when making new dirs and if there were
orphan profiles around, it would have appended.

If we really want to be certain of finding the real location of the current
profile dir, it means reading old registry - unless we're willing to lose some
edge cases.
Created attachment 128550 [details] [diff] [review]
even more updated patch

just like last one, but now . .. 
*using carbon file manager to get Application Support Directory.
*iterate through Chimera/Profiles to find the most-recently-modified directory
to count as our old directory.

This'll at least handle the folks that haven't moved anything but do have
default-x as their profile. but no, it still doesn't handle everything.
Attachment #128545 - Attachment is obsolete: true

Comment 14

15 years ago
CC'ing pinkerton. I thought the salted directory was for security reasons.

Comment 15

15 years ago
There is a VERY good security reason to have salted names for the profile. I'll
summarize, an attacker CAN write to files in the profile directory, e.g., to
your cookies file, to the cache, potentially to other places. An attacker
typically CANNOT read from the file system. However if they know the location of
the cookies/etc. file, they CAN trick you into loading that file which might
contain malicious code that could compromise your system. This patch would
introduce an attack vector that was deliberately blocked in Mozilla.

So IMO, making that change, to eliminate salted profile dirs, would be
boneheaded from a a security standpoint.

More info here: bug 56002.
(Assignee)

Comment 16

15 years ago
Simon: I'm familiar with bug 56002. That attack (of which there was never a
reported case before salting was implemented, anyway) relies on the attacking
code being able to know the *absolute* path to a file in your profile. On OS X,
that absolute path will contain your account name, which is just as unguessable
as the salt string. That expoit was really only possible on a single-user system
(like Win95) where C:\WINDOWS\Mozilla\Profiles\default was a valid and constant
path. That not being the case on OS X, the salt is useless.

Comment 17

15 years ago
Agreed. I withdraw my objection.

Comment 18

15 years ago
After installing this, Camino lost my preferences (e.g., home page setting)
is home page setting the only thing it lost?  in other words - did your
bookmarks get migrated over?  History?  Etc?

I believe Camino has code to specifically overrides homepage preferences to
bring you to http://www.mozilla.org/projects/camino/homepage.html unless a
certain flag is set on build.
my concern with just copying outright is that the first time the user upgrades
from 0.7, they'll have 50MB of cache in thousands of small files. this will take
a while to copy and they may force-quit right in the middle.

That's why I've been mulling about how to do this effectively, but haven't come
up with any ideas.
Assignee: sfraser → pinkerton
Status: REOPENED → NEW
Target Milestone: --- → Camino0.8
(Assignee)

Comment 21

15 years ago
The simple thing would be to just not copy any item that was named "Cache" or
"Cache.Trash." The patch is using an iterator already - skipping over certain
items would be pretty straightforward. But, my 50MB Cache dir can be copied (by
Finder) in about 5 secs. Assuming NSFileManager is comparable, I'm not sure how
much of an issue this is? I guess we could calc the size of the profile dir and,
if above a certain size, put up a progress dialog.
it copies quickly because you've updated to the new cache on the trunk,
probably, which has many fewer files. remember how long it takes the cache on
the branch to "clean up the cache" at startup? it can hang for tens of minutes
on slower macs iterating over every file and deleting it.

that's what i'm worried about.
It's easy enough to change from "copyPath:toPath" to "movePath:toPath", which I
believe would be pretty much instantaneous no matter how large the cache is. 

Of course, if you downgrade after upgrading, your bookmarks will appear to be
lost  . . . 
right. i think there will be people who downgrade back to 0.7, esp after using
pre-0.8 trunk builds. we cannot hose them by moving the profile.
Created attachment 130091 [details] [diff] [review]
yet another update

Updated the "even more updated patch" so that cache folders get moved but
everything else gets copied to the new profile directory. All the caveats of
the last past still apply - I have no idea if anything other than
PreferenceManager.mm will apply cleanly.
Attachment #128550 - Attachment is obsolete: true
Created attachment 130094 [details] [diff] [review]
fixed patch

arg - sorry about this - I didn't update before the last patch and apparently
somebody's worked on PreferenceManager.mm.
Attachment #130091 - Attachment is obsolete: true
i don't think it's necessary to move the cache dir. just don't copy it.
there are still a few things wrong with this latest patch:

- can't "Cache" be localized by mozilla? did any of the chimeral10n projects
include a localized mozilla?
- we perform the conversion if we don't find a "history.dat" file. it's
perfectly valid not to have one (if someone tosses it to get rid of their
history wholesale). This shouldn't re-trigger a conversion, and more
importantly, will clobber new data with the data from the 0.7 profile. Bad news.
- i say don't copy or move the cache folder (whatever we determine it to be
called, just skip it wholesale)
- conrad says that there's a way to read the registry inside the old profile
folder in order to get the salted dir. this would be much better than the adhoc
method here. i don't think it lives in the profile library, i think it's a
separate stub library.
Status: NEW → ASSIGNED
To answer your questions:

1) Did somebody localize Cache - I don't know.  And if they did - why?  It's a
freakin' hidden directory.  I'd say it'll just skip directories, but then it'd
leave out "chrome".  Which appears to be empty.  OK, it'll skip directories on
copy.  If there's a problem with this, let me know.

2) Sure, we can try something besides history.dat.  How about secmod.db instead?
 Or maybe bookmarks.plist?  A combo of the two or more?  Let me know what works.

3) I know next-to-nothing about Moz internals, and don't have the time to figure
them out right now. If nsIRegistry is located in a teeny tiny library someplace,
feel free to let me know which one & I'll try to figure out what call to make.
Or, maybe you could leave in the profile library for 0.8, and pull it for 0.9
(assuming no one would jump from 0.7 to 0.9 without going through 0.8)?  If you
want to do that, then I can steal from what was here before without a problem.
(Assignee)

Comment 30

15 years ago
1) "Cache" is never localized, It's set here:
http://lxr.mozilla.org/seamonkey/source/netwerk/cache/src/nsDiskCacheDevice.cpp#1129
to this nicely hardcoded string. In mozilla, it can be set to any arbitrary dir
by way of a pref. Since Camino has never supported setting that pref, we'll
never see that situation.

2) Let's read the registry and avoid all that.

3) There is way to do this that wouldn't rid us of the profile mgr dll: Use
nsIProfile::GetCurrrentProfile(), and use that name with 
nsIProfileInternal::GetProfileDir(). That will return the old profile dir in few
lines of code and no groveling around for files that might not exist. Maybe we
could do that short term and then, if I ever have spare time, I can crib the
nescesary code from nsProfileAccess.cpp to read the data without using the whole
profile mgr.
Created attachment 130207 [details] [diff] [review]
Non-working patch

This was my final attempt to patch PreferenceManager.mm.  It doesn't compile,
so clearly I've done something wrong.  I'm going to rely on someone else to
figure out what.

Things that may be good:  

*Patch checks to see if "new" profile directory contains 
bookmarks.plist, cookies.txt, cookperm.txt, history.dat, and prefs.js.	If any
of them are there - it doesn't import.	I haven't tested this yet.  Why?  The
patch won't compile.

*Patch copies files from old profile dir, and skips directories.  Also
untested.

*Patch attempts to read old profile dir using nsIProfile & nsIProfileInternal,
as suggested.  

Things that are bad:

It doesn't compile.  Why?  I get a "no matching function for call to
`nsDerivedSafe<nsIProfile>::GetProfileDir(nsXPIDLString&,
nsGetterAddRefs<nsIFile>)" error.  To set these calls, I pretty much copied &
pasted from other locations in the source where they've been used before.  I
missed something, but I don't know what.  I've tried including more headers,
which didn't work.  I've played around with different calls - still doesn't
work.  I doubt if I got past the first line the second line would work.
(particularly the do_queryinterface call).

I'm going to eat some chinese food and move on with my life.  I hope this helps
whoever ends up getting this thing fixed.
*** Bug 216244 has been marked as a duplicate of this bug. ***
fwiw, it builds after changing that line to:

if
(NS_FAILED(profileInternal->GetProfileDir(oldProfileName.get(),getter_AddRefs(oldProfileDir))))

you were calling a method on the wrong interface (doh!). 

Comment 34

15 years ago
Sorry I wasn't CC'd. 

Re comment #19: It lost all of the prefs stored using the Cocoa defaults system,
so my home page, my custom fonts settings, my tabs settings. Everything else
copied over fine.

Re: Cache dir, I agree with Mike, dump it.

Does that, with the correction by Mike, bring this up to speed? This is blocking
212630 the bookmarks manager, which I want to get in.

Comment 35

15 years ago
changed summary (was Crash after moving home directory (Chimera 0.6))
Summary: Crash after moving home directory (Chimera 0.6) → Move app support directory to Library/Application Support/Camino

Comment 36

15 years ago
Localizing "Cache" directory is unnecessary. David has resolved the issue of
which file to use and uses nsIRegistry now. It no longer copies the Cache
directory. People who downgrade after updating will still work. Mike has a patch
that buiids for him.

Is everyone happy now?
uh, no, cuz the patch doesn't even come close to working. all i said was that it
builds.

Updated

15 years ago
No longer blocks: 212630
Target Milestone: Camino0.8 → Camino0.9

Comment 38

15 years ago
don;'t think this bug is for the original crasher any more.
Severity: critical → normal
(Assignee)

Comment 39

15 years ago
Created attachment 140020 [details] [diff] [review]
new patch

This patch rids us of using the profile dylib altogether. It uses nsIRegistry
to read the profile location out of the existing profile registry. It uses the
standalone directory service provider that the profile mgr does (and exports
for embedding apps that have only 1 profile). It checks for the existance of
prefs.js in the new profile location and, if not present, migrates the old -
skipping the Cache dir. BTW, the patch contains a change where I added
USE_GCC3=YES so that the project can be built within PB when running on
Panther. That should probably be done for all the targets as well.
(Assignee)

Updated

15 years ago
Assignee: pinkerton → ccarlen
Attachment #107290 - Attachment is obsolete: true
Attachment #130094 - Attachment is obsolete: true
Attachment #130207 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #140020 - Flags: review?(pinkerton)
Attachment #140020 - Flags: review?(pinkerton)
Attachment #140020 - Flags: review?(josha)
Attachment #140020 - Flags: review+
(Assignee)

Comment 40

15 years ago
This patch will require a change to Tinderbox scripts for this:
> Deleting /Users/cltbld/Library/Application Support/Chimera...
Also, if the scripts set prefs by writing to prefs.js?

Who can help with the Tinderbox changes?
Pinkerton should update the tinbderboxes to Panther Next week or so, might be a
great time to do this too.
landed. thanks conrad!
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago15 years ago
Resolution: --- → FIXED
Comment on attachment 140020 [details] [diff] [review]
new patch

Mike some people are using the query for bugs needing reviews. Please take the
time to remove review flags.
Attachment #140020 - Flags: review?(josha)
You need to log in before you can comment on or make changes to this bug.