SIMS IMAP: migration causes imap folders to be copied to local mail folder

VERIFIED FIXED in mozilla1.0.1

Status

VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: torrey.mcmahon, Assigned: racham)

Tracking

({imap-interop})

Trunk
mozilla1.0.1
Sun
Solaris
imap-interop

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 RTM])

Attachments

(6 attachments, 8 obsolete attachments)

12.01 KB, text/plain
Details
1.20 KB, text/plain
Details
24.89 KB, image/jpeg
Details
29.62 KB, text/plain
Details
12.01 KB, patch
racham
: review+
sspitzer
: superreview+
chofmann
: approval+
Details | Diff | Splinter Review
12.02 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

18 years ago
From Bugzilla Helper:
User-Agent: Mozilla/4.77 [en] (X11; U; SunOS 5.9 sun4u)
BuildID:    Mozilla 0.9

When starting up mozilla for the first time the migration wizard takes my imap
only config and duplicates it just fine. However it also copies the contents of
my mail folders to the location it sets for LocalMail. This duplicates all of my
mailboxes.

Reproducible: Always
Steps to Reproduce:
1. Clear .mozilla directory
2. Start mozilla for the first time and migrate old 4.X setup using only imap
3. Look in .mozilla/somepathto/ImapMail and see copies of mail folders

Actual Results:  Mailboxes were duplicated taking up extra disk space.

Expected Results:  Not copied them. Maybe created an index file.

This only happens on migration. I can create a new profile and this doesn't
happen.

Comment 1

18 years ago
In 4.x had you downloaded folders for offline use?  Perhaps when we migrate,
because we didn't support offline, we copied the contents to Local Folders? 
Esther or Grace have you ever seen this?

Comment 2

18 years ago
From migration today: 

4.x mail:  /u/gbush/ns_imap/nsmail-2 

mozilla mail

.mozilla/gbush/y5e408xq.slt/ImapMail/ImapMail/nsmail-2 
.mozilla/gbush/y5e408xq.slt/Mail/Mail/Local Folders/Local Mail 

I see an extra level of directories- on both Imap and local mail but imap 
folders are not copied into local.  

adding laurel to cc list
that extra level bug is known.  I think cavin owns the bug now.
(Reporter)

Comment 4

18 years ago
Created attachment 35581 [details]
Netscape 4.X Preferences file
(Reporter)

Comment 5

18 years ago
I've attached my preferences file. I currently have no local folders set up and
run everything over IMAP.

Comment 6

18 years ago
Cavin, weren't you seeing cases where we were using imap:// instead of 
mailbox://?  I wonder if we are getting mixed up in the opposite way in this 
case?

Comment 7

18 years ago
Scott, I remember I saw the problem on windows when I was fixing #66018.  But
have not seen it after it's fixed.  The fix for #66018 and #50567 were checked
in on 5-22-01 so I would suggest the reporter to try any build after 5-22-01 and
see if it still has the same problem.  

The imap:// vs. mailbox:// problem you mentioned was discovered while fixing
#81741 where we would generate rul like "imap://nobody@Local Folders/Drafts" but
that was also fixed.

Let's see if any build after 5-22-01 would solve the problem and if it still
fails then we'll look more into it.
(Reporter)

Comment 8

18 years ago
Tried the build from 5/27 and the same thing happened.

Comment 9

18 years ago
Used WinNT 06-25-06-0.9.2 build.
Used IMAP account:
If I setup "Copies & Folders" on the IMAP server from 4.77, after migration, 
those setup have pointed to Local for 6.1....which is not correct!! :-(
Ccing myself.

Comment 10

18 years ago
Karen, it sounds like that might be a separate bug.

Comment 11

18 years ago
I logged another bug 87733 already...

Comment 12

18 years ago
I cannot reproduce this on 2001070604 build 
(Reporter)

Comment 13

18 years ago
Just tried the 07/08 nightly build and it still copies the mailboxes into the
ImapMail directory on migration. (At least for me....)
a quick look at torrey's 4.x prefs, and I see:

user_pref("news.default_fcc", "/home/tmcmahon/Mail/Sent");
user_pref("news.directory", "/home/tmcmahon/Mail");
user_pref("mail.default_fcc", "/home/tmcmahon/Mail/Sent");
user_pref("mail.default_drafts", "IMAP://tmcmahon@hutch.east/Drafts");
user_pref("mail.imap.root_dir", "/home/tmcmahon/Mail/");
user_pref("mail.imap.server.hutch.east.server_sub_directory",
"/home/tmcmahon/Mail/");
user_pref("mail.directory", "");

your pref values look odd to me.

here are mine:

user_pref("mail.directory", "/home/seth/nsmail/");
user_pref("mail.imap.root_dir", "/home/seth/ns_imap/");
user_pref("news.directory", "/home/seth/");      

which raises questions:

1) do you have "Local Mail" in 4.x?  when you send mail, does a copy of the
message you send go into your Sent folder in "Local Mail"?  how about when you
post to newsgroups?

2)  did you manually set "mail.directory" to ""?

3)  did you manually set your "news.directory" pref?

can you do a "ls -al" of /home/tmcmahon/Mail and attach the output it to the bug
report?
(Reporter)

Comment 15

18 years ago
Created attachment 41703 [details]
Output of ls -la on mailfolder directory
(Reporter)

Comment 16

18 years ago
1) do you have "Local Mail" in 4.x?  when you send mail, does a copy of the
message you send go into your Sent folder in "Local Mail"?  how about when you
post to newsgroups?

