292804 - Unable to build reporter

Status: VERIFIED FIXED

(Other Applications Graveyard :: Reporter, defect)

Description

[Robert Accettura [:raccettura]]

copy
"/cygdrive/c/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/xpinstall/packager/common/share.t"
reporter.template
cat: reporter.jst: No such file or directory

 Making reporter.xpi...
  adding: install.js (stored 0%)

This doesn't look right.

Comment 1

[Jay Patel [:jay]]

bsmedberg:  Do you think any of the l10n changes you made in bug 285653 could
have broken something?  after my first checking
(https://bugzilla.mozilla.org/attachment.cgi?id=177084) and before i "disabled"
reporter (https://bugzilla.mozilla.org/attachment.cgi?id=179810), things were
ok.  it looks like a lot of the reporter directory structure changed for the
localization, could the be the cause?  We need to get this figured out asap if
reporter is going to make it into 1.1a.

Comment 2

[Benjamin Smedberg]

Jay, it doesn't look like you are copying the reporter.jst file at
http://lxr.mozilla.org/mozilla/source/browser/installer/windows/Makefile.in#58

In addition, you should remove the locale registration at
http://lxr.mozilla.org/mozilla/source/browser/installer/windows/reporter.jst#13

Comment 3

[Jay Patel [:jay]]

Attachment: bsmedberg's suggested changes

These are the changes Benjamin suggested, but I am unable to complete a build. 
Can someone else try this patch and see if they have any luck?	I'll be out of
town 4/5 - 4/8, so if anyone is able to fix this, pleases get reviews and check
it in.	Thanks.

Comment 4

[Robert Accettura [:raccettura]]

bsmedberg: ca you try this patch and see if this fixes us?  We really need to
get this going quickly at this point, so any help you can provide is _greatly_
appreciated as always.


--> jay as he's doing the lifting in this bug.

Comment 5

[Jay Patel [:jay]]

I finally got my machine setup for builds and was able to compile a fresh pulled
tree...unfortunately, I'll be out of town until Sunday afternoon.  I'll try
making my changes again and trying out a build then...but if someone can test it
out before, that will be great.  Thanks.

Comment 6

[Benjamin Smedberg]

I am in the middle of l10n and software update work; I would prefer if somebody
else could fix whatever's wrong here.

Comment 7

[Jay Patel [:jay]]

I think I know what broke, but I don't know enough about the build system to fix
it.  Here are the things that have changed since I orginally checked in everything:

new files in extensions/reporter/locales:
  there is a new jar.mn that seems to be a template for localized files
  there are en-US and generic directories with chrome files

changes in extensions/reporter/jar.mn:
  bsmedberg's changes removed all the locale files from the reporter.jar, and it
looks like the new jar.mn file above should take care of those for different
locales, but i don't know how to make that work

in browser/installer/windows/reporter.jst
  i removed the registerChrome line for the LOCALE, and the new files above
should take care of this as well, but i have no idea how

Those are the changes I think are causing problems in getting the reporter.xpi
created properly.  How everything is linked and what to do to get the new
localized files included is something that I have not been able to figure out.  

bsmedberg:  I know you're busy, but if what I just noted makes sense to you, can
you either help me figure it out or take a couple of hours to look into it and
get it working?  Any help is appreciated because both Robert and I are stumped
at this point.  Thanks.

bryner:  If you have some time, I can work with you too if you know what we need
to do here.

Comment 8

[Benjamin Smedberg]

Attachment: Installer fixup

This is what I think you need, and it compiles on my tree. The change to
"iconv" and install.it are incidental l10n fixup I had hanging around in my
tree.

Comment 9

[Robert Accettura [:raccettura]]

Comment on attachment 183545 [details] [diff] [review]
Installer fixup

bryner is [much] more qualified to review this.

My box isn't building (cygwin can go to hell), and I don't get enough of this
stuff to intelegently look at it.

Comment 10

[Jay Patel [:jay]]

Robert:  I have a tree with all of the changes from your patch already in
(except for the iconv and the config.it changes)...  So let me try adding those
couple of things later today and see if that works.  I am getting builds right
now, but although the installer allows me to choose Reporter during a custom
install, I don't see it in the UI and the build log doesn't show the xpi being
built properly.  If the two things that are missing fix it, I'll update this bug
with my patch and we can go from there.

Comment 11

[Jay Patel [:jay]]

I was on my way out when I left my last comment, but it looks like bsmedberg's
latest patch is what I already have in my tree..and it's not working!  The two
changes I didn't have don't seem to be related to the problem (as he said
himself, that they were incidental l10n changes left over in his tree). 
Soo...we're still back where we started.  Any other ideas?

Comment 12

[Benjamin Smedberg]

When I build an installer with this patch it works fine (the reporter appears in
the help menu, assuming I choose custom install +reporter -talkback). What is
the specific problem you're seeing?

Comment 13

[Robert Accettura [:raccettura]]

If you can build and see it in the help menu... I'd say it's working.

Do we have an option in the installer?

Comment 14

[Jay Patel [:jay]]

I realized that I might still need the /reporter/resources/locale directory
(which I had deleted from my tree a while back after the l10n changes).  I did a
"cvs up -d" from the extensions/reporter dir and am building now.  I'll build
the installer in the morning and see what happens.  Hopefully this was all my
mistake/stupidity and it works. :-)  I'm learning as I go...

Comment 15

[Jay Patel [:jay]]

I have no idea what's wrong with my setup, it's still not working for me.  I get
a -214: DOES NOT EXIST error during a custom install where I choose to install
Website Reporter.  

I'm going to pull a fresh tree and try again, but if I still can't get it
working, I say we get the reviews we need and get these last few changes checked
in to see if it works for others (Since it appears to be working for Benjamin).

Comment 16

[Robert Accettura [:raccettura]]

Agreed.

You can get the review(s) needed, checkin, and enable reporter at this point.

Once we have some successful builds, we'll point towards production, and life
will be good.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 183545 [details] [diff] [review]
Installer fixup

a=asa for landing as soon as reviews are in. Thanks, everyone, for all your
hard work on making this happen.

Comment 18

[Chase Phillips]

Comment on attachment 183545 [details] [diff] [review]
Installer fixup

r=[e-mail]

Comment 19

[Jay Patel [:jay]]

Comment on attachment 183545 [details] [diff] [review]
Installer fixup

Checked in bsmedberg's suggested fix.  I am still unable to get it working on
my machine, but he said it worked for him.  Please test with tomorrow's builds
and see if Reporter is working.

Comment 20

[Jay Patel [:jay]]

For future reference, the problem was that I also needed to add reporter to
default list of extensions to be built in configure.in and run autoconf on my
local tree to get a build that reflected that change.