322628 - Palm Sync Build Changes to automate build and install in profile extensions

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Scott MacGregor]

some minor tweaks are needed to facilitate building the palm sync extension.

Comment 1

[Scott MacGregor]

Attachment: support the palm sync build option in static builds

step one, make sure the palmsync dll doesn't get folded into the static package list.

Comment 2

[Wayne Mery (:wsmwk)]

how will this "facilitate building the palm sync extension"?

Comment 3

[Scott MacGregor]

Attachment: make palm sync an install.rdf based extension

Changes to make us build an install.rdf based extension for palm sync, and to automate the building of the extension.

1) The build system automatically generates a palmsync-*version*.xpi file in dist\xpi-stage now. That file can be hosted on addons.mozilla.org once we fix the rest of the issues listed below.

2) I added entries to the removed-files list so users who install Thunderbird 2.0 will get the old files living in the application directory cleaned up and deleted.

Problems:

1) We need to find a way to call PalmSyncInstall.exe after the extenion is installed
2) We need to find a way to call PalmSyncInstall.exe /u when the user trys to disable or uninstall the extension. 

I don't know how we can make those two things happen with the extension manager.

3) Palm desktop is old! They have a 110 character limit on the directory name containing our conduit dll (mozABConduit.dll). If the path to the dll is longer than 110 characters, it fails to acknowledge the DLL as a valid conduit. This dll now lives in your user profile extension directory with this change because it's a real extension now. However that path alone on windows takes up almost 100 characters. That only leaves ~10 characters for your user name in the profile dir and the name of the extension (usually [e-mail]). For now, I made the name of the extenion be t@t as that was the shortest string I could think of. But users with long profile names will clearly run into this issue.

Comment 4

[Scott MacGregor]

Attachment: palmsync dll changes to support calling PalmSyncInstall.exe

changes to the xpcom component palmsync dll to call PalmSyncInstall.exe the first time the app is run after installing the extension. When EM gives us an uninstall notification, we clear our conduit registration pref so we'll re-run PalmSyncInstall.exe the next time the extension is installed. 

Code clean up for some observer notification (we were adding ourselves as observers to the same topics multiple times) and category registration code (we were never unregistering).

Comment 5

[Scott MacGregor]

Attachment: experimental XPI

Here's an XPI built on the code in the patches above.

Please don't point the mozillazine thunderbird palm community at this. It needs more testing.

Just drag the XPI into the thunderbird extension manager. You should see it act like a regular extension which you can later uninstall.

Comment 6

[David :Bienvenu]

Comment on attachment 209032 [details] [diff] [review]
palmsync dll changes to support calling PalmSyncInstall.exe

cool, Scott.

Comment 7

[Scott MacGregor]

FYI, I let the extension name / guid be as small as possible: "p@m" (short for [e-mail]) in order to minimize the number of users who won't be able to run this extension because the directory path for the extension files is too long for the palm desktop app to handle. I'm not sure there's anything we can do here other than pester Palm.

Comment 8

[Scott MacGregor]

Attachment: changes to call the uninstaller and to stop shipping CondMgr and HSAPI

Some of these changes are things you've already seen.

1) Call PalmSyncInstall.exe /us (silent uninstall) when we receive the uninstall notifcation from the extension manager. This causes us to unregister ourselves with hot sync when the extension is uninstalled.

2) PalmSyncInstall changes to stop looking for a local version of CondMgr and HSAPI.dll if we can't find the Palm Desktop version of those files. 

3) Stop putting the forementioned DLLS into the palm sync XPI.

4) Changed the wording of one of the install dialog strings based on feedback from Gordon.

Comment 9

[Scott MacGregor]

Attachment: updated version of the extension

Comment 10

[Scott MacGregor]

I've landed the build changes on the trunk and the thunderbird 2.0 branch so we can deal with further tweaks as incremental changes.

I'd still like to figure out a better solution to the long path name issue for palm desktop. And there might be some further install/uninstallation steps we might have to hook up.

Leaving the bug open for further tweaks and until the extension actually shows up on addons.mozilla.org.

Comment 11

[Ian Neal]

Latest checkin seems to have broken SeaMonkey 1.8 Branch builds (and will break trunk builds once PalmSync SDK is re-enabled)
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla1.8-SeaMonkey/1138068120.6317.gz

Comment 12

[Scott MacGregor]

I wasn't aware anyone was building this. Sorry.

Comment 13

[Kevin Teuscher]