Nope. Everything goes through IMAP.

2)  did you manually set "mail.directory" to ""?

Nope. I seem to remember the GUI always appending that but it seems to no longer
occur

3)  did you manually set your "news.directory" pref?

Yes. Just to keep things neat I point everything to that folder.

can you do a "ls -al" of /home/tmcmahon/Mail and attach the output it to the bug
report?

Attached.
> 1) do you have "Local Mail" in 4.x?  

curious, in messengenr (in 4.x) is there a "Local Mail" account at the top level?

(Reporter)

Comment 18

18 years ago
Yes. There is a Local Mail heading in the pull down. I'll attach a snapshot.

Also, if it helps I can move the .netscape directory out of the way, recreate my
preferences, and see if it cleans anything up.
(Reporter)

Comment 19

18 years ago
Created attachment 41771 [details]
Window from messenger showing LocalMail
(Reporter)

Comment 20

18 years ago
I completely recreated my Netscape 4.77 profile, mostly through the gui with a
couple of things like moving my cookies/bookmarks/customeprefs by hand, and the
migration went 20X faster and didn't copy the folders. (That's with the
07/10/2001 nightly build.)

I'll attach output from diff -c on my original and current 4.X preferences shortly.
(Reporter)

Comment 21

18 years ago
Created attachment 41778 [details]
Diff -c from Netscape preferences
(Reporter)

Comment 22

18 years ago
Looking at the diffs myself I see a lot of cruft seems to have gone away when I
recreated my preferences from scratch. I'm guessing that over the years
migrating across different netscape versions, enabling roaming access, setting
up fully qualified domain names for my mail server, etc. etc. etc. something
wasn't cleaned up correctly and caused the bug.

Comment 23

18 years ago
Change QA Contact to me. 
What kind of the IMAP server are you using?
Also, can you check the IMAP directory setting from the Advance IMAP for us from 
4.7? Thanks.
QA Contact: gbush → huang
(Reporter)

Comment 24

18 years ago
Imap server is SIMS 2.0 from, you guessed it, Sun.

The imap directory is /home/tmcmahon/Mail/

Comment 25

18 years ago
Adding interop for the keywords
Keywords: interop

Comment 26

18 years ago
Have you changed the IMAP directory setting before?
I mean have you setup other IMAP directory before you setup current 
"/home/tmcmahon/Mail/" for the IMAP directory from the IMAP advanced setup?
Summary: migration causes imap folders to be copied to local mail folder → SIMS IMAP: migration causes imap folders to be copied to local mail folder
(Reporter)

Comment 27

18 years ago
I changed the imap server a couple of times. Most recently from "hutch.east" to 
"hutch.east.sun.com". I would delete it and then re add it or change it as
needbe. (MY mail has moved servers a couple of times.) I take it the preferences
module doesn't clean up after itself?

/home/tmcmahon/Mail from the IMAP advanced setup.

Comment 28

17 years ago
BuildID =2001092805-0.9.4
OS = Win XP Ja, Professional, Version 2002, Win ME Ja 

Can't create a new account using migrated user profile having IMAP setting. 
Is this ImapMail folder copy problem caused this ?  

Comment 29

17 years ago
Created attachment 72948 [details] [diff] [review]
patch

In the patch, I modified four files and added three files. I don't know how to
give the diff of the new added files, so I just put them in the end of the
patch. If there are anything needed I to change the patch, just tell me. Thank
you.

Comment 30

17 years ago
Can anyone give review to this bug? Thanks.

Comment 31

17 years ago
Hi, Seth Spitzer, can you review this patch 72948? Thanks a lot.

Updated

17 years ago
QA Contact: huang → meehansqa

Comment 32

17 years ago
We need someone to check on the patch attached.  Is sspitzer@netscape.com out
till the end of the month?  If so, can someone else check on this for us please?
 We would like to check it in to the 1.0.1 branch if possible.  Thanks, Margaret

Comment 33

17 years ago
Not sure why I'm not here. CCing to myself.

Comment 34

17 years ago
you need to get buyoff from the UI group for this UI, I believe, and also the
i18n group since we're in a localization freeze, as I understand it.

Comment 35

17 years ago
Ummm...so it has very little chance going into 1.0.1 then, I guess.  Henry:  Can
we bypass the GUI and make the default as the norm.  Would that work?  If so,
then we may not need buy in from the UI and localization team.

Comment 36

17 years ago
Created attachment 88289 [details] [diff] [review]
new patch with UI

This new patch have the same function as attachment #72948 [details] [diff] [review]. Please take a look
at the UI parts and give some feedback, UI & L10N experts. Thx.
Attachment #72948 - Attachment is obsolete: true

Comment 37

17 years ago
Created attachment 88291 [details] [diff] [review]
new patch without UI

This patch has the same function without UI. So, please r= this patch first.
Please. It's important for some users who have heavy mail folders.

Updated

17 years ago
Keywords: patch, review

Comment 38

17 years ago
bienvenu@netscape.com:  Can you give us some comments on Henry's patch.  It is
becoming a hot issue for us to deploy the browser within Sun.  We really need to
get this issue resolved and check it into the trunk at least.  Ideally, we would
like to see this going into the 1.0.1 branch and eventually go out with our 7.0
release.  That's why I've asked Henry to modify the patch to skip the popup. 
Any help you can provide or any suggestions you may have is very much
appreciated.  At least we need to know whether or not this approach is headed
towards the right direction.  Thanks, Margaret

