Closed Bug 1372381 Opened 7 years ago Closed 6 years ago

Suport TEST_HARNESS_FILES in the Tup backend

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox57 wontfix, firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox57 --- wontfix
firefox61 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(4 files)

Bug 1372381 - Generate automation.py with GENERATED_FILES rather than Makefile.in
59 bytes, text/x-review-board-request
mshal
: review+
Details
Bug 1372381 - Temporarily skip certain problematic binaries in the Tup backend.
59 bytes, text/x-review-board-request
mshal
: review+
Details
Bug 1372381 - Compile host libraries, host programs, and simple programs in the Tup backend.
59 bytes, text/x-review-board-request
mshal
: review+
Details
Bug 1372381 - Install TEST_HARNESS_FILES in the tup backend.
59 bytes, text/x-review-board-request
mshal
: review+
Details 
These will need an additional fix on top of bug 1371817.
I just found automation.py is generated by a makefile, so handling that will be part of this.
Product: Core → Firefox Build System
Assignee: nobody → cmanchester
Will this wind up making tup install all these files as part of the normal build? ahal did some hoop-jumping to make that not happen in the recursive make build because it's a lot of files. If it does, it'd be good to measure the overhead.

n.b., that hoop-jumping put a few calls to _run_make in mach commands, like this one to regenerate the build backend:
https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/testing/mozbase/moztest/moztest/resolve.py#391

And this one in `install_tests`:
https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/python/mozbuild/mozbuild/controller/building.py#1293

It looks like the non-wholesale `install_test_files` function does avoid make, which is nice:
https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/python/mozbuild/mozbuild/testing.py#207
That stuff is mostly aimed at not writing out the test database or installing tests from test manifests until we need to, which we'll have to handle at some point. TEST_HARNESS_FILES themselves aren't much overhead by comparison (several hundred vs. tens of thousands of files).
Comment on attachment 8970424 [details]
Bug 1372381 - Generate automation.py with GENERATED_FILES rather than Makefile.in

https://reviewboard.mozilla.org/r/239186/#review245448

::: build/gen_automation.py:14
(Diff revision 1)
> +from mozbuild.preprocessor import Preprocessor
> +
> +
> +def main(output, input_file):
> +    pp = Preprocessor()
> +    pp.context.update(dict(buildconfig.defines.iteritems()))

I think we want to use buildconfig.defines['ALLDEFINES'] here from bug 1402012 instead of iterating them all. I probably should've just made the preprocessor optionally support the buildconfig defines directly so we don't have to manually add them whenever we use the preprocessor, and it could more accurately track which defines are actually used...
Attachment #8970424 - Flags: review+
Attachment #8970424 - Flags: review?(core-build-config-reviews)
Comment on attachment 8970425 [details]
Bug 1372381 - Temporarily skip certain problematic binaries in the Tup backend.

https://reviewboard.mozilla.org/r/239188/#review245538
Attachment #8970425 - Flags: review?(mshal) → review+
Comment on attachment 8970426 [details]
Bug 1372381 - Compile host libraries, host programs, and simple programs in the Tup backend.

https://reviewboard.mozilla.org/r/239190/#review245542

