Closed Bug 144551 Opened 18 years ago Closed 17 years ago
inconsistent en-mac/en-unix in different platform langenus
Currently, the en-mac.jar and en-unix.jar are different between different platform langenus.xpi. For example, in win32 langenus.xpi, the en-mac.jar contains 7 files, but in mac langenus.xpi, the en-mac.jar contains 9 files. The xpi packaging needs to pick up those files from same place, to keep them consistent. Otherwise, the langenus.xpi won't be cross-platform.
which version? trunk? branch? I can confirm the problem on the trunk, at least, but don't know if we need to fix this for mozilla1.0 if it's on the branch. Please nominate as appropriate if this is expected to be fixed. Looks like bug 104485 was the culprit, over to Ben (note, this has nothing to do with the XPI packaging, it's the build system that creates the en-<plat>.jar files)
Assignee: dveditz → ben
Status: UNCONFIRMED → NEW
Component: Installer: XPI Packages → XP Apps
Ever confirmed: true
for all, trunk and branch
could you list the file name for each one. thanks
There are actually two problems in the en-mac/unix.jar. 1) In these 2 jar files, there are some inconsistent files between different platforms' langenus.xpi: - en-mac.jar: communicator-platform\*.* are in Mac xpi but not Win xpi; - en-unix.jar: platformCommunicatorOverlay.dtd is in Unix xpi, but not Win xpi. 2) In these 2 jar files, all file format (line breaks) are different between different platforms' langenus.xpi. The first problem could make the language pack (langXXXX.xpi) not really cross-platform. The second problem confused our localization tool, the software think all the files are total different. Ideally, all the en-win/mac/unix.jar should be exactly totally same in all platforms' langenus.xpi, file amount and file format.
I am lost here. Isn't the whole purpose of en-[platform].jar is to intelligently install only needed files on a specific platform? We are supposed to put win-specific files (such as winhook, n2p, etc) in en-win.jar but not the rest (en-mac.jar or en-unix.jar).
regardless installation, en-win/mac/unix.jar should be together in the langenus.xpi, which suppose to be cross-platform. For example, we always post Win32 langXXXX.xpi as language pack for all platforms.
yxia: again, please list the file names here. >communicator-platform\*.* what are *.* here? please list them. so we can look at them.
communicator-platform\*.* means the directory itself and all files inside, contents.rdf and platformCommunicatorOverlay.dtd
> the directory itself and all files inside, could you list all the file inside ?
2 files listed above
nominating it nsbeta1
Nav triage team needs info: how does this affect end users? What is the effect of the missing files? Ben? Ying?
Whiteboard: [need info]
To end user: for example, if a Mac user download a language pack from our web, I think he will see some blank menu items: <!ENTITY closeCmd.label <!ENTITY printSetupCmd.label <!ENTITY quitApplicationCmd.label <!ENTITY redoCmd.label Because we always post win32 language pack on the web, and in that pack, the Mac platformCommunicatorOverlay.dtd is not in the en-mac.jar. I have not try it yet though...
No, missing DTD files to not result in blank strings, they result in blank windows because the XML doesn't validate. missing .properties files might result in the more benign blank strings. DTD's are way too fragile to have been a good choice for localization mechanism. Maybe we need to handle XUL in a Quirks mode that doesn't enforce the rule that all entities must exist :-)
yokoyama- could you please help to address the issue. thanks
what am I suppose to do here? I am kinda new in xpi arena. So by reading comments, we have to do: - add contents.rdf and platformCommunicatorOverlay.dtd into en-mac.jar and en-unix.jar - then update langenus.xpi Is this correct? If so, then do you really need my help? dan, can you own this bug?
This is not a xpi problem. The Xpi process is correctly slurping up exactly what the build process on each platform produces. The problem is that there are now some mac-only and linux-only jar.mn files so that the en-mac etc produced on the different platforms no longer match (ignoring line-ending differences, which don't matter to our parser). You need to make the windows and linux build process look like the mac process for en-mac. You need to make the windows and mac build process look like linux for en-unix, and you need to make the mac and linux build process look like windows for en-win. These all used to be the same, but a few new platform-only jar.mn files were introduced. These should probably get moved back up into the parent jar.mn
Who knows _how_ to do this? I certainly don't. Tao?
Hi, Don - would you please take a look? thx!
assign to don.
Assignee: yokoyama → dbragg
- Use lxr to search for the files in question - find which jar.mn they're in - work up the tree to find the common jar.mn they should be moved to - move the references to the new jar.mn, fixing up the paths as req'd - remove old single-platform jar.mn - remove platform build system references to those jar.mn
marking assigned to indicate work in progress...
Status: NEW → ASSIGNED
Hmm, isn't this more or les a dupe of bug 133849? I filed that one when I found out the inconsistency problem is much bigger than bug 118140 that is still one of the biggest headache for L10n people...
After discussion with dveditz I felt that the best place for the jar references for the platform-specific file was in the "root" jar.mn files. There's already a whole bunch of chrome-related files there so there won't be much of a problem for developers who just want to build chrome. They already had to go to this level anyway. The one exception to this is the winhooks. This is only one file so I added the directory to the unix makefile.in and a CreateFileFromJar entry in the MozillaBuildList.pm
Comment on attachment 86639 [details] [diff] [review] Patch for jar.mn and "make" file mods r=tao
Attachment #86639 - Flags: review+
Comment on attachment 86639 [details] [diff] [review] Patch for jar.mn and "make" file mods Please do another diff with the -N option -- I want to see the obsolete jar.mn files and asssociated makefiles go away -- otherwise people can get suckered into updating the wrong files. Have you tested (or had build buddies test) on all platforms? >Index: mailnews/jar.mn >=================================================================== >+en-win.jar: >+ locale/en-US/messenger-mapi/pref-mailnewsOverlay.dtd (mapi/resources/locale/en-US/pref-mailnewsOverlay.dtd) >+ locale/en-US/messenger-mapi/mapi.properties (mapi/resources/locale/en-US/mapi.properties) >+ locale/en-US/messenger-mapi/contents.rdf (mapi/resources/locale/en-US/contents.rdf) I don't see these removed from anywhere else. Will these get processed twice on windows? At a quick glance looks like you could remove "locale" from DIRS in both makefiles in mozilla/mailnews/mapi/resources/, and then remove all the makefiles in locales and below. >Index: xpfe/communicator/jar.mn >=================================================================== >+ >+en-win.jar: >+ locale/en-US/communicator-platform/contents.rdf (win/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (win/platformCommunicatorOverlay.dtd) >+ >+en-mac.jar: >+ locale/en-US/communicator-platform/contents.rdf (mac/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (mac/platformCommunicatorOverlay.dtd) >+ >+en-unix.jar: >+ locale/en-US/communicator-platform/contents.rdf (unix/contents-platform.rdf) >+ locale/en-US/communicator-platform/platformCommunicatorOverlay.dtd (unix/platformCommunicatorOverlay.dtd) These too, where do they come out of, and what makefile entries now need to be removed? There's a mozilla/xpfe/communicator/resources/locale/en-US/os2 directory, but according to the en-US Makefile.in the os2 folks use the win versions. Should we remove the os2 stuff to avoid confusion? Leave them but snip the makefile stuff up to the resources dir level (because you have to leave the content stuff platform-specific). >Index: xpfe/components/Makefile.in >=================================================================== >-DIRS = bookmarks directory download-manager filepicker find history search sidebar related regviewer xfer prefwindow shistory timebomb console autocomplete updates urlbarhistory intl resetPref killAll >+DIRS = bookmarks directory download-manager filepicker find history search sidebar related regviewer xfer prefwindow shistory timebomb console autocomplete updates urlbarhistory intl resetPref killAll winhooks > > ifeq ($(OS_ARCH),WINNT) >-DIRS += winhooks urlwidget alerts >+DIRS += urlwidget alerts > endif I don't see any mods to the winhooks Makefile.in, which up to now has assumed only win folks get there (thus my question above about platform build buddies). Wouldn't it be better to zap the winhooks/jar.mn and just move the lines up to mozilla/xpfe/components/jar.mn? >Index: xpfe/global/jar.mn Same thing, you've added stuff here but not removed the old mechanism. We'll eventually end up with the same problem again only harder to diagnose because it'll look like we've done the right thing. xpfe/browser appears to have the same problematic structure, although at a quick glance at least win and linux traverse through all three platform directories. I guess if that part's working we shouldn't mess with it on the branch, but I'd like a trunk patch to deal with it.
Attachment #86639 - Attachment is obsolete: true
This diff shows the removed files.
Dan made a good suggestion about the xpfe/components/winhooks dir. It would be a can of worms to open up that tree to Unix. It's better to just move the one file that was listed in the jar.mn to the xpfe/components' jar.mn file. I didn't have that in the last diff. This also removes one modification to the Mac's MozillaBuildList.pm file. (a good thing)
Attachment #86890 - Attachment is obsolete: true
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion r=tao
Attachment #86897 - Flags: review+
Attachment #86897 - Flags: superreview+
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion looks good
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion oops, sr=dveditz
new owner ->tao
Assignee: dbragg → tao
Status: ASSIGNED → NEW
Comment on attachment 86897 [details] [diff] [review] New -N diff with mod to xpfe/components per Dan's suggestion The new chrome regsitry system has the side-effect to this patch; en-mac.jar becomes the provider on win/unix. We need to find a place to explicitly designate the chrome provider.
Attachment #86897 - Flags: needs-work+
That problem is a build system artifact, the installed versions don't have that problem. Therefore it can be fixed by the build system. Figure out whether it's the first-registered or last registered that takes precedence (I'm guessing first). Order the jar.mn such that mac will get registered. Then in the windows and gmake scripts echo the appropriate registration line into installed-chrome.txt explicitly such that the correct platform is registered instead. Remember that the gmake system also handles win(and os2) and Mac os x so you'll have to ifdef which platform gets registered there.
Status: NEW → ASSIGNED
Assignee: tao → jbetak
Status: ASSIGNED → NEW
tao, dan: if en-mac.jar is used as a chrome provider, it would show up in installed- chrome.txt, correct? I tried to verify that claim on my current gmake win32 build and two Mac builds (old-style and Fizilla OS X), but it doesn't seem to be the case. However, it seems like en-mac.jar is *still* out of sync. I'm seeing platformglobaloverlay.dtd in Mac builds - it's missing on win32 builds...
Status: NEW → ASSIGNED
Hi, Juraj: In a win32 or linux buid, we don't use en-mac.jar. Therefore, you shouldn't see it registered on non-Mac build. For classic OS9 build, you are right that en-mac.jar should be registered. Are you referring to a build with the patch http://bugzilla.mozilla.org/attachment.cgi?id=86897&action=view ?
Hi Tao, I just applied the patch, realizing that your comment might have been related to its effects, and not the current state of the build scripts :-)
dveditz: I've updated dbragg's patch and and opted to modify add-chrome.pl and make-jars.pl instead of "echoing" the correct platform jar into installed-chrome.txt to override previous entries. Do you think we could take such a change into the tree?
Attachment #86897 - Attachment is obsolete: true
jbetak: I can't help with r= as you requested in bug 118140 Comment #39 because I don't know the original code good enough to really check what the patch does in detail. Sorry. Can't await having this all fixed though.
Verification builds look good on all platforms. Tao idicated we should run this by sfraser and seawood - cc'ing them for review. Could we get this into Moz 1.2a?
Comment on attachment 95168 [details] [diff] [review] updated patch, resolves the incorrect chrome provider issue raised by tao I'm not sure who the right person to review this behavior change is since warren left. My guess would be one of the xpapps guys. The OS detection really needs to be fixed though. It doesn't properly detect cygwin perl ($^O == cygwin) and linux isn't the only unix platform that we support. We should default to unix if win32 & macos are not detected.
Attachment #95168 - Flags: needs-work+
Chris, thanks for having a look at this. I agree with the fix for other Unix platforms -- I haven't thought of that at all. In addition to this we could list the Unix jar files last in the corresponding jar.mn. This would prevent the wrong platform file from becoming chrome provider even if the detection failed. I'm not sure that I agree with the $^O == cygwin comment. I believe I tried that before and ran into some issues, the current Windows/cygwin detection seems to work quite well. I have to admit that I plagiarized Gerv's code, which means that there are at least two precedents in our code base: http://lxr.mozilla.org/seamonkey/source/config/make-chromelist.pl#71 http://lxr.mozilla.org/seamonkey/source/embedding/config/gen_mn.pl#15
Are you sure that you're using cygwin perl and that $win32 is being set to 1? ARCHANGEL:/cygdrive/c/root/mozilla> perl -e '$win32 = ($^O =~ /((MS)?win32)|os2/ i) ? 1 : 0; print "$win32\n"' 0 ARCHANGEL:/cygdrive/c/root/mozilla> perl -v This is perl, v5.6.1 built for cygwin-multi The other scripts that use this method of platform detection should be fixed as well (though not necessarily in this bug).
Chris, you are obviously right. I filed a new bug 166105 for cygwin perl support in make-chromelist.pl and gen_mn.pl. I still don't like the platform detection on Windows, what would you say to something less kludgy? my $win32 = ($^O =~ /win|os2/i) ? 1 : 0;
Attachment #95168 - Attachment is obsolete: true
> my $win32 = ($^O =~ /win|os2/i) ? 1 : 0; You can't do this - remember "darwin". Gerv
oh, right :-)
Gerv, thanks for pointing out my oversight in regards to Darwin... I hope that this could finally be what we've been looking for. my $macos = ($^O =~ /MacOS|darwin/i) ? 1 : 0; my $win32 = (!$macos && ($^O =~ /win|os2/i)) ? 1 : 0; my $unix = !($win32 || $macos) ? 1 : 0;
Attachment #97479 - Attachment is obsolete: true
Comment on attachment 97552 [details] [diff] [review] yet another patch iteration I preferred the previous win32 detection string though it still needed cygwin added to the list. Just grepping for /win/ seems to be asking for trouble.
Attachment #97552 - Flags: needs-work+
thanks Chris - I'm changing back to my $win32 = ($^O =~ /((MS)?win32)|cygwin|os2/i) ? 1 : 0;
Attachment #97552 - Attachment is obsolete: true
For Mac, these changes scare me. Make sure they are thoroughly tested for both MacPerl-based CFM builds, and Mach-O based builds, using both Classic and Modern skins. This seems like it could cause a bunch of Mac chrome regressions. Who runs add-chrome.pl and make-jars.pl for MacPerl builds? Is this a packaging-only thing? +my $macos = ($^O =~ /MacOS|darwin/i) ? 1 : 0; Does $^O work in MacPerl?
One more comment: > 2) In these 2 jar files, all file format (line breaks) are different between different platforms' langenus.xpi. ... > The second problem confused our localization tool, the software think all the files are total different. None of the changes in this bug address this issue, and it's really an issue with your localization tools. The tools _have_ to be smart about linebreaks.
Well, as long as the first cross-platform problem can be fixed, we can live with it then. Because for localization, we can just simply use the en-[platform].jar in the win32 langenus.xpi, for all other platforms - then we won't have line-break problem anymore. This means we will use the DOS-linebreak format files in our localized Mac builds, which won't be a problem - as long as the en-[platform].jar inconsistency problem can be fixed, we will be safe to use the win32 en-mac.jar in Mac build.
Simon: Mach-O-based builds work great. It appears that for the MacPerl-based build, we need to change Jar.pm http://lxr.mozilla.org/seamonkey/source/build/mac/build_scripts/Moz/Jar.pm#407 The only reason MacPerl builds worked for me is that the mac-specific files are listed last in installed-chrome.txt. This is due to the reordering of static jar manifest entries introduced by the rest of the patch. (Competing chrome providers are awarded precedence based on their position in the installed-chrome.txt files.) $^O works great with both MacPerl and on OS X. I do realize the risk of regressions and since the platform detection is "merely" aiming at elimination of unnecessary platform files in installed-chrome.txt, failure is not catastrophic. The most fragile platform (Unix? Mac?) could be simply listed last in that file by the virtue of reordering static jar manifest entries. locale,install,url,jar:resource:/chrome/en-unix.jar!/locale/en-US/navigator-platform/ locale,install,url,jar:resource:/chrome/en-win.jar!/locale/en-US/navigator-platform/ locale,install,url,jar:resource:/chrome/en-mac.jar!/locale/en-US/navigator-platform/
sfraser: this is an analogous change for Jar.pm, it excludes platform-dependent chrome files from being registered via installed-chrome.txt. In theory this minimizes the probability of a foreign platform file from becoming chrome provider.
Attachment #97709 - Attachment is obsolete: true
Would anyone dare to r=/sr=? Please? This has been tested to death and I can't see any issues in verification builds. The packaging change is on the top of the L10n and L12y wish list for 1.2. Let's get this in in time for the 1.2 release. Giving it a longer trunk baking time and QA exposure can only help to prevent any potential regressions. The original incarnation of this patch was reviewed by tao and dveditz. We had to broaden the scope due to some build system relicts pointed out by dveditz. Thanks Dan!
Whiteboard: [adt3] [ETA: 6/3/02] → have patch / need r= and sr=
Comment on attachment 100488 [details] [diff] [review] complete & updated patch r=cls
Attachment #100488 - Flags: review+
thanks Chris! Simon would feel comfortable giving an sr=?
Whiteboard: have patch / need r= and sr= → has r= / needs sr=
not really. Maybe Dan V. can?
thanks Simon, I've sent him an email... I really hope this patch can make it into 1.2.
I hate to be nagging, but l10n really needs this. Dan has given sr= on the original patch and might be swamped right now. He is not responding to my emails. I'll try to track him down in time for the 1.2b freeze, but if anyone gave an sr= on the perl script changes, that would be great :-)
Comment on attachment 100488 [details] [diff] [review] complete & updated patch rs=sfraser
Attachment #100488 - Flags: superreview+
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
*** Bug 173502 has been marked as a duplicate of this bug. ***
Yeah!!! With today's trunk, it looks great to me :-) MILLION THANKS to jbetak! This is going to benefit us a lot.
Ying, that's great! This was not a trivial change, and I'm still half expecting someone to slam me with some nasty regression. And I hope to hear from kairo and his experience with the change. I'm cc'ing dbragg for the bulk of the credit. I'd say that he and dveditz did most of the work. I only happened to be driving towards the end ;-)
I built today's CVS (after I re-pulled the files I had conflicts with, as I had 118140's patch applied in my tree), and all the files look fine on Linux here. I hope this really is resolved now. Thanks! Marking verified.
Status: RESOLVED → VERIFIED
Thanks kairo! ying, rchen, mcarlson - does l10n need this on the branch (bug 173552)? If so, would early next week be a workable timeframe?
Thanks! early next week would be fine. This is so great!
nominating for the branch -- please seek drivers and adt approval to land.
Plussing for adt. Need checkin by cob 10/16. Please make sure to get all required approvals (drivers) before checkin if this is not already done.
*** Bug 173552 has been marked as a duplicate of this bug. ***
Has this landed on the branch yet?
*** Bug 172075 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.