Comment 39

17 years ago
I don't really understand this code. Seth wrote this code, so you probably will
have to talk to him. From what little I can understand here, the imap server
profile migration code is copying the files in the imap server directory? Why
would it do that? The Mozilla client will figure out the server hierarchy when
it connects. All we need to migrate is the filters file. So I'm guessing somehow
we're migrating the local directory, which to us is the local folders directory.
You can't simply decide not to migrate local folders, so that can't be right. Is
the problem that we're setting the local path for the "none" server to the imap
server directory, and then migrating the imap server directory? That doesn't
really make sense either...

Comment 40

17 years ago
Hi, David,

When migrating, mozilla copies all the contents in the directories of IMAP
server and 'Local Folders'. For an example, on unix, the two folders are always
'ns_imap' and 'nsmail' in the user's home directory. Of course, just as you
said, the contents in the IMAP server directory are the index files and the
filters file, not the really contents on the IMAP server. But it is a heavy copy
when copying the 'Local Folders' where there are a lot of folders and mails.
Someone reports the bug because there are more than 1G mails in his 'Local
Folders' and mozilla seems to be dead when migrating, and the migration causes
great disk consuming. That's the main point of the bug. The patch is to supply a
option for the user to copy the contents in NS 4.x mail folder into the new
created folder in '.mozilla/...' or just use the NS 4.x mail folder and don't
copy. The patch without UI is defaultly using the NS 4.x mail folder and not
copying.

Just as Margaret said, this is considered as a big issue for SUN. We need the
patch to be put into the OEM_Branch. What should we do now? Who else should we
contact besides Seth? you, mscott, naving...? Thx very much.

Henry

Comment 41

17 years ago
 So this has nothing to do with imap, if I understand correctly. You're just
turning off migration of local folders? You can't do that by default (which is
what I assume w/o a UI means). Also, you can't just have 6.x and 4.x point at
the same directory - that's not supported, and could cause lots of problems if
they're both running at the same time and changing the mailboxes at the same time.

I think we have to wait for Seth to get back. Since you can't do this by
default, you need a UI, which I guess means it won't make it for 1.01, if I
understand you correctly.

Comment 42

17 years ago
Hi bienvenu@netscape.com:

Thanks for your help on checking out Henry's proposed solutions.  Can you
elaborate on the problems which you have mentioned if we leave the mail in the
exact same location as 4.x?  We do not spot any problem so far with the patch
applied.  The only awkward thing we see is that if we switch back to 4.x after
running 6.x, then there are some irrelevant 6/7.x files showing up, but they
don't seem to be harmful in terms of functionality.  Do you know of any
potential problem that we may run into that we have not yet found out?

Comment 43

17 years ago
As I said before, the problems would occur if you had both 4.x and 6.x running
at the same time, both reading and writing to the mailbox at the same time. It
could cause corruption of your mail store very quickly. We decided at the start
of 6.x that migrated profiles would be in a new directory, in part to avoid
these sorts of problems.

Comment 44

17 years ago
Ah, I have not thought of that scenario at all mainly because we have used other
mailer and Netscape 4.7.x mailer before.  Setting local folder to the same
location never poses a problem for us.  Perhaps they have certain built-in
locking mechanism which can prevent this problem, and that's missing in mozilla
or Netscape 6/7.  Anyway, as I have said, we have applied the patch in our 6.2.x
release and so far we have not heard of any report on this problem yet.  

Now that I understand your concern, I wonder what will happen if I have 2
sessions of 6.x mailer running at the same time.   Now their data store are in
the exact same location, will they possibly have the same kind of data
corruption problem?

Comment 45

17 years ago
you can't have two sessions of netscape 6 or 7.x running at the same time, at
least on windows or mac (not sure about linux) because the OS prevents multiple
instances. And with profile locking, it's not allowed even on Linux.

Comment 46

17 years ago
For PC and Mac, in single user mode, I can see the limitation.  I have not 
tried to bring up multiple sessions of mozilla on my Solaris box lately (I do 
try to avoid doing so because of its resource consumption), but I have 
definitely done that before (many times).  Unless something has changed that I 
am not aware of, I certainly don't see the profile being locked to prevent 
multiple sessionsof mozilla being run on Solaris.  If there is profile locking 
on Linux, I sure want to check it out.  I will try it out on my unix box once 
again tomorrow and report here on what I see.

Anyway, so far we have not received any reports on data corruption (we have 
made solaris download available since 6.0 days).  However, we do receive quite 
a few complaints on the copying of local mailboxes.  In a shared filing system 
enterprise environment like ours, this sure is a big problem.  I am not trying 
to down grade the importance of the potential data corruption problem, but we 
sure need to address the copying problem right away. 

Would it be possible to have a preference setting available for the user (some 
enterprise user like our IT folks) so that they can override the default 
action.  That is:

1.  Keep the default setting of mail migration to copy the local folders to a 
separate workspace under .mozilla.
2.  But provide a preference setting for the user to override this default 
setting if they choose to do so.

This way, whoever chooses to override the default will take the consequences of 
what the overriding will lead you to.

Do you think that this can be a possible solution?  Thanks, Margaret

Comment 47

17 years ago
Created attachment 88626 [details] [diff] [review]
patch without UI and use preference

This patch has the same function without UI and use preference for the
migration mode.

Comment 48

17 years ago
For using the patch without UI and using preference.

Before migration, add a line to the preference file of NS 4.x.

user_pref("mail.migration.mode", 1);
1 standards for share folder with 4.x
0 standards for use different folder from 4.x, this is default and this is equal
to without this line

Comment 49

17 years ago
These patches are for discussion with the same function.

Comment 50

17 years ago
Just want to report back on my findings.  I am able to bring up two sessions of 
mozilla on a Linux box and a Solaris box.  And same for mail.  Perhaps this 
practice is not expected or not encouraged, but I can certainly do so without 
any blockage of any locking mechanism.  It would be nice to find out if 
multiple sessions of mozilla are supported especially with the important apps 
like mail, address book, etc.  If it is not, then I think that some sort of 
blocking should be used to prevent such action.  If it is supported, then I am 
seeing similar risk here as two different browsers, 4.7x and 6.x/7.x, being 
running at the same time.  Wonder if some file locking mechanism should be 
considered here to prevent such risk.  

Comment 51

17 years ago
On the trunk (not the branch), there is profile locking.

I'll say it again - Mozilla does not support multiple clients accessing the same
profile data. Period. Pretty much all of the data files mozilla uses can get
corrupted if you have multiple clients accessing the same profile data. I'm not
talking just about mail files. Browser history, the browsing cache, etc., can
all get corrupted if you have multiple clients accessing the same profile.
That's why the trunk has profile locking. Obviously, this should be prevented in
some sort of general way, but what we've done for now is to have 4.x and 6.x
have different profile directories, and, at least on Windows and Mac, prevent
multiple instances of the client running at the same time.

Comment 52

17 years ago
bienvenu@netscape.com:  Would you mind telling me the bug id where the profile 
locking is being patched?  We need to look at that in more detail to see what 
the impact would be.  On one hand, I agree that some kind of locking mechanism 
is certainly needed if data corruption will happen without it.  However, on the 
other hand, I am concerned that once it is locked, no access can be granted 
even though the user is trying to bring it up on another system.  It is a very 
common practice in our networking environment, where the profile is in one's 
home directory that is accessible everywhere.  For example, one can bring up 
one instance of the client on one system in the office and another instance of 
the client on another system in another place.  If this is not supported, we'll 
have work to do...  We certainly need to understand how this profile locking 
mechanism works.  Is it locked per system or per user?  It sounds like that it 
is per user.  Anyway, hope that you can provide us with some pointers of this 
profile locking enhancement.

Getting back to the original problem of this bug.  It has several reports 
saying that sometimes local mails are copied to the .mozilla directory and 
sometimes they are not.  Do you know what causes this inconsistency?   Is this 
a conditional copy?  Henry, do you also have any clue on why we are seeing this 
inconsistent behavior? 

Thanks, Margaret 

Comment 53

17 years ago
Margaret,

Not sure. For me and from the code, mozilla should copy always.

Henry

Comment 54

17 years ago
I don't know the profile bug number off the top of my head (I'm not cc'ed on it
or anything - I've just seen the general problem discussed in e-mail), but you
can search for it using bugzilla.

Comment 55

17 years ago
Thanks much.  I search in bugzilla and it looks like that it is 76431.  OK, so
the profile locking mechanism is to address the problem with multiple 6.x/7.x
instances, and the copying during migration is to address the problem with both
4.7x and 6.x/7.x running.  With this understanding, and the copying is still not
acceptable by our enterprise deployment team, we would still need an option for
not copying the mail.  Would you and Seth (he should be back today, I hope)
consider Henry's last patch?  Basically, we need to have a way to not doing the
copy.  By taking this approach, we'll definitely give our users enough warning
on the risk of running both 4.7x and 6.x/7.x at the same time.  As a matter of
fact, I would suggest Henry to modify the patch to look at the preference in the
mailnews.js file in .mozilla instead of reading it out from 4.x if that's
possible.  That seems to be cleaner.  Please consider this preference reading
alternative as we do need them to move forward.  Thanks, Margaret

Comment 56

17 years ago
I'm confused why copying files on Solaris is so slow. It seems to work OK on
windows. Are we somehow copying more than we should, or is this copy happening
over a network? Also, it seems like that pref has to go in 4.x somehow, because
during profile migration, there is no 6.x profile to put the pref in!

Comment 57

17 years ago
>Are we somehow copying more than we should, or is this copy happening
>over a network? Also, it seems like that pref has to go in 4.x somehow, because
>during profile migration, there is no 6.x profile to put the pref in!

That's a very good question:  What should be copied and what should not?  Our
home directory is always network mounted, and therefore the copy is happening
over the network.  And it is unlikely that it will be changed.  The big concern
besides speed is space consumption.  With our freedom of keeping as much mail as
we want to, some of the mailboxes can be extremely huge.  I understand that
users' 6.x profile is not there yet during migration, but the ITOPS folks can
put them in the default mailnews.js file which will stay in the installed
location.  Would that work?  

Comment 58

17 years ago
we shouldn't be copying the whole home directory, but just our mail
subdirectory. Re the default pref stuff, that sounds like it should work but
it's beyond my area of expertise :-) Seth is back today, however.
(Reporter)

Comment 59

17 years ago
Why are we copying the whole mail directory? Or are you saying that's a bad
thing and we shouldn't be doing it at all though we know how to fix that? ;^)

Comment 60

17 years ago
Torrey, I've tried to explain this in prior comments, but basically the answer
is that 6.x has a completely different profile directory than 4.x, and that
profile directory is made by copying the data from the 4.x profile to the 6.x
profile (and this is not just mail, but everything, bookmarks, certs, etc).
(Reporter)

