745532 - Use nsinstall for browserscope, not tar/untar

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Bug 550569 added this to the makefile:

  (cd $(srcdir) && tar $(TAR_CREATE_FLAGS) - browserscope 2> /dev/null) | (cd $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir) && tar -xf -)

This has at least two annoying side-effects:

1) It copies the files instead of symlinking them, on platforms where they'd normally be symlinked.  This is surprising, and means that if you update currentStatus.js or whatever in the source, you have to copy it to the objdir before rerunning the tests.

2) It copies all the files that are lying around there, even ones you don't want.  I've been bitten by this more than once when test_richtext2.html.orig or something got copied and made the test runner get confused and hang.

The files should be enumerated and installed with $(INSTALL), like all the others.  I'm not actually sure what the best way to do this is -- just adding "browserscope" (the directory) to _TEST_FILES seems to fix (1), at least on Linux, but not (2).

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Ted, do you know what the best way is to do this?  browserscope/ is deeply nested, and we need to copy a list of files along the lines of

browserscope/test_richtext.html
browserscope/test_richtext2.html
browserscope/lib/richtext2/currentStatus.js
browserscope/lib/richtext2/richtext2/tests/selection.py
browserscope/lib/richtext2/richtext2/tests/queryState.py
browserscope/lib/richtext2/richtext2/tests/queryIndeterm.py
browserscope/lib/richtext2/richtext2/tests/queryValue.py
browserscope/lib/richtext2/richtext2/tests/forwarddelete.py
browserscope/lib/richtext2/richtext2/tests/applyCSS.py
browserscope/lib/richtext2/richtext2/tests/changeCSS.py
browserscope/lib/richtext2/richtext2/tests/queryEnabled.py
browserscope/lib/richtext2/richtext2/tests/querySupported.py
browserscope/lib/richtext2/richtext2/tests/unapplyCSS.py
browserscope/lib/richtext2/richtext2/tests/insert.py
browserscope/lib/richtext2/richtext2/tests/change.py
browserscope/lib/richtext2/richtext2/tests/delete.py
browserscope/lib/richtext2/richtext2/tests/apply.py
browserscope/lib/richtext2/richtext2/tests/unapply.py
browserscope/lib/richtext2/richtext2/static/editable-dM.html
browserscope/lib/richtext2/richtext2/static/editable.css
browserscope/lib/richtext2/richtext2/static/editable-body.html
browserscope/lib/richtext2/richtext2/static/editable-div.html
browserscope/lib/richtext2/richtext2/static/js/variables.js
browserscope/lib/richtext2/richtext2/static/js/units.js
browserscope/lib/richtext2/richtext2/static/js/canonicalize.js
browserscope/lib/richtext2/richtext2/static/js/run.js
browserscope/lib/richtext2/richtext2/static/js/compare.js
browserscope/lib/richtext2/richtext2/static/js/output.js
browserscope/lib/richtext2/richtext2/static/js/range-bootstrap.js
browserscope/lib/richtext2/richtext2/static/js/pad.js
browserscope/lib/richtext2/richtext2/static/js/range.js
browserscope/lib/richtext/currentStatus.js
browserscope/lib/richtext/richtext/richtext.html
browserscope/lib/richtext/richtext/js/range.js
browserscope/lib/richtext/richtext/editable.html

preserving directory structure.

Comment 2

[(no longer active)]

Right.  The reason why I used tar instead of nsinstall initially was to make sure that updates from upstream would not require updating the makefiles.

In hindsight, that was probably the wrong thing to optimize for.  I'm not quite sure about the makefile magic we need if we want to use nsinstall, but you should try to avoid nesting makefiles in the richtext* directories.  Maybe we can mkdir -p the target directories before calling nsinstall?  Or maybe nsinstall takes an argument to create the target directory if needed?

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Unassigning from self until it's clear how we should handle this . . .

Comment 4

[Gregory Szorc [:gps]]

There is a generic install rule in the build system: https://mxr.mozilla.org/mozilla-central/source/config/rules.mk#1623

See services/sync/Makefile.in for example usage.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Looks fixed now.