Last Comment Bug 689437 - Drop support for migrating from old profiles (drop usages of libreg)
: Drop support for migrating from old profiles (drop usages of libreg)
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Import (show other bugs)
: Trunk
: All All
: -- blocker with 1 vote (vote)
: Thunderbird 11.0
Assigned To: Justin Wood (:Callek)
:
Mentors:
: 689120 (view as bug list)
Depends on:
Blocks: 679352
  Show dependency treegraph
 
Reported: 2011-09-26 21:30 PDT by Philip Chee
Modified: 2015-04-16 16:11 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
unaffected
+
fixed


Attachments
WIP (105.39 KB, patch)
2011-12-11 19:07 PST, Ed Morley [:emorley]
bugspam.Callek: feedback+
Details | Diff | Splinter Review
patch2 (on top of WIP) (44.05 KB, patch)
2011-12-12 00:54 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
folded patch (do not land me) (134.89 KB, patch)
2011-12-12 00:56 PST, Justin Wood (:Callek)
standard8: review+
neil: superreview+
Details | Diff | Splinter Review

Description Philip Chee 2011-09-26 21:30:53 PDT
+++ This bug was initially created as a clone of Bug #689120 +++

Bug 679352 wishes to drop libreg support.

The easy way to do this for s/Thunderbird/SeaMonkey/ is to drop support for migrating from older Mozilla profiles - SeaMonkey 1.1.x and earlier (and hence the suite), and early versions of Thunderbird, from bug 679352 comment 4 I believe this is equivalent to:

> This code exists to import the profile list from Firefox <0.9.2 to
> Firefox >=0.9.3. I think it can go now.
Comment 1 Philip Chee 2011-09-27 11:26:19 PDT
Jens, I think this should be relnoted or otherwise documented.

http://forums.mozillazine.org/viewtopic.php?f=6&t=2316533

[quote]
So, Mozilla Core wants to get rid of some functionality which was necessary to import/migrate profiles prior to Firefox 0.9.x stored in the Mozilla Suite style (bug 679352). There is a corresponding Thunderbird version (bug 689120) to remove that functionality from comm-central as well. My guess is that it somehow (i.e., equally) would affect SeaMonkey as well.

Consequences? Starting with, let's say, SM 2.6, profiles from SeaMonkey 1.x, Netscape 6.x/7.x, and early (pre-toolkit) Firefox/Thunderbird version would no longer be recognized and migrated to the new format used since 2.0 (which was introduced much earlier by Firefox/Thunderbird).

Workaround: Users on 1.x or earlier would have to migrate to 2.[0-5] first, then update to 2.6 or later to be current.

I'm wondering how many users are actually still on SM 1.x or Netscape/Suite versions, thus would be affected by dropping that migration code... :-k
Last edited by rsx11m on Tue 27 Sep 2011 22:22, edited 1 time in total.
[/quote]
Comment 2 rsx11m 2011-09-27 11:32:14 PDT
Yes, this should definitely have a release note with the workaround once implemented.
Comment 3 Jens Hatlak (:InvisibleSmiley) 2011-12-11 04:38:56 PST
So, bug 679352 landed, libreg is gone. We need to get this done until 2.8 ships.

I guess a future relnote should recommend first upgrading to 2.0.14 and then to the latest 2.x.

While we're at it, we should probably go through the Startup & Profiles component and invalidate any bugs that still (exclusively) refer to the old migrator. And file bugs about porting new migrators, like for Opera and Chrome. :-)
Comment 4 Tony Mechelynck [:tonymec] 2011-12-11 11:16:13 PST
Without this fixed, 2.8a1 nightlies don't build. Escalating to major and suggest blocking.
Comment 5 rsx11m 2011-12-11 13:43:03 PST
The severity of Thunderbird bug 689120 has been elevated to "blocker" today.
Comment 6 Ed Morley [:emorley] 2011-12-11 19:07:37 PST
Created attachment 580813 [details] [diff] [review]
WIP