Comment 61

17 years ago
Sorry about that. (Too many bugs flying by at light speed.)

One way to fix this might be to write a wrapper around the netscape startup, ala
the Sun shared applications scripts, that pulls the LocalMail preference and
then puts it back in. That's really messy though.



Comment 62

17 years ago
Hi bienvenu@netscape.com:  It is definitely not copying the whole home
directory, but mail subdirectory alone will create enough headache for us. 
Thanks for your comments on the default pref solution, I will solicit Seth on
this.  Hopefully, we can get his OK.  

Torrey:  The reasoning for the copying is already discussed earlier.  Thanks for
your suggestion on the wrapper solution.  We will take that into consideration. 
The wrapper is owned by IT, but we're working closely with IT to resolve this
issue.   

Comment 63

17 years ago
Steps to reproduce on windows.
1. Make sure there is no generated 'Mozilla' directory in your profile
directory. Something is like 'C:\Documents and
Settings\henry.BASEBALL\Application Data\Mozilla', where henry is your username,
BASEBALL is the machine name.
2. Run 4.x and make sure you can access your mail, then put some mails into the
'Local Folders' or its sub folders and close it. Check the following directory
to see the profile in 4.x. Something is like 'C:\Program
Files\Netscape\Users\default', where Netscape is the directory where you install
NS 4.x, default is the profile name. In the profile directory, you'll see
'ImapMail' and 'Mail' folders, and in the sub folders in 'ImapMail', you'll see
'.snm' files, in 'Mail', you'll see some files and '.snm' files with the same
basic name.
3. Run NS 6/7, make sure you can access your mail, check the generated profile
directory, 'C:\Documents and Settings\henry.BASEBALL\Application Data\Mozilla'.
You'll see 'ImapMail' and 'Mail' folders. In them, you'll see the copied files
with the name of '.snm' files changed to '.msf' files.

I tested with NS 4.76 and NS 7 PR1.

Comment 64

17 years ago
Henry, are you describing a problem? That seems like correct behaviour. Could
you describe what in particular is wrong about that behaviour? Are you just
saying we shouldn't be copying the files?

Comment 65

17 years ago
Because some people can't reproduce this, I just give out the steps to
reproduce. This is the steps on windows. I'll also give out the steps on linux.
Then everyone who is interested in this issue can discuss more easily. Just as
you said, it is the mode mozilla is doing in.

Because a lot of people in Sun have a lot of mails in the 'Local Folders' and
they don't want the copy operation, because it will take a lot of time and
consume a lot of space. This is why Sun think this is a showstopper. We're now
trying to find some solution.

Comment 66

17 years ago
Steps to reproduce on Linux & Solaris.
1. Make sure there is no generated '.mozilla' directory in your home directory.
2. Run 4.x and make sure you can access your mail, then put some mails into the
'Local Folders' or its sub folders and close it. Check the related directory,
'ns_imap' and 'nsmail' by default, in your home directory to see the mail files,
in the sub folders in 'ns_imap', you'll see '.summary' and '.sbd' files, in
'nsmail', you'll see some files and '.summary' files with the same basic name,
of course, you'll also see '.sbd' files standarding for folder that contains sub
folders.
3. Run NS 6/7, make sure you can access your mail, check the generated profile
directory, '.mozilla'. You'll see 'ImapMail' and 'Mail' folders. In them, you'll
see the copied files with the name of '.summary' files changed to '.msf' files.

I tested with NS 4.76 and Mozilla 1.0(Gecko/20020529).

Comment 67

17 years ago
Henry mentions this with regard to his last patch (patch id # 88626)

>My third patch to use the preference file of NS 4.x also can be used to 
>work with mailnews.js. Just add a line in this file is OK.
>pref("mail.migration.mode", x);
>where if x is equal to 0, then do the copy operation as mozilla usually 
>do, if x is not 0(of course, we may set it to 1), then forbid the copy 
>operation.

In other words, his last patch should work if the preference is set in the
default mailnews.js file.
re-assigning to racham, who will be doing the first level reviews.  (I'll do the
super review).

some initial comments / questions about the patch:

1)  you should default this pref in mailnews.js
2)  the pref should be a boolean pref, not an integer pref
3)  The patch isn't as low risk as I'd hoped.  (And the higher the risk, the
less likely it will make it into 7.0).  I was hoping the fix for this would just
be a new (and hidden pref), and we'd check the pref and skip existing code if
the pref was set and just set a given pref.

In your patch, are you just renaming (moving) the ~/nsmail directory to the
right place under ~/.mozilla?  Why not just check the hidden pref, and if we've
set to not copy, just skip the copy and set the directory pref to point to the
existing ~/nsmail location?  That seems like it would be a lot simpler and a lot
lower risk.  Henry, can you look into doing it that way?

Some things you need to worry about, which bienvenu has already raised:

1)  there is no profile locking, file locking between 4.x and 7.0.  So if your
users use 4.x and 7.0 at the same time to write local mail, bad things will happen. 

2)  after using 7.0 on the ~/nsmail directory, it will create .msf summary files.
While 7.0 should be ignoring .foo.summary files (see nsShouldIgnoreFile() in
nsLocalMailFolder()), 4.x will *not* ignore the 7.0 summary files, example foo.msf