(In reply to comment #7)
> FYI, I let the extension name / guid be as small as possible: "p@m" (short for
> [e-mail]) in order to minimize the number of users who won't be
> able to run this extension because the directory path for the extension files
> is too long for the palm desktop app to handle. I'm not sure there's anything
> we can do here other than pester Palm.
> 

Could you use the capability to use the registry in window to point at a directory other than the profile directory (e.g. the Palm Desktop directory) and install the extension in that location?

Kevin

Comment 14

[Wayne Mery (:wsmwk)]

install on a windows 98SE PC that never had TB, Palm Desktop and palmsync installed. result after TB restart:

"Failed to register the Thinderbird Palm Sync Support Proxy Dll while installing the Thunderbird Address Book Palm Scyn Conduit."

similar to bug 214407 comment 172 (he has since resolved his problem) so not a new problem obviously.

uninstalled and reinstalled a couple times with same results.

Comment 15

[Gordon Dickens]

Hey Scott,

IMHO, the 110 character pathname limitation may be a showstopper. Can we put the extention here instead?:

C:\Program Files\Mozilla Thunderbird\extensions\p@m\

As an example, it appears that palmsync-1.5.xpi will not install on one of the computers that I use to test your Palm Sync pre-release versions because of this limitation. 

Thanks,

Gordon

Comment 16

[Wayne Mery (:wsmwk)]

(In reply to comment #3)
> 
> 3) Palm desktop is old! They have a 110 character limit on the directory name
> containing our conduit dll (mozABConduit.dll). If the path to the dll is longer
> than 110 characters, it fails to acknowledge the DLL as a valid conduit. This
> dll now lives in your user profile extension directory with this change because
> it's a real extension now. However that path alone on windows takes up almost
> 100 characters. 

Scott - what is the field/structure name(s) and your reference?  

from Table A.1. at http://www.palmos.com/dev/support/docs/conduits/win/Intro_CfgEntries.html

"Conduit [Config. Entry]
Name of the conduit DLL file. If this entry is only a filename, the DLL must be in the HotSync Manager executable's directory *or in the current Windows user's PATH*. If it is a path and filename, you can put the DLL in any directory."

Is it possible to use PATH?


And from http://www.codeproject.com/ce/PalmOS_II.asp something which bears on the issue of versions:

"But wait! Palm has CDK 6.0.1. Isn't that newer and better?

Yes and no. CDK 6.0.1 provides minor updates on the generic conduit framework, integrates its help into the MSDN Library, supports system and user conduits, as well as has support for Palm OS 5.0 and up. The development tools are backwards compatible with Palm OS 3.5, except the generic conduit framework. Unfortunately for us, the framework was redesigned to target the extended database API, which Palm OS 3.5 does not support (only classic databases). This is likely the reason why Palm has kept the older 4.0.3 release online for download."

Comment 17

[Wayne Mery (:wsmwk)]

Scott, what code changed to eliminate need for contacts sidebar?

Comment 18

[Scott MacGregor]

(In reply to comment #13)
> (In reply to comment #7)
> > FYI, I let the extension name / guid be as small as possible: "p@m" (short for
> > [e-mail]) in order to minimize the number of users who won't be
> > able to run this extension because the directory path for the extension files
> > is too long for the palm desktop app to handle. I'm not sure there's anything
> > we can do here other than pester Palm.
> > 
> 
> Could you use the capability to use the registry in window to point at a
> directory other than the profile directory (e.g. the Palm Desktop directory)
> and install the extension in that location?
> 
> Kevin
> 

I thought that capability refers to having fx/tb auto discover XPI files in other install locations by looking them up in the windows registry. I don't think that means you can actually control the directory it gets installed to, but maybe I'm wrong on that front.

Comment 19

[Kevin Teuscher]

(In reply to comment #18)
> I thought that capability refers to having fx/tb auto discover XPI files in
> other install locations by looking them up in the windows registry. I don't
> think that means you can actually control the directory it gets installed to,
> but maybe I'm wrong on that front.
> 
Actually it allows you to expand the XPI in a location and specify the location in the registry.  At the next start-up the program will recognize the extension and use it from the location noted in the registry as if it were installed in the profile extension location

For example,
1. Grab this zip file: http://mozilla.doslash.org/stuff/helloworld.zip
2. unzip it to: "c:\temp" (keeping the directory structure of the zip)
3. Create the key HKCU\Software\Mozilla\Firefox\Extensions
4. Create the value (REG_SZ): [e-mail]
5. Set the value to: 'c:\temp\helloworld'
6. Start Firefox and see that you have the "Hello World" extension installed


I would think that we could use this and just install the palmsync extension in C:\Palm or c:\Program Files\PalmOne and be able to get around the path name length limitations.

Kevin

Comment 20

[Scott MacGregor]

That's an interesting idea Kevin. I'm not sure how an extension would do that though. For an extension to be up on addons.mozilla.org, it needs to be an install.rdf based extension. Your install.rdf would have to install a dummy extension perhaps which installed some JS the next time the product was launched which in turn installed the real palm sync extension and added the registry keys. Then you'd have to restart again to get the palm sync extension installed. I think in that scenario you'd also end up with two extensions in the extension manager: one for your bootstrap extension and the other one for the real palm sync extension. 

Not ruling this idea out, I'm just not sure how it would fit together. 

Comment 21

[Kevin Teuscher]

Another approach vector that could be taken is to install the sync conduit and as part of the install, add the registry entries and extension file.  That way you could distribute an executable to do all of this.  You already have to run an executable after installing the extension in order to install the conduit, so why not just reverse the install vector.

The drawback is that you can't distribute from amo, but maybe you could have the install link go to the TB product page where you have an "extras" section in the downloads.

Kevin

Comment 22

[Wayne Mery (:wsmwk)]

Scott, did you do any work with both VC8 and CDK 4.0.3?

Comment 24

[Wayne Mery (:wsmwk)]

At least one tester got burned by the 110 character limit, demonstrating a release note is needed.  The restriction is profile-name + windows-user-name <= 20 characters.  

Can palmsync's install process test if it's attempting to install into a directory name that is too long?

On the issue of workarounds I tested global install with 
  thunderbird -install-global-extension palmsync.xpi 
to put the extension into the program directory instead of profiles, noted at
http://kb.mozillazine.org/Installing_extensions#Global_installation
It works but the down side is you can't use the addons UI to install.

In investigated a dead end - hoping it might be possible to install into /documents and settings/<user>/application data/Thunderbird/Extensions but Mossop tells me that support for this dir was removed (for firefox anyways) Feb 2006


Scott, David, Vista needs to be tested (I forgot to mention this a few weeks ago).  Bug 374151 mentions a crash but details are lacking.  I'll try to get more info into the bug, but as of now I don't have Vista and don't have anyone to test it.

Comment 25

[Gordon Dickens]

(In reply to comment #24)
> At least one tester got burned by the 110 character limit, demonstrating a
> release note is needed.  The restriction is profile-name + windows-user-name <=
> 20 characters.
>
> Can palmsync's install process test if it's attempting to 
> install into a directory name that is too long?

Beware that modifying the profile name in the user profile interface is not adequate since that does not actually change the profile folder name at: C:\Documents and Settings\<user name>\Application Data\Thunderbird\Profiles\<profile folder name>.  The user will need to create a new profile that meets the 110 character limitation and then manually copy their files from their old profile to a the new profile.  I believe that a procedure should be written for creating a profile that meets the 110 character limit and for the migration of data to the new profile.  I agree with Wayne that we also need to have PalmSync's install process detect if the 110 character limit is not being met and then report back to the user accordingly.

Comment 26

[Mark Banner (:standard8)]

(In reply to comment #24)
> At least one tester got burned by the 110 character limit, demonstrating a
> release note is needed.  The restriction is profile-name + windows-user-name <=
> 20 characters.  

I don't know which bit of code is used to set the directory name, but is it possible we could use the short folder name/path for palm sync? nsILocalFileWin supports this: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsILocalFileWin.idl

Comment 27

[timeless]

there's no guarantee that short paths exist. NT isn't required to offer them. (seriously, it's entirely optional and a system can easily have the feature disabled)

Comment 28

[Wayne Mery (:wsmwk)]

(In reply to comment #27)
> there's no guarantee that short paths exist. NT isn't required to offer them.

but by default systems come with it turned on, no?

does this describe the mechanism you are referring to?  http://support.microsoft.com/kb/121007/


Does vista use the exact same directory name - documents and settings?

Comment 29

[Wayne Mery (:wsmwk)]

Attachment: 8.3 path

I tested a short path name of C:\DOCUME~1\<username>\APPLIC~1\... and it works fine (not surprising).  It gives us almost 20 additional characters and might be a way out of this predicament.  

I suspect the current limitation 20 characters is unrealistic and going to get us burned with someone or some company that has standards where the user name easily exceed 15 characters.

The question is whose executable code sets the path name, ours or palm's?

FWIW, here is a palm registry entry that shows an 8.3 name but perhaps that we set by windows and not palm.

Comment 30

[Wayne Mery (:wsmwk)]

Scott, what's your opinion on using a short path name?  Are there issues that still need to be addressed?

The choices so far are
* find another install location (like program directory)
* document a restriction on length of username+profilename 
* 8.3 pathname

I googled and found very few references that talk about it let alone recommend disabling it (a handfull) - all in the context of using products that create hundreds to thousands of files in a directory.  I think we are more likely to encounter people with user names long enough to fail installation than people who have disabled 8.3.

Comment 31

[David :Bienvenu]

I wonder if the Palm stuff itself works without 8.3 file names - it's possible it doesn't, in which case this would be a no brainer...

Comment 32

[Worcester12345]

(In reply to comment #31)
> I wonder if the Palm stuff itself works without 8.3 file names - it's possible
> it doesn't, in which case this would be a no brainer...
> 
Has anyone bothered asking Palm any of this? I guess it wouldn't hurt for a developer to pick up the phone. It would be mutually beneficial in fact.

Comment 33

[Wayne Mery (:wsmwk)]

max windows XP username length is 20 characters
mozilla profile name appears to have no imposed max length (I tested ~70 char)

possible solution incorporating 8.3:
get short path
if failure, get normal path
if path length > max, issue error and exit
[register conduit]

David, can you do the patch? 

Comment 34

[Scott MacGregor]

if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't work that way already!

Comment 35

[Wayne Mery (:wsmwk)]

apparently names in vista are c:\username\... with symbolic names (junctions) that map to "Documents and Settings"\username  to help software that still expects files to be at the "old" locations.  Reports differ however about how this is implemented, for example http://www.devsource.com/article2/0,1895,1999637,00.asp and http://4sysops.com/archives/documents-and-settings-in-vista/ 

I did not find information about whether 8.3 name capability changed in vista.

Comment 36

[Mark Banner (:standard8)]

(In reply to comment #34)
> if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't
> work that way already!

Note that canonicalPath won't force 8.3 paths, but it will use them where possible.

Comment 37

[Wayne Mery (:wsmwk)]

Scott in comment #34:
> if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't
> work that way already!

Path is determined here, set to where PalmSyncInstall.exe is running from
http://lxr.mozilla.org/seamonkey/source/xpinstall/wizard/windows/palmsync/PalmSyncInstall.cpp#148

I got positive results today with long path name. I tested trunk suiterunner xpi Mark gave me, installed into a path with 192 characters, with seamonkey 1.1.1 and it ran fine.  palm HSM is version 4.1  

We don't have documented under what conditions the sync with >110 path failed.  Does anyone remember whether it's HSM can't call palmsync.dll, or palmsync.dll fails after being started?  Or have an older version of HSM/palm desktop that can be tested?

Comment 38

[Wayne Mery (:wsmwk)]

Mark, the last xpi for bug 378723 worked but I forgot to mention it throws up a small dialog box 
   Test [window title]
   results: ##"
where ## appears to be a return code from somewhere for both successful and unsuccessful installs. never seen it before and I don't see where it comes from in the source. Does not happen during uninstall. 

David, in addition to fixing the issue above, what should happen before the 8.3 path portion of the Mark's patch can be checked in to branch?  

David/Scott - is dist\xpi-stage mentioned in comment 3 a publicly accessible directory? 

Comment 39

[Mark Banner (:standard8)]

(In reply to comment #38)
> Mark, the last xpi for bug 378723 worked but I forgot to mention it throws up a
> small dialog box 
>    Test [window title]
>    results: ##"
> where ## appears to be a return code from somewhere for both successful and
> unsuccessful installs. never seen it before and I don't see where it comes from
> in the source. Does not happen during uninstall. 

Can you try the latest suiterunner build version (ftp://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/tpol-trunk/)? I think this is just some debug code I've got hanging around in my build. If it works (i.e. not the 50% problem) then its probably the right version - I think ftp.mozilla.org has the latest, if not wait a while and try again ;-)

You'll still need to change the directory name.

Comment 40

[Mark Banner (:standard8)]

(In reply to comment #38)
> David/Scott - is dist\xpi-stage mentioned in comment 3 a publicly accessible
> directory? 

dist\xpi-stage is a directory created as part of a build. Its not on a ftp server if thats what you were thinking.

Comment 41

[Mark Banner (:standard8)]

Attachment: Change installation path to match extension id.

This path changes the INSTALL_EXTENSION_ID variable to match that in the RDF file. This means when we build it in tree (developer or suiterunner) it'll go into extensions/ in the same directory as the id in the rdf file so it'll register properly as an extension when the app runs up.

Note that I changed install.rdf to use INSTALL_EXTENSION_ID in case we ever decide to change it in future...

Comment 42

[Mark Banner (:standard8)]

(In reply to comment #41)
> Created an attachment (id=263381) [details]
> Change installation path to match extension id.

I've just checked this patch in.

Comment 43

[Wayne Mery (:wsmwk)]

David, Scott, should all of attachment 209150 [details] [diff] [review] and attachment 209032 [details] [diff] [review] be on branch?

Comment 44

[Wayne Mery (:wsmwk)]

Attachment: 8.3 patch adapted from bug 378723

adapted for branch from mark's trunk patches in bug 378723 - attachment 263024 [details] [diff] [review] and attachment 263211 [details] [diff] [review]

Mark, does it look like I got the right pieces?

David, I don't have branch source or build so I don't have the means to verify correctness of source nor ability to compile

Comment 45

[Mark Banner (:standard8)]

(In reply to comment #44)
> Created an attachment (id=266394) [details]

> Mark, does it look like I got the right pieces?

Well it looks a little strange as I think you've knitted them together, but it does look like you've got the right bits.

Comment 46

[Wayne Mery (:wsmwk)]

Attachment: point to palmsync kb article

David, will also need checkin branch and trunk

Comment 47

[Wayne Mery (:wsmwk)]

Comment on attachment 266927 [details] [diff] [review]
point to palmsync kb article

patch flag for install.rdf mozilla\mailnews\extensions\palmsync

Comment 48

[David :Bienvenu]

this patch is mal-formed - you can't just add the two diffs together; you need to merge the lines that changed into one diff. I'll try to figure out what specifically needs to change, though I'll be flying blind a bit.

Comment 49

[Wayne Mery (:wsmwk)]

Attachment: 8.3 patch v2 for branch (adapted from bug 378723)

looks better. hope it applies

Comment 50

[David :Bienvenu]

Comment on attachment 266946 [details] [diff] [review]
8.3 patch v2 for branch (adapted from bug 378723)

applies and builds, thanks! I'll try package up an xpi for you asap.

Comment 51

[Wayne Mery (:wsmwk)]

(In reply to comment #47)
> (From update of attachment 266927 [details] [diff] [review])
> patch flag for install.rdf mozilla\mailnews\extensions\palmsync

David, attachment 266927 [details] [diff] [review] is for trunk.
For branch, I don't know where the file is - don't see it in mozilla\mailnews\extensions\palmsync nor in xpinstall/wizard/windows/palmsync/.

Comment 52

[Wayne Mery (:wsmwk)]

Scott in comment #3:
> Created an attachment (id=208814) [details]
> make palm sync an install.rdf based extension 
> 
> Changes to make us build an install.rdf based extension for palm sync, and to
> automate the building of the extension.
> 
> 1) The build system automatically generates a palmsync-*version*.xpi file in
> dist\xpi-stage now. That file can be hosted on addons.mozilla.org once we fix
> the rest of the issues listed below.

Mark, David, 

presently the extension version is keyed to gecko, ala 1.8.x or 1.9.x. Would it not make sense for it to be keyed to the version of thunderbird or seamonkey by  changing the last line of 
 XULAPP_DEFINES = \
         -DINSTALL_EXTENSION_ID=$(INSTALL_EXTENSION_ID) \
         -DTHUNDERBIRD_VERSION=$(THUNDERBIRD_VERSION) \
         -DSEAMONKEY_VERSION=$(SEAMONKEY_VERSION) \
         -DEXTENSION_VERSION=$(MOZILLA_VERSION) \

to 
ifdef MOZ_XUL_APP
         -DEXTENSION_VERSION=$(SEAMONKEY_VERSION) \
else
         -DEXTENSION_VERSION=$(THUNDERBIRD_VERSION) \

Or, a better alternative?

Comment 53

[Mark Banner (:standard8)]

(In reply to comment #52)
> presently the extension version is keyed to gecko, ala 1.8.x or 1.9.x. Would it
> not make sense for it to be keyed to the version of thunderbird or seamonkey by
>  changing the last line of 
...
> Or, a better alternative?

Given that the extension dependencies are such that it appears to need to be built for each specific version of tree, I think that linking to the gecko version isn't too bad (and we certainly do that with some of the other in-tree extensions).  However if you did change it, then I'd suggest just sticking in a constant that we can increment when we think it appropriate.

Comment 54

[Wayne Mery (:wsmwk)]

(In reply to comment #53)
>> Given that the extension dependencies are such that it appears to need to be
> built for each specific version of tree, I think that linking to the gecko
> version isn't too bad (and we certainly do that with some of the other in-tree
> extensions).  

what example(s) should I look at?  

I should have elaborated on my concerns that prompted the question: 
a) version was not previously visible in the extension manager, so no one has seen it before and we can do whatever we want.  
b) almost everyone who sees the new xpi now asks me "why is the version 1.8.x?" (including Dave) 
c) min+max version checks are tied to the application version, not the tree version.
d) the build process names the xpi with the application version


> However if you did change it, then I'd suggest just sticking in a
> constant that we can increment when we think it appropriate.

Is this relevant? ... Scott comments in the makefile
 # We should really pull THUNDERBIRD_VERSION from
 # mail/config/version.txt but we can't be assured
 # that we've even pulled that file. So we hardcode them.

I have no idea why.

Comment 55

[Robert Kaiser]

That comment is bogus nowadays.

Anyways, I think using the Gecko version is really correct. Version 1.0 of the PalmSync extension was shipped with Mozilla 1.0, even though it may not have been publicly visible. And versioning the same extension differently in SeaMonkey and Thunderbird and maybe Penelope is, forgive me, bullshit. DOMI is also using the Gecko version, and nobody complains about that there.

Comment 56

[Wayne Mery (:wsmwk)]

(In reply to comment #55)
> That comment is bogus nowadays.

you mean about mail/config/version.txt ?


> Anyways, I think using the Gecko version is really correct. Version 1.0 of the
> PalmSync extension was shipped with Mozilla 1.0, even though it may not have
> been publicly visible. And versioning the same extension differently in
> SeaMonkey and Thunderbird and maybe Penelope is, forgive me, bullshit. DOMI is
> also using the Gecko version, and nobody complains about that there.

Can the extension installation process check max/min version against gecko?
If not, then I submit versioning based on gecko doesn't make sense from that viewpoint alone.

Gecko version may be correct from tech viewepoint, but it has it's problems - at least for Thunderbird. And unlike suite/seamonkey  the distribution location at http://ftp.mozilla.org/pub/mozilla.org/thunderbird/extensions/palmsync/ has clearly linked the extension to a specific version of thunderbird. 

But your comment goes to another issue which has not been discussed - is it possible to have one extension for both seamonkey (or suiterunner) and thunderbird. i.e. should there be a convergence and just one version of palmsync for both suite and thunderbird? 

Comment 57

[Robert Kaiser]

(In reply to comment #56)
> (In reply to comment #55)
> > That comment is bogus nowadays.
> 
> you mean about mail/config/version.txt ?

Did you see it actually hardcoded there? I don't think so, it uses THUNDERBIRD_VERSION and this var is set by configure, which directly sets it from mail/config/version.txt and client.mk ensures we pull it.
So, actually, about everything the comment is saying is wrong nowadays.


> Can the extension installation process check max/min version against gecko?

That doesn't matter (but actually, work is going on to support that). This is the version of the palmsync extension, not of whatever app it is installed in.

> should there be a convergence and just one version of
> palmsync for both suite and thunderbird? 

I didn't know there even might be different versions for the two apps. We're both using exactly the same code, right?

Oh, and the distribution  location is wrong anyways if it is on ftp.m.o as nothing that is only on ftp.m.o should be official releases, from what I know, as releases.m.o or download.m.o have much better load balancing and mirroring. Anyways, an extension is an add-on and belongs on AMO. And there, it can quite easily be marked to support both Thunderbird and SeaMonkey and show up for both, btw.

Comment 58

[Mark Banner (:standard8)]

(In reply to comment #57)
> (In reply to comment #56)
> > should there be a convergence and just one version of
> > palmsync for both suite and thunderbird? 
> I didn't know there even might be different versions for the two apps. We're
> both using exactly the same code, right?

The extension did have a time of being slightly different code, but now we're both XUL apps, the code base is common again (I specifically made it to be).

My only concern here currently is that the palm sync code is at the moment being linked directly against the mailnews shared libraries because it is using class implementations from within mailnews/addrbook. Therefore if you compile it against Thunderbird for instance, I'm not sure how reliable/sensible it would be to use against SeaMonkey because essentially it'll be a slightly different shared library that its calling up. Possibly its not too much of an issue if we're using exactly the same code base (which is what we do on stable branches) and tie it to specific versions of the code.

IMHO this is really something we should fix, though it will possibly make the palm sync extension bigger/more complex. Fixing this would mean that we'd only need to update the extension when particular AB interfaces changed.


So as for versioning, I think the version should be independent of which app we're building it against. At the moment I don't think there is anything wrong with tying it to the gecko version, as you generally need a specific code base to run it against. We could if we want use specific/manually incrementing version numbers, but I suggest that if we did then we should definitely fix the above issue with the linking.

Also remember that the version number is independent of the app version that it gets installed into - the app version is restricted independently by install.rdf. We don't have to limit it to one specific version of the app, just at the moment its better to. 

As for shipping I think SeaMonkey is happy to continue shipping with the installer. Though, publishing the extension to AMO would be a good idea (again once we fix the lib issues) - Thunderbird users will be able to install and download it, SeaMonkey users would be able to pick up updates if we did some feature improvements.

Comment 59

[Wayne Mery (:wsmwk)]

(In reply to comment #55)
> DOMI is also using the Gecko version, and nobody complains about that there.

the major difference is DOMI is a technical tool and anyone who uses DOMI clearly understands it operates on gekco related elements. But what average user would expect an extension like palmsync, which is address book oriented, to have no dependency on the "application" and be strictly dependent to back-end code? 


For my own education, what other classic extensions are dependent only on gecko?

(In reply to comment #57)
>Did you see it actually hardcoded there? I don't think so, it uses
>...

I know little of the history of palmsync and nothing of the build process nor how it operates, so thanks for the info.


>> Can the extension installation process check max/min version against gecko?
>
>That doesn't matter (but actually, work is going on to support that). This is
>the version of the palmsync extension, not of whatever app it is installed in.

it matters to me if we move toward a common extension.


(In reply to comment #58)
>However if you did change it, then I'd suggest just sticking in a
>constant that we can increment when we think it appropriate.
(In reply to comment #58)
>We could if we want use specific/manually incrementing version numbers, but I 
>suggest that if we did then we should definitely fix the above issue with the
>linking.

A palmsync version that is independent of both gecko and the application may be a good way to go.  Users are accustomed to this with most other extensions. 

What is needed for the linking issue?  An API?

OTH, if http://wiki.mozilla.org/Mozilla2:Device_Sync is delivered within 1-2 years it might not be worth the effort.

Comment 60

[Robert Kaiser]

Wayne, your arguments contradict each other:
First, so say that we should use the same version as the app, as the user expects that. A few lines later, you say a version independent of both gecko and the app is what users are accustomed to.

So why can the version independent of the app just happen to be the version that "accidentally" is also used by Gecko?

IMHO, that's as good as a completely independent version but has the good side that no developer needs to explicitely care about bumping it for new versions, as that happens automatically.

Comment 61

[Wayne Mery (:wsmwk)]

you're right I may have contradicted myself :)

So there needs to be two packages - one for Thunderbird and one for Suite. And there is a difference of opinion regarding version numbers. 

New info regarding version numbering - sdwilsh and others on #addons state AMO "updates are done based on version number alone".  So if palmsync is tied to gecko version then we can't push a palmsync update, be it fixes or enhancements, without someone else changing the gecko version. Not a good dependency in my opinion.  So versioning based on gecko I think need to be reconsidered in this new light.

More details about versioning and updating at http://developer.mozilla.org/en/docs/Extension_Versioning%2C_Update_and_Compatibility#Automatic_Extension_Update_Checking

I wonder too, since hotsync can occur without thunderbird or suite knowing about it, do we need to worry whether thunderbird might update while hotsync is in progress and the conduit is in active use?

Comment 62

[Wayne Mery (:wsmwk)]

(In reply to comment #61)
> I wonder too, since hotsync can occur without thunderbird or suite knowing
> about it, do we need to worry whether thunderbird might update while hotsync is
> in progress and the conduit is in active use?

I'll answer my own dumb question - because an extension update requires a restart probably not - that is, if a user is being smart s/he won't intentionally restart thunderbird while a hotsync is in progress.

Comment 63

[Robert Kaiser]

(In reply to comment #61)
> So there needs to be two packages - one for Thunderbird and one for Suite.

We don't.

> New info regarding version numbering

No new info for people like me and Mark, but maybe for you ;-)
Anyways, this even more means that we can't go with the SeaMonkey/Thunderbird version numbers but need to do a separate one. Still, we could spare that and go with the Gecko number if we agree that changes happen rarely enough that this is sufficient.

Comment 64

[David :Bienvenu]

Comment on attachment 266927 [details] [diff] [review]
point to palmsync kb article

landed on trunk and 1.8.1 branch

Comment 65

[Wayne Mery (:wsmwk)]

"keep on top" needs to be added to the palmsync install dialog yes/no prompt so that thunderbird's/seamonkey's restart doesn't cover it. so need to add MB_TOPMOST ~line 201 of palmsyncinstall.cpp 
     if (MessageBox(NULL, msgStr, appTitle, MB_YESNO) == IDYES) 

Comment 66

[Wayne Mery (:wsmwk)]

Attachment: Add MB_TOPMOST to MessageBox

follow up to comment 65.  Please checkin to branch and trunk after approval.

  if (MessageBox(NULL, msgStr, appTitle, MB_YESNO | MB_TOPMOST) == IDYES)
to keep palmsyncinstall.exe dialog box on top so it is visible to user after thunderbird restarts following extension installation via addons.

David provided an extension back in June (6/22 iirc) with this modification and it's gotten lots of testing from many people but was never submitted as a patch for checkin.

AFAIK, this is the last patch needed before making the extension public, except perhaps versioning.

Comment 67

[Wayne Mery (:wsmwk)]

Comment on attachment 291942 [details] [diff] [review]
Add MB_TOPMOST to MessageBox

(it didn't keep my review assignment)

Comment 68

[Worcester12345]

Missed the Target Milestone: 	 Thunderbird2.0 , but there is no way to change this field.

Comment 69

[Wayne Mery (:wsmwk)]

Comment on attachment 291942 [details] [diff] [review]
Add MB_TOPMOST to MessageBox

revised patch file coming up

Comment 70

[Wayne Mery (:wsmwk)]

Attachment: Add MB_TOPMOST to MessageBox to prevent install dialog getting hidden

Previous patch attachment 291942 [details] [diff] [review] never made it to branch or trunk afaik.  We will want checkin of this patch (NPOTB) to both before building extension from those sources for the purpose of pushing Thunderbird extension to AMO. See comment 65 and 66

David, regarding review - the original line 
  if (MessageBox(NULL, msgStr, appTitle, MB_YESNO) == IDYES)
appears twice in the program. This patch touches only the install code. uninstall code is left untouched because afaict MB_TOPMOST is not required for proper uninstall behavior.  However, consider in review whether we should be symmetric, i.e. that uninstall should also be changed.  


Also, some incomplete business about Vista - it never got good testing - I'm awaiting feedback about what works on Vista (ref: bug 422494 - unclear if problem is palm's vendor or palmsync's)

Comment 71

[David :Bienvenu]

adding checkin-needed, both branch and trunk, per Wayne's prev comment.

Comment 72

[Mark Banner (:standard8)]

(In reply to comment #70)
> Created an attachment (id=320988) [details]
> Add MB_TOPMOST to MessageBox to prevent install dialog getting hidden

Checking in mailnews/extensions/palmsync/installer/PalmSyncInstall.cpp;
/cvsroot/mozilla/mailnews/extensions/palmsync/installer/PalmSyncInstall.cpp,v  <--  PalmSyncInstall.cpp
new revision: 1.14; previous revision: 1.13

David, can we check this in on branch as NPOTB? If so, I'll do it tomorrow.

Comment 73

[David :Bienvenu]

yes, that's fine, as long as you make that clear in the checkin comment - afaik, they're not doing any tagging or building on the branch.

Comment 74

[Mark Banner (:standard8)]

(In reply to comment #72)
> (In reply to comment #70)
> > Created an attachment (id=320988) [details] [details]
> > Add MB_TOPMOST to MessageBox to prevent install dialog getting hidden

Checked in on branch:

Checking in PalmSyncInstall.cpp;
/cvsroot/mozilla/xpinstall/wizard/windows/palmsync/Attic/PalmSyncInstall.cpp,v  <--  PalmSyncInstall.cpp
new revision: 1.9.18.2; previous revision: 1.9.18.1
done

with a=KaiRo over irc.

Comment 75

[Mark Banner (:standard8)]

Is there anything else we need to do on this bug now or can it be closed? AFAIK the build mechanism works, and other issues should be covered by other bugs.

Comment 76

[Wayne Mery (:wsmwk)]

Scott and David have longer history here and can better comment about closing, but from my perspective the important open issues are covered in other bugs.  

If the thinking is to push the extension to AMO after this is closed, one relnote is appropriate about bug 422494 / problems with HSM 7, palm desktop 6.

Comment 77

[Mark Banner (:standard8)]

(In reply to comment #76)
> Scott and David have longer history here and can better comment about closing,
> but from my perspective the important open issues are covered in other bugs.  
> 
> If the thinking is to push the extension to AMO after this is closed, one
> relnote is appropriate about bug 422494 / problems with HSM 7, palm desktop 6.
> 
I believe pushing the extension to AMO is going to be a manual process (although the xpi could be done dynamically). Therefore I think we can close this bug now. Its already done a lot.