WIP, SeaMonkey and Thunderbird now build, but a bit rough around the edges (I don't have any experience with libreg/profile migration) & haven't tested it / looked at what tests need removing/changing. 

Sorry lumped together, but wasn't really sure what belonged here vs bug 689120, since today was the first time I've built c-c / really looked at the tree properly.

Anyway, enough excuses, hope it's ok enough to get you started :-)
Comment 7 Justin Wood (:Callek) 2011-12-12 00:54:11 PST
Comment on attachment 580813 [details] [diff] [review]
WIP

Review of attachment 580813 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, looks good to overall. There is lots and lots of cruft in this code as it is though :( But either way, a wonderful first step.

Going to attach my patch 2 (I want to give you credit when we push this) and then qfold it together for Mark and Neil to both review (either can say the others review is not necessary) and then land asap.

::: mail/components/migration/src/nsSeamonkeyProfileMigrator.cpp
@@ -309,5 @@
> -  seamonkeyData->Append(NS_LITERAL_STRING(OLD_FOLDER));
> -  seamonkeyData->Append(NS_LITERAL_STRING(REGISTRY_FILE));
> -
> -  return GetProfileDataFromRegistry(seamonkeyData, mProfileNames,
> -                                    mProfileLocations);

This whole section needs cleanup, doing bare minimum (so I don't break stuff) of that myself in patch 2

::: suite/locales/jar.mn
@@ -239,5 @@
>    locale/@AB_CD@/messenger/am-server-advanced.dtd                           (%chrome/mailnews/pref/am-server-advanced.dtd)
>    locale/@AB_CD@/messenger/am-server-top.dtd                                (%chrome/mailnews/pref/am-server-top.dtd)
>    locale/@AB_CD@/messenger/am-archiveoptions.dtd                            (%chrome/mailnews/pref/am-archiveoptions.dtd)
>    locale/@AB_CD@/messenger/appleMailImportMsgs.properties                   (%chrome/mailnews/appleMailImportMsgs.properties)
> -  locale/@AB_CD@/messenger/comm4xMailImportMsgs.properties                  (%chrome/mailnews/comm4xMailImportMsgs.properties)

NOTE: Seems suite was packaging this in the wrong exposed location anyway.

::: suite/profile/migration/src/nsProfileMigrator.h
@@ -42,5 @@
>  #include "nsIProfileMigrator.h"
>  #include "nsCOMPtr.h"
>  
>  #define NS_SUITEPROFILEMIGRATOR_CID \
> -{ 0x4ca3c946, 0x5408, 0x49f0, { 0x9e, 0xca, 0x3a, 0x97, 0xd5, 0xc6, 0x77, 0x50 } }

Technically I doubt we needed the CID change here, but I'd rather keep it in that risk breakage.

::: suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp
@@ +226,5 @@
>  
>  nsresult
>  nsSeamonkeyProfileMigrator::FillProfileDataFromRegistry()
>  {
> +  return NS_ERROR_FILE_NOT_FOUND;

We should probably just remove this provider.... (doing so in patch 2)
Comment 8 Justin Wood (:Callek) 2011-12-12 00:54:50 PST
Created attachment 580846 [details] [diff] [review]
patch2 (on top of WIP)
Comment 9 Justin Wood (:Callek) 2011-12-12 00:56:59 PST
Created attachment 580850 [details] [diff] [review]
folded patch (do not land me)

This is the folded version of the above 2 patches, for easier overall review (I'll be tackling any review comments anyway).

Feel free to cancel each others review if you like, I also already reviewed the code, but this is not my expertise of an area, so even if I didn't make changes would want one of you to review.
Comment 10 Mark Banner (:standard8) 2011-12-12 01:05:14 PST
Lets start by shifting this to mailnews core as it is effectively a core bug.
Comment 11 Mark Banner (:standard8) 2011-12-12 01:07:30 PST
*** Bug 689120 has been marked as a duplicate of this bug. ***
Comment 12 Mark Banner (:standard8) 2011-12-12 01:49:24 PST
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

Review of attachment 580850 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/migration/src/nsNetscapeProfileMigratorBase.cpp
@@ -165,5 @@
>    return NS_OK;
>  }
>  
> -nsresult
> -nsNetscapeProfileMigratorBase::GetProfileDataFromRegistry(nsILocalFile* aRegistryFile,

As we're stopping import from old versions, we need to change the importFromSeamonkey2.label (migration.dtd) string from

"Netscape 6, 7, Mozilla 1.x or SeaMonkey"

to

"SeaMonkey 2 or later".

Please bump the id of the string and the accesskey as well.

Whilst you're here, please also just remove the three sourceName* strings from Thunderbird's migration.properties as they aren't used and would be wrong after this patch anyway:

http://mxr.mozilla.org/comm-central/search?string=sourceName&find=%2Fmail%2F&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

(Suite uses them for setting the homepage in migration.js)

::: mail/components/migration/src/nsProfileMigrator.h
@@ +48,4 @@
>  #include "nsStringGlue.h"
>  
>  #define NS_THUNDERBIRD_PROFILEIMPORT_CID \
> +{ 0xea05dd97, 0xb66b, 0x4305, { 0xa6, 0x58, 0xd5, 0xde, 0x7f, 0x54, 0x0c, 0xc9 } }

Unless I'm very much mistaken, nsProfileMigrator is not an interface, so we don't need to change its CID. This is only a component id used to look up the component and not something binary extensions would hook into.

::: mailnews/import/content/importDialog.js
@@ -679,5 @@
> -              if (promptService) {
> -                var clickedOk = false;
> -                clickedOk = promptService.select(window, 
> -                                               gImportMsgsBundle.getString('profileTitle'),
> -                                               gImportMsgsBundle.getString('profileText'),

profileTitle & profileText need removing from importMsgs.properties as well.
Comment 13 Mark Banner (:standard8) 2011-12-12 02:21:24 PST
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

This looks fine with the changes mentioned addressed. I've not looked in detail at the changes in suite though, but they seem approximately correct.

This should also get sr due to the amount we're changing/removing.
Comment 14 neil@parkwaycc.co.uk 2011-12-12 02:40:44 PST
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

>   // Try profiles.ini first
>   nsresult rv = GetProfileDataFromProfilesIni(thunderbirdData,
>                                               mProfileNames,
>                                               mProfileLocations);
> 
>-  if (rv != NS_ERROR_FILE_NOT_FOUND)
>-    return rv;
>-
>-  thunderbirdData->Append(NS_LITERAL_STRING(REGISTRY_FILE));
>-
>-  // Then try the old registry format
>-  return GetProfileDataFromRegistry(thunderbirdData, mProfileNames,
>-                                    mProfileLocations);
>+  return rv;
> }
This entire block (including the unchanged lines) should just read

return GetProfileDataFromProfilesIni(thunderbirdData,
                                     mProfileNames,
                                     mProfileLocations);
Comment 16 Justin Wood (:Callek) 2011-12-12 04:34:41 PST
Landed followup (but pointed at wrong bug) http://hg.mozilla.org/comm-central/rev/7814c0f2732d

Note You need to log in before you can comment on or make changes to this bug.