So even if you don't use 4.x and 7.0 simultaneously, there will be problems
using them sequentially.  Basically, if you don't copy nsmail, you can't use 4.x
anymore.
Assignee: sspitzer → racham
After thinking about it, henry's patch might have been trying to solve some of
the 4.x / 7.0 using the same directory problems by moving ~/nsmail to the
desired location instead of copying it.  Assuming the ~/nsmail directory lived
on the same machine as ~/.mozilla (I'm guessing for SUN they are both on the
same NFS mounted file system), renaming would be fast and not require any copy.

Then, if users started up 4.x, there shouldn't be any problems, as 4.x will just
create a new ~/nsmail directory.  (Of course, to 4.x users, it will look like
all the local mail has been deleted, but that's not as bad as having 4.x and 7.0
writing to the same files.)

If I've got that right, the bug is really about checking the hidden pref and
doing a rename instead of a copy (for local mail.)  I think we could still make
the patch less risky, but I'll leave that for henry and racham to work about.

Comment 70

17 years ago
does that mean that 4.x users will not be able to access their old mail after
profile migration? Effectively, they won't be able to go back to 4.x? That might
upset some people.
(Reporter)

Comment 71

17 years ago
Moving the entire mailfolder is fraught with peril. Creating a link is much
easier. There may be some confusion in people seeing .MSF files if they switch
back but I'm not as concerened about that as I am people killing the migration,
or it bombing out, half way through the move, permission problems, etc. etc. etc.

Comment 72

17 years ago
I reproduced this with Netscape 7.0pr1 on solaris 8

