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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

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.

Comment 1

[Michael Shal [:mshal]]

Attachment: MozReview Request: Bug 1254120 - Remove toolkit/crashreporter/test/Makefile.in; r?ted

Review commit: https://reviewboard.mozilla.org/r/39585/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39585/

Comment 2

[Michael Shal [: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 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Michael Shal [:mshal]]

(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.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1513ce8788b5

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1513ce8788b5

Comment 7

[(not currently active) Ted Mielczarek]

(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. :)