Closed Bug 534694 Opened 10 years ago Closed 10 years ago

Can't build layout-debug extension in a libxul build

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: bzbarsky, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

STEPS TO REPRODUCE: Configure with:

ac_add_options --enable-extensions=default,layout-debug
ac_add_options --enable-debug
ac_add_options --enable-libxul
ac_add_options --disable-optimize

and build.

ACTUAL RESULTS:
make[4]: *** No rule to make target `../../staticlib/components/libgkdebug.a', needed by `XUL'.  Stop.

EXPECTED RESULTS: Able to build.
Does adding GRE_MODULE=1 to extensions/layout-debug/src/Makefile.in fix this?  That's the difference I notice comparing to other similar makefiles.

mrbkap is testing that now, I think.
Adding GRE_MODULE=1 seems to have helped.
http://hg.mozilla.org/mozilla-central/rev/925595f3c086
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reopening.  This is still broken for me.  I pulled rev b20cff5e2157 (sometime today), clobbered my objdir, confugured on Linux with:

ac_add_options --enable-debug
ac_add_options --disable-optimize
ac_add_options --enable-extensions=default,layout-debug
ac_add_options --enable-libxul

and built... I get:

gmake[4]: *** No rule to make target `../../staticlib/components/libgkdebug.a', needed by `libxul.so'.  Stop.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
That makes things compile for me (on Linux).  Want to ask :bs for review?  ;)
Did you intend to remove the GRE_MODULE=1 line?  Otherwise looks good to me.
Comment on attachment 418121 [details] [diff] [review]
extensions/layout-debug wants to be in tier_toolkit

This will end up building extensions/layout-debug twice (once in tier_toolkit and another time when extensions/ is built).
Attachment #418121 - Flags: review?(benjamin) → review-
bsmedberg suggested just moving the layout debugger to layout/debugger or layout/tools.  I would be fine with that, and just building it always in debug builds or something.  David?
Blocks: 536237
That sounds good, and fulfills part of bug 536237, which makes me happy.
(In reply to comment #9)
> bsmedberg suggested just moving the layout debugger to layout/debugger or
> layout/tools.  I would be fine with that, and just building it always in debug
> builds or something.  David?

Sounds good to me, although perhaps it should be conditioned on both DEBUG (since it's useless without DEBUG) *and* ENABLE_TESTS (since that seems to fit better, perhaps, though maybe not).
OS: Mac OS X → All
Hardware: x86 → All
Attached patch WIP 1 (obsolete) — Splinter Review
Move layout-debug to layout/tools.  I'm still waiting for this patch to build.  If everything is fine, I will post it to the try server to see if it builds fine there, and will then request a review on this patch.  Feel free to comment on it in the mean time, though.
Assignee: nobody → ehsan.akhgari
Attachment #418121 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
So, layout-debug depends on things like nsICommandLineHandler.h, which live in toolkit.  We either have to move that (and possibly other similar) interface(s) to layout, or modify toolkit-tiers.mk to special case this directory and build it when toolkit has been built.

Benjamin, which approach is more appropriate in your opinion?
http://hg.mozilla.org/projects/firefox-lorentz/rev/bfc0723b3c3a

And FWIW on trunk toolkit and layout are in the same tier-platform, so you probably don't have to do anything special at all.
Attached patch Patch (v1)Splinter Review
You're right, it's now possible because their both part of tier platform.

So, now this patch actually works, and it enables the layout-debug extension when --enable-debug and --enable-tests are defined, and it makes incremental builds to work as well.

This patch is only the unbitrotted version of the previous one.
Attachment #425568 - Attachment is obsolete: true
Attachment #434756 - Flags: review?(ted.mielczarek)
Attachment #434756 - Flags: review?(benjamin)
Comment on attachment 434756 [details] [diff] [review]
Patch (v1)

The build bits look fine. Just curious, our debug builds on Tinderbox have tests enabled (as evidenced by our debug unittest boxes). Having layout debug enabled won't cause any problems there, right?
Attachment #434756 - Flags: review?(ted.mielczarek) → review+
(In reply to comment #16)
> (From update of attachment 434756 [details] [diff] [review])
> The build bits look fine. Just curious, our debug builds on Tinderbox have
> tests enabled (as evidenced by our debug unittest boxes). Having layout debug
> enabled won't cause any problems there, right?

I wouldn't expect there to be any problems.  The extension basically does two things: it adds a command line flag which would open an entirely different XUL window, and it adds an entry to the Tools menu.  It also has an XPCOM component registered.

That said, how can I make sure that there would be no problems there?
I guess if you can do a local debug build with this patch, and just sanity check that running a subset of the tests doesn't break anything obvious, that'd be fine. From what you've described it doesn't sound like it will be a problem.
I've been using it for some time now, and I've been able to run different types of tests.  So I guess we're fine.
Comment on attachment 434756 [details] [diff] [review]
Patch (v1)

Roc, can you review this?  FWIW, it's been in my mq (applied!) ever since I wrote the patch, and I've ran the resulting builds, and ran our entire test suite on top of this quite a bit of times, without any problems.

I guess this extension is too good to be hidden in an obscure location where almost nobody sees it!
Attachment #434756 - Flags: review?(roc)
Attachment #434756 - Flags: review?(benjamin)
...or most of the people who use it do non-libxul builds :-)
http://hg.mozilla.org/mozilla-central/rev/2dc11a7e6bb5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Also, maybe someone who knows a fair bit about this tool can write some documentation for it on MDC?
Keywords: dev-doc-needed
Depends on: 568204
Depends on: 589667
Bug 589667 says this didn't actually make layout-debug *work* in libxul builds.
Depends on: 595620
You need to log in before you can comment on or make changes to this bug.