Netscape copy all local mail's contents form /yourhome/nsmail(4.7's local
folder) to /yourhome/.mozilla/.../Mail/Local Folders(7.0's local folder)

If 4.7 user save big local file,the migration will spend long time and waste
disk space.

I suggest netscape should check 4.7's local file when migrate.If local file is
small(<10M or other),cp local file for 7.0. If local file is big,show the
instruction to cp local file by manual or tell potential risk to user.
(Reporter)

Comment 73

17 years ago
I'd have to vote against Patrick's proposal. Much too confusing for the average
user that barely knows what a mailfile is let alone what 10MB is. Not to mention
it would cause lots of issues if users switch between 4.X and 7.X.

I still think a link is the best way to go.

Comment 74

17 years ago
Created attachment 89386 [details] [diff] [review]
patch without UI and use bool preference

Explain to the patch(same function as others):
1. use mailnews.js for the setting
2. use bool value instead of integer value according to Seth's suggestion
3. forbid copy operation, and not use 'move' operation, thus, user can still
use NS 4.7 and see the old mails
4. the only insufficiency is that when user use NS 4.7, he/she will see the
unexpected '.msf' folders

Comment 75

17 years ago
If you want to forbid the copy operation, set the preference
pref("mail.migration.mode", false);
in mailnews.js.

racham, pls r=

Comment 76

17 years ago
Hi Bhuvan:

Any comments on Henry's latest patch?

Margaret
(Assignee)

Comment 77

17 years ago
Some comments & questions :

1. Since we are going to have a default behavior (which can be changed by anyone
who can modify the pref via cnofiguration tools), the default pref should be in
mozilla/modules/libpref/src/init/mailnews.js. When you add the pref (with value
set to true by default), please add lot of comments as to what this pref means
and what side effects it will bring in when the value changed. I would also
prefer a different name to the pref something like mail.migration.copyMailFiles
or anything better.

2. Once you add the pref to defaults file (i.e., mailnews.js) the following can
be changed to return rv on failure (please break this into 2 lines - something
that has been advocated a lot lately. will be useful while debugging).

+  if (NS_FAILED(rv)) copyMailFileInMigration = PR_TRUE; 

3. Can you tell me why 
+    rv = newIMAPLocalMailPath->AppendRelativeUnixPath(NEW_LOCAL_MAIL_DIR_NAME);
is moved up in the default choice path..?

4. Your new overloaded Function nsPrefMigration::DoTheCopyAndRename is just
renaming the files. I think you should get rid of word Copy from the name.

5. The code you have removed in Index: nsMessengerMigrator.cpp, isn't something
that need to be done conditionally depending on the pref (new pref we have
introduced here) value..?

6. How much of testing was done on this one ? Did you test
sending/receiving/filtering mails ? If you have done testing, was it all done
only on Solaris..?

Patch looks fine for the things you want to do. Please add comments wherever
possible as to what will be benifits/consequences when a particular path is taken.

Comment 78

17 years ago
I also reproduced the bug on linux and windows2k.(NS4.7x,mozilla 1.0)

Comment 79

17 years ago
Created attachment 89541 [details] [diff] [review]
patch without UI and use bool preference - new

Since we agree on this solution, obsolete others.
Attachment #88289 - Attachment is obsolete: true
Attachment #88291 - Attachment is obsolete: true
Attachment #88626 - Attachment is obsolete: true
Attachment #89386 - Attachment is obsolete: true

Comment 80

17 years ago
Created attachment 89543 [details] [diff] [review]
patch without UI and use bool preference - new

Thx, racham, for your review of the patch.

I've just made a mistake. I gave a old patch. Here is the new one.

Following are explains for the new patch and answers for your questions.
1. give the explain for the preference as the comments in the source code,
including how to use it, the risk and how to handle the risk
2. change
+  if (NS_FAILED(rv)) copyMailFileInMigration = PR_TRUE;
into
+  if (NS_FAILED(rv))
+    copyMailFileInMigration = PR_TRUE;
3. the reason to move +    rv =
newIMAPLocalMailPath->AppendRelativeUnixPath(NEW_LOCAL_MAIL_DIR_NAME); ... up
after the move, then we can write the exact folder name in the preference
'prefs.js' in the continuous code, something like that
user_pref("mail.directory", "C:\Documents and
Settings\henry.BASEBALL\Application
Data\Mozilla\Profiles\default\msvk65gz.slt\Mail\Local Folders");
instead of 
user_pref("mail.directory", "C:\Documents and
Settings\henry.BASEBALL\Application
Data\Mozilla\Profiles\default\msvk65gz.slt\Mail");
Thus, after the migration, when we use this preference to access the 'Local
Folders', we need not to append 'Local Folders'.
4. This function now do the copy and rename operation. I changed a little in
the new patch. Should not directly rename the files.
5. because the two conditions of different setting of the mail files copy have
been handled in nsPrefMigration.cpp, so in nsMessengerMigrator.cpp, no need to
handle the conditions. see part 2 in this comment for the implementation in
nsPrefMigration.cpp
6. I've tested sending/receiving/filtering. I tested on win2k and solaris.

racham, please review again. thx.

Henry
Attachment #89541 - Attachment is obsolete: true
(Assignee)

Comment 81

17 years ago
Comment on attachment 89543 [details] [diff] [review]
patch without UI and use bool preference - new

minor nits :

1.
+// this is for the hidden preference setting in
mozilla/modules/libpref/src/init/mailnews.js
+// pref("mailnews.timeline_is_enabled", true);

--> +pref("mail.migration.copyMailFiles", true);

2. 
+// see 80035

--> see bugzilla bug 80005 (http://bugzilla.mozilla.org/show_bug.cgi?id=80035)

3. 
+// mozilla will copy all the contents of Local Folders and Imap Folder to the
new created subfolders of .mozilla

--> +// mozilla will copy all the contents of Local Folders and Imap Folder to
the newly created subfolders of migrated mozilla profile

Also, please have 80 or so chars as per line limit for the comments so that it
will be easy to read.

4. 
+// Advantage of forbiding copy operation:

--> +// Advantage of forbidding copy operation:

+// Disvantage of forbiding copy operation:

--> +// Disadvantage of forbidding copy operation:

5.
+  if (NS_FAILED(rv))	// if we fail to get the value, just use the default
value
+    copyMailFileInMigration = PR_TRUE;

--> You can return rv now. Since you have default now in the mailnews.js file. 

+  if (NS_FAILED(rv))
+    return rv;

This is a candidate for trunk baking.

r=bhuvan
Attachment #89543 - Flags: review+
(Assignee)

Comment 82

17 years ago
4. 
+// Advantage of forbiding copy operation:

--> +// Advantages of forbidding copy operation:

Updated

17 years ago
Blocks: 121324

Comment 83

17 years ago
Created attachment 89752 [details] [diff] [review]
patch without UI and use bool preference - new

new patch according to racham's suggestion
Attachment #89543 - Attachment is obsolete: true

Comment 84

17 years ago
Comment on attachment 89752 [details] [diff] [review]
patch without UI and use bool preference - new

Just some changes in the comments, so still change the status to has-review.

racham, if you find something not right, let me know. Thx, racham.

sr= wanted
Attachment #89752 - Flags: review+
in his review, bhuvan asked why we needed these changes:

   // set the local path for this "none" server
-  //
-  // we need to set this to <profile>/Mail/Local Folders, because that's where
-  // the 4.x "Local Mail" (when using imap) got copied.
-  // it would be great to use the server key, but we don't know it
-  // when we are copying of the mail.
-  rv = mailDirSpec->AppendRelativeUnixPath(mLocalFoldersHostname.get());
-  if (NS_FAILED(rv)) return rv; 


+    rv = newIMAPLocalMailPath->AppendRelativeUnixPath(NEW_LOCAL_MAIL_DIR_NAME);
+    if (NS_FAILED(rv)) return rv;
+    rv = newIMAPLocalMailPath->Exists(&exists);
+    if (NS_FAILED(rv)) return rv;
+    if (!exists)  {
+      rv = newIMAPLocalMailPath->CreateDir();
+      if (NS_FAILED(rv)) return rv;
+    }
+
     {
       // temporarily go through nsFileSpec
       nsFileSpec newIMAPLocalMailPathSpec;
@@ -924,15 +960,6 @@
       if (NS_FAILED(rv)) return rv;
     }
 
-    rv = newIMAPLocalMailPath->AppendRelativeUnixPath(NEW_LOCAL_MAIL_DIR_NAME);
-    if (NS_FAILED(rv)) return rv;
-    rv = newIMAPLocalMailPath->Exists(&exists);
-    if (NS_FAILED(rv)) return rv;
-    if (!exists)  {
-      rv = newIMAPLocalMailPath->CreateDir();
-      if (NS_FAILED(rv)) return rv;
-    }
-

Why do we need to change the code path for copy style migration?

I agree with bhuvan, and something like this would be better:

// get the migration mode for mail
rv = m_prefs->GetBoolPref(PREF_MIGRATION_MODE_FOR_MAIL, &copyMailFileInMigration);
if (NS_FAILED(rv))
  return rv;


// only do thise for copy style migration because....
if (copyMailFileInMigration) {
   //
   // we need to set this to <profile>/Mail/Local Folders, because that's where
   // the 4.x "Local Mail" (when using imap) got copied.
   // it would be great to use the server key, but we don't know it
   // when we are copying of the mail.
   rv = mailDirSpec->AppendRelativeUnixPath(mLocalFoldersHostname.get());
   if (NS_FAILED(rv)) return rv; 
}

That would make it so the code path for copy migration is the same as before,
which is less risky.

Also, "Disvantage" should be spelled "Disadvantage"

Comment 86

17 years ago
Created attachment 89881 [details] [diff] [review]
patch without UI and use bool preference - new

According to Seth's suggestion, make this patch less risky by keeping the code
same as before for the 'copy' mode in migration.

Seth, pls sr= again. Thx.

I tested on solaris and win2k.
Attachment #89752 - Attachment is obsolete: true

Comment 87

17 years ago
Comment on attachment 89881 [details] [diff] [review]
patch without UI and use bool preference - new

Just little changes to keep the same code as before for the original 'copy'
mode in migration, so still change the status to has-review.

racham, if you find something not right, let me know. Also, if you have some
time, please take a look at it. Thx, racham.
Attachment #89881 - Flags: review+

Comment 88

17 years ago
Seth, please sr= the patch. Thx.

Comment 89

17 years ago
Seth:  

Any more comments on Henry's latest patch?  If not, can yo give him a sr= 
please?  Hopefully, it can be checked in soon.

Thanks, Margaret
Comment on attachment 89881 [details] [diff] [review]
patch without UI and use bool preference - new

this patch looks good.	sr=sspitzer

removing has-review, because I don't think racham has actually reviewed this
patch yet.
Attachment #89881 - Flags: review+ → superreview+

Comment 91

17 years ago
Thank you!  Seth.  Since you have removed the has-review, I guess you would 
expect us to get a r= from racham for the last patch then, right?  If so, 
Bhuvan:  Would you review the last patch for us and give us a r= please?  
Hopefully, we would be able to get this all set to go before everybody is off 
for July 4th.  Thanks, Margaret 

putterman@netscape.com:  We need approval to go into the 1.0.1 branch once it 
is in the branch.  Shall we add the mozilla1.0.1 keyword to the Keywords field?
(Assignee)

Comment 92

17 years ago
Comment on attachment 89881 [details] [diff] [review]
patch without UI and use bool preference - new

looks good. r=bhuvan.
Attachment #89881 - Flags: review+
(Assignee)

Comment 93

17 years ago
Henry,

Do you have CVS write access ? If not, I can land this for you. Let me know.

Bhuvan

Comment 94

17 years ago
Thank you very much, Bhuvan.  If you won't mind, would you check it in for us 
please?  It will be another 5/6 hours before Henry can see this update anyway.  
He is in a different timezone.  By the way, I have added the mozilla1.0.1 
keyword to flag that we want it to be in the branch.  Hopefully it is the right 
protocol.  Margaret
Keywords: mozilla1.0.1
margaret, you should also send mail to drivers@mozilla.org
(Assignee)

Comment 96

17 years ago
Fix landed on the trunk. Marking fixed. 

Adding adt1.0.1 keyword & setting mozilla1.0.1 milestone.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Keywords: adt1.0.1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.0.1

Comment 97

17 years ago
Comment on attachment 89881 [details] [diff] [review]
patch without UI and use bool preference - new

a=chofmann for 1.0.1. add the fixed1.0.1 keyword after checking into the
branch.
Attachment #89881 - Flags: approval+

Comment 98

17 years ago
Thank you Bhuvan.  Seth:  Henry has already sent mail to drivers, so I did not 
do that.  It looks like Chris has just given us the approval to check into the 
branch.  Henry:  would you get this check in to the branch please?  Thanks, 
Margaret

Comment 99

17 years ago
Created attachment 90174 [details] [diff] [review]
patch for branch 1.0

I've checked the patch into branch 1.0.

Comment 100

17 years ago
Add the key word fixed1.0.1

Thx, everyone.

Margaret, would you please verify the branch?
Keywords: fixed1.0.1

Comment 101

17 years ago
Hi Henry:  Can Patrick or someone in QA verify this?  I won't be able to verify 
anything until next Monday/Tuesday.  Sorry about that, Margaret

Comment 102

17 years ago
I will verify this once the build is done. Grace Bush will be verifying as well.

Comment 103

17 years ago
ok on Windows trunk 2002070508

Comment 104

17 years ago
verified on Win2k - IMAP and POP migration 
verified on Mac 9.2- IMAP and POP migration
verified on Mac OS X - IMAP and POP migration 

Help Desk working on my mail on Linux- will try to verify that soon

Comment 105

17 years ago
I've verified on Win2K and Mac 9.2 as well, looks good
posthumus adt1.0.1+, as this has been checked into the 1.0 branch.
Keywords: adt1.0.1 → adt1.0.1+
Whiteboard: [adt2 RTM]

Comment 107

17 years ago
finally got to a Linux machine- verified both IMAP and POP migrations 

Comment 108

17 years ago
Torrey - we've tested in all environments here, migration is behaving as
expected. Would like to mark this as VERIFIED - have you tested it there, do you
concur?

Comment 109

17 years ago
I verified the latest 1.0 branch on linux,w2k and solaris 9.

Comment 110

17 years ago
Marked it verified1.0.1 per Patrick's comments.
Keywords: fixed1.0.1 → verified1.0.1

Comment 111

17 years ago
This bug has been tested and verified on the following (POP and IMAP):
Linux, W2K, Solaris9, MAC 9.2, MAC OSX

Marking as VERIFIED
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.