Closed Bug 1254120 Opened 6 years ago Closed 6 years ago

Change toolkit/crashreporter/test/Makefile.in to use FINAL_TARGET / FINAL_TARGET_FILES in moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: ted, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This Makefile:
https://hg.mozilla.org/mozilla-central/file/e7319545eb3819da67ffe1d4233022ae71e3a9a1/toolkit/crashreporter/test/Makefile.in

Installs a shared library and a jsm to two xpcshell test directories. I think we can just set FINAL_TARGET to the first directory, remove DIST_INSTALL = False, use FINAL_TARGET_FILES to install the .jsm, and maybe fix up the test in unit_ipc to deal with the files not being in the current test directory.
Assignee: nobody → mshal
Per our IRC conversation, we can install libtestcrasher.so up one level, and teach CrashTestUtils.jsm how to find it.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a45d4a10a7b8
Comment on attachment 8729781 [details]
MozReview Request: Bug 1254120 - Remove toolkit/crashreporter/test/Makefile.in; r?ted

https://reviewboard.mozilla.org/r/39585/#review36295

::: toolkit/crashreporter/test/CrashTestUtils.jsm:33
(Diff revision 1)
>  // Grab APIs from the testcrasher shared library
>  Components.utils.import("resource://gre/modules/Services.jsm");
>  Components.utils.import("resource://gre/modules/ctypes.jsm");
>  var dir = Services.dirsvc.get("CurWorkD", Components.interfaces.nsILocalFile);
>  var file = dir.clone();
> +file.append('..')

You should do `dir = dir.parent;` or `file = file.parent` instead.

::: toolkit/crashreporter/test/moz.build:6
(Diff revision 1)
>  # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
>  # vim: set filetype=python:
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> -DIST_INSTALL = False
> +FINAL_TARGET = '_tests/xpcshell/toolkit/crashreporter/test'

I'm starting to think this pattern is common enough that we ought to have a shorthand for it that doesn't involve spelling the directory out completely.
Attachment #8729781 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> > +file.append('..')
> 
> You should do `dir = dir.parent;` or `file = file.parent` instead.

Fixed!

> ::: toolkit/crashreporter/test/moz.build:6
> > -DIST_INSTALL = False
> > +FINAL_TARGET = '_tests/xpcshell/toolkit/crashreporter/test'
> 
> I'm starting to think this pattern is common enough that we ought to have a
> shorthand for it that doesn't involve spelling the directory out completely.

What pattern exactly? Of the moz.build files that specify FINAL_TARGET, only two (this one and js/xpconnect/tests/components/native/moz.build) currently install into _tests/xpcshell/$(relativesrcdir). The others may only use some part of relativesrcdir, or add other things. Eg:

./xpcom/tests/component_no_aslr/moz.build:FINAL_TARGET = '_tests/xpcshell/xpcom/tests/unit'

There will probably be more as we finish moving over INSTALL_TARGETS & PP_TARGETS, but it doesn't seem like there will be a lot.
https://hg.mozilla.org/mozilla-central/rev/1513ce8788b5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(In reply to Michael Shal [:mshal] from comment #4)
> What pattern exactly? Of the moz.build files that specify FINAL_TARGET, only
> two (this one and js/xpconnect/tests/components/native/moz.build) currently
> install into _tests/xpcshell/$(relativesrcdir). The others may only use some
> part of relativesrcdir, or add other things. Eg:

Ah! It seemed like we had more things installing to $relativesrcdir. I guess we'll just live with it. :)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.