Closed
Bug 190336
Opened 22 years ago
Closed 21 years ago
[mach-o] unable to migrate 4.x profiles
Categories
(Core Graveyard :: Profile: Migration, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: bugzilla, Assigned: ccarlen)
References
Details
(Keywords: platform-parity, relnote)
Attachments
(4 files, 6 obsolete files)
1.26 KB,
patch
|
ssu0262
:
review+
cavin
:
superreview+
|
Details | Diff | Splinter Review |
659 bytes,
patch
|
ssu0262
:
review+
cavin
:
superreview+
|
Details | Diff | Splinter Review |
14.37 KB,
patch
|
Details | Diff | Splinter Review | |
2.13 KB,
patch
|
mikepinkerton
:
review+
sfraser_bugs
:
superreview+
asa
:
approval1.4b+
|
Details | Diff | Splinter Review |
spun off from bug 190174. according to conrad, the mach-o build cannot migrate 4.x profiles. tentatively nominating. if minused, it prolly should be relnoted (combined with bug 190174).
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.3final
Updated•22 years ago
|
QA Contact: ktrina → gbush
Comment 1•21 years ago
|
||
4.x profiles were migratable using the smi.bin file for Netscape 7.0x see the document on early mac os x testing http://slip.mcom.com/projects/mojo/profile/MacOSXprofiletests.htm I'm not sure how many users will have the dual boot machine and still be using 4.x up to now for migration but it is a change in behavior and I am SURE we will get questions on it.
Comment 3•21 years ago
|
||
there might be a dup of this, already nsbeta1+. cavin and I were talking about it, and we definitely see places in mozilla/profile and mozilla/profile/pref-migrator and mozilla/mailnews (and ns/mailnews) places to worry about. here's my initial guess at a todo list: 1) XP_MAC vs XP_MACOSX issues 2) file spec / file path issues 3) finding and reading the 4.x profile registry issues I'm hoping that mach o builds already find and read mozilla 1.x profile registries? cavin is already starting on some of this. cavin, can you coordinate with ccarlen, to avoid duplicating efforts?
Comment 4•21 years ago
|
||
I think this bug is only for not being able to see the 4.x profiles at all (and so you can't migrate!) and bug 199565 is more for the migration problem itself.
Assignee | ||
Comment 5•21 years ago
|
||
This fixes the profile mgr so that 4.x profiles can be read. There is some overlap with Cavin's patch on bug 199565, but this has some additional profile mgr cleanup: (1) nsProfile::GetOldRegLocation started with an nsIFile, converted it to a path, only to have the one method which used it turn it back into an nsIFile. Now, it just stays as an nsIFile. (2) There was code in ProfileStruct::InternalizeLocation which handled reading a path from a mozilla5.0 profile as a Unicode HFS path: - // For a brief time, this was a unicode path - PRInt32 firstColon = regLocationData.FindChar(PRUnichar(':')); It was a brief time - a few days back in the "M" milestone days when somebody confused a persistent descriptor with a path. It's time that code path can be gotten rid of.
Comment 6•21 years ago
|
||
Conrad and I decided to merge the patch of this bug and bug 199565 together and post it here since there are overlaps in both patches.
Assignee | ||
Comment 7•21 years ago
|
||
Only did basic testing - migrating a 4.x profile with a POP account works.
Attachment #119771 -
Attachment is obsolete: true
Comment 8•21 years ago
|
||
I tried the patch and it works for imap as well. Filter rules are migrated fine too. We just need to remove two printf statements in the patch before getting review. So now we can see a list of 4x profiles when importing 4x mail (ie, from Tools|Import and then select Mail radio button), but the import process does not work due to some code was not compiled for OSX. A patch is coming.
Comment 9•21 years ago
|
||
Comment 10•21 years ago
|
||
Also, definition for "NewsFAT" needs to be corrected.
Comment 11•21 years ago
|
||
Comment 12•21 years ago
|
||
*** Bug 199565 has been marked as a duplicate of this bug. ***
Comment 13•21 years ago
|
||
Comment on attachment 119990 [details] [diff] [review] merged with cavin's nsPrefMigration changes Please ignore the two debugging printf calls for now as they will be removed before checking in.
Attachment #119990 -
Flags: review?(sfraser)
Updated•21 years ago
|
Attachment #120095 -
Flags: review?(ssu)
Updated•21 years ago
|
Attachment #120097 -
Flags: review?(ssu)
Comment 14•21 years ago
|
||
There is another addrbook sorting problem after 4.x migration is finished and it's addressed in bug 201588 (since it's not a MacOS X specific bug).
Comment 15•21 years ago
|
||
Comment on attachment 120095 [details] [diff] [review] Fix 4x mail import problem. r=ssu
Attachment #120095 -
Flags: review?(ssu) → review+
Comment 16•21 years ago
|
||
Comment on attachment 120097 [details] [diff] [review] Fix "NewsFAT" definition. r=ssu
Attachment #120097 -
Flags: review?(ssu) → review+
Comment 17•21 years ago
|
||
Comment on attachment 119990 [details] [diff] [review] merged with cavin's nsPrefMigration changes >Index: pref-migrator/src/nsPrefMigration.cpp >=================================================================== >-#if defined(XP_UNIX) >-#define IMAP_MAIL_FILTER_FILE_NAME_IN_4x "mailrule" >+#if defined(XP_UNIX) && !defined(XP_MACOSX) >+#define IMAP_MAIL_FILTER_FILE_NAME_IN_4x "maillrule" "maillrule" looks like a typo. >+#if defined(XP_MACOSX) >+ // For Mac OS X, the input path string is base64 encoded so we need >+ // to do something to get the slash-delimited pathname out of it. >+ printf("*** In ConvertPersistentStringToFileSpec(), input str=%s\n", str); Remove the printfs here and below. >+ nsCAutoString pathStr(str); >+ nsCOMPtr<nsILocalFile> localFile; >+ >+ rv = NS_NewNativeLocalFile(nsCString(), PR_TRUE, getter_AddRefs(localFile)); >+ NS_ENSURE_SUCCESS(rv,rv); >+ rv = localFile->SetPersistentDescriptor(pathStr); >+ NS_ENSURE_SUCCESS(rv,rv); >+ >+ nsXPIDLCString nativePath; >+ rv = localFile->GetNativePath(nativePath); >+ NS_ENSURE_SUCCESS(rv,rv); >+ printf("*** In ConvertPersistentStringToFileSpec(), str=%s\n", nativePath.get()); >+ rv = path->SetPersistentDescriptorString(nativePath); >+#else > rv = path->SetPersistentDescriptorString(str); >+#endif > return rv; > } >+#if defined(XP_MACOSX) >+ // For Mac OS X, make sure 'oldPath' contain slash-delimited pathname. >+ nsXPIDLCString nativePath; >+ rv = oldLocalFile->GetNativePath(nativePath); >+ NS_ENSURE_SUCCESS(rv,rv); >+ rv = oldPath->SetPersistentDescriptorString(nativePath); >+ NS_ENSURE_SUCCESS(rv,rv); I'm not sure I understand the comment, or the code. 'oldPath' is an nsIFile, it doesn't "contain" a pathname. It seems wrong that oldLocalFile exists with its native path set to a persistent descriptor. This should never have been created this way. >Index: src/nsProfile.cpp >=================================================================== >-#if defined(XP_WIN) || defined(XP_OS2) || defined(XP_MAC) >+#ifndef XP_BEOS >+ nsCOMPtr<nsIFile> oldRegFile; >+ rv = GetOldRegLocation(getter_AddRefs(oldRegFile)); >+ if (NS_SUCCEEDED(rv)) >+ rv = gProfileDataAccess->Get4xProfileInfo(oldRegFile, PR_TRUE); >+#endif Is it OK that that code didn't execute for XP_UNIX before, but does now? >+#ifndef XP_BEOS >+ nsCOMPtr<nsIFile> oldRegFile; >+ rv = GetOldRegLocation(getter_AddRefs(oldRegFile)); >+ if (NS_SUCCEEDED(rv)) { >+ rv = gProfileDataAccess->Get4xProfileInfo(oldRegFile, PR_FALSE); >+ gProfileDataAccess->mProfileDataChanged = PR_TRUE; >+ gProfileDataAccess->UpdateRegistry(nsnull); >+ } > #endif Same here.
Attachment #119990 -
Flags: review?(sfraser) → review-
Assignee | ||
Comment 18•21 years ago
|
||
I did the nsProfile/nsProfileAccess portion, so I'll speak for it ;-)
> Is it OK that that code didn't execute for XP_UNIX before, but does now?
Yes, the change makes it so that GetOldRegLocation returns null for XP_UNIX.
Get4xProfileInfo() takes nsnull for that param. That XP_UNIX uses nsnull as the
file param was a detail I wanted to hide at a lower level. Really, #ifndef
XP_BEOS could be changed to #ifdef SUPPORTS_4X_MIGRATION and it might read better.
Comment 19•21 years ago
|
||
>>+#if defined(XP_MACOSX) >>+ // For Mac OS X, make sure 'oldPath' contain slash-delimited pathname. >>+ nsXPIDLCString nativePath; >>+ rv = oldLocalFile->GetNativePath(nativePath); >>+ NS_ENSURE_SUCCESS(rv,rv); >>+ rv = oldPath->SetPersistentDescriptorString(nativePath); >>+ NS_ENSURE_SUCCESS(rv,rv); > >I'm not sure I understand the comment, or the code. 'oldPath' is an nsIFile, it >doesn't "contain" a pathname. It seems wrong that oldLocalFile exists with >its native path set to a persistent descriptor. This should never have >been created this way. > I remember if we use the old code: > rv = oldLocalFile->GetPersistentDescriptor(persistentDescriptor); > if (NS_FAILED(rv)) return rv; > rv = oldPath->SetPersistentDescriptorString(persistentDescriptor.get()); then the pathname in 'oldOath' is the base64 encoded stuff so that's the reason for the change.
Comment 20•21 years ago
|
||
Can you supply enough context for that last change so that I can see what's going on?
Comment 21•21 years ago
|
||
This patch fixes old-style nsPersistentFileDescriptor to use Base64-encoded aliases, and not full paths, for Mac OS X.
Updated•21 years ago
|
Attachment #120767 -
Flags: review?(ccarlen)
Assignee | ||
Comment 22•21 years ago
|
||
Comment on attachment 120767 [details] [diff] [review] Patch to fix nsFileSpec persistent desciptors for XP_MACOSX The patch looks good, but be careful to check all usage of nsFileSpec persistent descs.
Attachment #120767 -
Flags: review?(ccarlen) → review+
Comment 23•21 years ago
|
||
Simon's patch to nsFileSpec persistent desciptors for XP_MACOSX does work so removing code to get native path from nsILocalFile and put it into nsFileSpec code.
Attachment #119990 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #120782 -
Flags: review?(sfraser)
Comment 24•21 years ago
|
||
The patch description was set wrong, this is a better one.
Attachment #120782 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #120782 -
Flags: review?(sfraser)
Updated•21 years ago
|
Attachment #120783 -
Flags: review?(sfraser)
Comment 25•21 years ago
|
||
You still have a typo here: +#if defined(XP_UNIX) && !defined(XP_MACOSX) +#define IMAP_MAIL_FILTER_FILE_NAME_IN_4x "maillrule"
Comment 26•21 years ago
|
||
sfraser is right, if you don't fix that, you'll break UNIX/Linux profile migration.
Updated•21 years ago
|
Attachment #120783 -
Flags: review?(sfraser)
Updated•21 years ago
|
Attachment #120851 -
Flags: review?(sfraser)
Updated•21 years ago
|
Attachment #120851 -
Flags: review?(sfraser) → review+
Updated•21 years ago
|
Attachment #120851 -
Flags: superreview?(sspitzer)
Updated•21 years ago
|
Attachment #120095 -
Flags: superreview?(sspitzer)
Updated•21 years ago
|
Attachment #120097 -
Flags: superreview?(sspitzer)
Assignee | ||
Comment 28•21 years ago
|
||
Comment on attachment 120767 [details] [diff] [review] Patch to fix nsFileSpec persistent desciptors for XP_MACOSX Seth, do you know of places in the mailnews code where, under XP_UNIX, the format of a persistent desc is assumed to be a native path? Hopefully not - if that was the case, we would have broken when we started reading CFM profiles with the Mach-O build (6 months ago).
Attachment #120767 -
Flags: superreview?(sspitzer)
Comment 29•21 years ago
|
||
Comment on attachment 120095 [details] [diff] [review] Fix 4x mail import problem. sr=sspitzer.
Attachment #120095 -
Flags: superreview?(sspitzer) → superreview+
Comment 30•21 years ago
|
||
Comment on attachment 120097 [details] [diff] [review] Fix "NewsFAT" definition. sr=sspitzer.
Attachment #120097 -
Flags: superreview?(sspitzer) → superreview+
Comment 31•21 years ago
|
||
Comment on attachment 120767 [details] [diff] [review] Patch to fix nsFileSpec persistent desciptors for XP_MACOSX sr=sspitzer.
Attachment #120767 -
Flags: superreview?(sspitzer) → superreview+
Comment 32•21 years ago
|
||
Comment on attachment 120851 [details] [diff] [review] merged with cavin's nsPrefMigration changes , v3 sr=sspitzer.
Attachment #120851 -
Flags: superreview?(sspitzer) → superreview+
Comment 33•21 years ago
|
||
Need to add check for imap type before copying msg filter rules. Everything else stays the same.
Attachment #120851 -
Attachment is obsolete: true
Comment 34•21 years ago
|
||
Comment on attachment 120922 [details] [diff] [review] 120851: merged with cavin's nsPrefMigration changes , v4 carry over r=sfraser and sr=sspitzer (from v3).
Attachment #120922 -
Flags: superreview?
Updated•21 years ago
|
Attachment #120922 -
Flags: superreview?
Assignee | ||
Comment 35•21 years ago
|
||
All patches, by a cast of many, are checked in. Thanks for the help.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 37•21 years ago
|
||
spoke too soon- not getting mail settings/ bookmarks
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 38•21 years ago
|
||
On a tree I pulled last night, I did both debug and opt (-O) builds from the same src. Debug works, opt doesn't. nsPrefMigration.cpp is a challenge for optimizers, and it might be the problem. See: http://lxr.mozilla.org/seamonkey/source/profile/pref-migrator/src/nsPrefMigration.cpp#78
Assignee | ||
Comment 39•21 years ago
|
||
We have run into this same bug before - with the very same code even :-/ Quicker to explain than find that bug: You can't take the address of a cast. gcc compiles it and it works in debug build. In opt build, it doesn't complain when compiling it - it just silently fails at runtime. Patch fixes the cast and combines more of the XP_MAC and XP_MACOSX code.
Attachment #120767 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #121578 -
Flags: superreview?(sfraser)
Attachment #121578 -
Flags: review?(pinkerton)
Updated•21 years ago
|
Attachment #121578 -
Flags: superreview?(sfraser) → superreview+
Comment 40•21 years ago
|
||
Comment on attachment 121578 [details] [diff] [review] patch to fix opt build r=pink
Attachment #121578 -
Flags: review?(pinkerton) → review+
Assignee | ||
Comment 41•21 years ago
|
||
Request is only for attachment 121578 [details] [diff] [review]. That fixes a bug which prevents the whole rest of the patch from working on opt builds. Low risk since this is to fix code which was not previously enabled.
Flags: blocking1.4b?
Updated•21 years ago
|
Flags: blocking1.4b? → blocking1.4b+
Comment 42•21 years ago
|
||
Comment on attachment 121578 [details] [diff] [review] patch to fix opt build a=asa (on behalf of drivers) for checkin to 1.4b
Attachment #121578 -
Flags: approval1.4b+
Assignee | ||
Comment 43•21 years ago
|
||
This was checked in on 04/25/2003 12:29. Thought I marked this fixed :-/
Status: REOPENED → RESOLVED
Closed: 21 years ago → 21 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•