::: python/mozbuild/mozbuild/backend/tup.py:415
(Diff revision 1)
> +        )
> +        backend_file.rule(
> +            cmd=cmd,
> +            inputs=inputs,
> +            outputs=outputs,
> +            display='LINK %o'

nit: This should be AR instead of LINK to show what we're actually doing (just creating an archive, not linking). We could also consider making it something like 'AR \[host\] %o' to make it clear that it's a host operation, though that might be redundant with the host\_ prefix in the output filename.
Attachment #8970426 - Flags: review?(mshal) → review+
Comment on attachment 8970427 [details]
Bug 1372381 - Install TEST_HARNESS_FILES in the tup backend.

https://reviewboard.mozilla.org/r/239196/#review245562

::: python/mozbuild/mozbuild/backend/tup.py:717
(Diff revision 1)
>                              self.backend_input_files.add(prefix)
>                              finder = FileFinder(prefix)
>                              for p, _ in finder.find(f.full_path[len(prefix):]):
> +                                install_dir = prefix[len(obj.srcdir) + 1:]
>                                  backend_file.symlink_rule(mozpath.join(prefix, p),
> -                                                          output=mozpath.join(f.target_basename, p),
> +                                                          output=mozpath.join(install_dir, p),

Something seems not quite right here - some files from jar manifests are getting moved around. For example, now tab-frame.js gets installed as:

./obj-x86_64-pc-linux-gnu/dist/bin/browser/features/webcompat-reporter@mozilla.org/chrome/content/content/tab-frame.js

while previously it was:

./obj-x86_64-pc-linux-gnu/dist/bin/browser/features/webcompat-reporter@mozilla.org/chrome/content/tab-frame.js

(note the extra /content in the first one). The RecursiveMake backend does the latter as well.
Attachment #8970427 - Flags: review?(mshal)
Comment on attachment 8970427 [details]
Bug 1372381 - Install TEST_HARNESS_FILES in the tup backend.

https://reviewboard.mozilla.org/r/239196/#review245562

> Something seems not quite right here - some files from jar manifests are getting moved around. For example, now tab-frame.js gets installed as:
> 
> ./obj-x86_64-pc-linux-gnu/dist/bin/browser/features/webcompat-reporter@mozilla.org/chrome/content/content/tab-frame.js
> 
> while previously it was:
> 
> ./obj-x86_64-pc-linux-gnu/dist/bin/browser/features/webcompat-reporter@mozilla.org/chrome/content/tab-frame.js
> 
> (note the extra /content in the first one). The RecursiveMake backend does the latter as well.

There's a strange inconsistency in the handling of wildcards between `FinalTargetFiles` generated by the common backend from jar.mn and those from moz.build files. I have a hackaround that appears to have a correct end result, but we may need to re-visit this at some point to fix the inconsistency.
Comment on attachment 8970427 [details]
Bug 1372381 - Install TEST_HARNESS_FILES in the tup backend.

https://reviewboard.mozilla.org/r/239196/#review246470
Attachment #8970427 - Flags: review?(mshal) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7f382c1e43d95368c44d90f463ea2ad640add9c7 -d ee07e4d094bd: rebasing 461142:7f382c1e43d9 "Bug 1372381 - Generate automation.py with GENERATED_FILES rather than Makefile.in r=mshal"
merging python/mozbuild/mozbuild/backend/tup.py
rebasing 461143:859510eeac8a "Bug 1372381 - Temporarily skip certain problematic binaries in the Tup backend. r=mshal"
merging python/mozbuild/mozbuild/backend/tup.py
warning: conflicts while merging python/mozbuild/mozbuild/backend/tup.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
hg error in cmd: hg pull upstream: pulling from https://hg.mozilla.org/integration/autoland
searching for changes
abort: HTTP Error 500: Internal Server Error
hg error in cmd: hg pull upstream: pulling from https://hg.mozilla.org/integration/autoland
searching for changes
abort: HTTP Error 500: Internal Server Error
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/620bdfafd42f
Generate automation.py with GENERATED_FILES rather than Makefile.in r=mshal
https://hg.mozilla.org/integration/autoland/rev/92a96d6b599b
Temporarily skip certain problematic binaries in the Tup backend. r=mshal
https://hg.mozilla.org/integration/autoland/rev/4e0f7fe818d0
Compile host libraries, host programs, and simple programs in the Tup backend. r=mshal
https://hg.mozilla.org/integration/autoland/rev/56c7ffeaa9fd
Install TEST_HARNESS_FILES in the tup backend. r=mshal
Backout by aiakab@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ba36d8d9923
Backed out 4 changesets for artifact build bustages
(In reply to Pulsebot from comment #26)
> Backout by aiakab@mozilla.com:
> https://hg.mozilla.org/integration/autoland/rev/8ba36d8d9923
> Backed out 4 changesets for artifact build bustages

I exposed a hazard here where if we have a cross directory dependency between a test harness file and a generated file that we think will be needed for compilation artifact builds will break.

This is kind of annoying, but I don't see a simple fix without making artifact builds slower, so I think I'm just going to move the rules to install automation.py into build/moz.build so the rules get written to the same backend.mk.
Comment on attachment 8970424 [details]
Bug 1372381 - Generate automation.py with GENERATED_FILES rather than Makefile.in

https://reviewboard.mozilla.org/r/239186/#review246684
Attachment #8970424 - Flags: review?(mshal) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/79f78ded94a2
Generate automation.py with GENERATED_FILES rather than Makefile.in r=mshal
https://hg.mozilla.org/integration/autoland/rev/0a1897a68d63
Temporarily skip certain problematic binaries in the Tup backend. r=mshal
https://hg.mozilla.org/integration/autoland/rev/539a4a607ef9
Compile host libraries, host programs, and simple programs in the Tup backend. r=mshal
https://hg.mozilla.org/integration/autoland/rev/c2420955ce61
Install TEST_HARNESS_FILES in the tup backend. r=mshal
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: