Closed Bug 1254120 Opened 6 years ago Closed 6 years ago
.in to use FINAL _TARGET / FINAL _TARGET _FILES in moz .build
58 bytes, text/x-review-board-request
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.
Review commit: https://reviewboard.mozilla.org/r/39585/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/39585/
Attachment #8729781 - Flags: review?(ted)
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.
(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. :)
You need to log in before you can comment on or make changes to this bug.