We have GENERATED_FILES, but we still wind up having to write INSTALL_TARGETS bits for any headers that we generate. It would be better if moz.build wrote out the necessary INSTALL_TARGETS bits. Suggested syntax: EXPORTS += [ '!GeneratedHeader.h', ] since a '!' prefix means things in the objdir.

We might not even need the '!' prefix, since presumably mozbuild already knows if the file is in GENERATED_FILES. But being consistent with other variables doesn't hurt.

My original thought when I was writing the GENERATED_FILES stuff was that you'd do like g = GeneratedFile('foo.h') EXPORTS += [g] ...but having the moz.build reader deduce this from the filenames seems fine too, just like: GENERATED_FILES += ['foo.h'] EXPORTS += ['foo.h'] # make this work

For prior art here, I was toying around with Bazel last week and wrote a bazel BUILD file for rr: https://github.com/luser/rr/blob/bazel/BUILD You'll note there the GENERATED_FILES (that variable name isn't important) get listed in the cc_binary(srcs=...) list, and the rule (that I wrote) that generates them simply states that these filenames are outputs: https://github.com/luser/rr/blob/bazel/gen_syscalls.bzl#L15 and bazel figures the rest out.

The deduction approach seems better, especially since experience with the current incarnation of GENERATED_FILES says that GENERATED_FILES is clunky. It really should be something like: with GeneratedFile('foo'): SCRIPT = ... ENTRY_POINT = ... INPUTS = ....

bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium This change allows specifying objdir-relative paths in EXPORTS to enable exporting entries from GENERATED_FILES. Objdir paths in EXPORTS that are not in GENERATED_FILE will raise an exception. Example: ``` EXPORTS += ['!g.h', 'f.h'] GENERATED_FILES += ['g.h'] ```

I didn't go through the entire tree and fix every instance of GENERATED_FILES possible. It should be fairly straightforward, if you look at the patch you can see that I fixed one instance in accessible/xpcom. It should just be a matter of adding !foo.h to EXPORTS in any dir where foo.h is in GENERATED_FILES and removing the corresponding INSTALL_TARGETS bits from the Makefile.in.

Comment on attachment 8694209 [details] MozReview Request: bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium https://reviewboard.mozilla.org/r/26705/#review24281 I'd rather avoid the fragmentation and do the following: - Modify the mozbuild.frontend.data.Exports class to be like the TestingFiles one (i.e. a FinalTargetFiles derivative with a specific overridden install_target). - Have the Exports objects be generated from the loop that handles FINAL_TARGET{,_PP}_FILES and TESTING_FILES in emitter.py, and handle the GENERATED_FILES check there too. This will have the magic effect of making the right thing happen whenever we add !paths to e.g. FINAL_TARGET_FILES in the future, which we will undoubtedly, as that would allow to move some more stuff from moz.build. - The above means recursivemake would treat those Exports in _process_final_target_files, which would get the code to add INSTALL_TARGETS as well. Note: I haven't looked at the tests.

OK, I will take a look at making those changes.

Comment on attachment 8694209 [details] MozReview Request: bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26705/diff/1-2/

Comment on attachment 8694209 [details] MozReview Request: bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium https://reviewboard.mozilla.org/r/26705/#review24527 ::: python/mozbuild/mozbuild/backend/recursivemake.py:1292 (Diff revision 2) > - if target.startswith('dist/bin'): > - install_manifest = self._install_manifests['dist_bin'] > - reltarget = mozpath.relpath(target, 'dist/bin') > - elif target.startswith('dist/xpi-stage'): > - install_manifest = self._install_manifests['dist_xpi-stage'] > - reltarget = mozpath.relpath(target, 'dist/xpi-stage') > + for (path, manifest) in ( > + ('dist/bin', 'dist_bin'), > + ('dist/xpi-stage', 'dist_xpi-stage'), > + ('_tests', 'tests'), > + ('dist/include', 'dist_include'), > + ): Note, if the _tests install_manifest was _tests, we could use path.replace('/', '_'). That's been bugging me for a while... Followup? ::: python/mozbuild/mozbuild/backend/recursivemake.py:1310 (Diff revision 2) > + if isinstance(f, SourcePath): You want this branch to handle AbsolutePaths as well, so you might as well use not isinstance(f, ObjDirPath) ::: python/mozbuild/mozbuild/frontend/emitter.py:675 (Diff revision 2) > + if isinstance(f, SourcePath): Same as previous comment, you want to cover AbsolutePaths here too.

Comment on attachment 8694209 [details] MozReview Request: bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium https://reviewboard.mozilla.org/r/26705/#review24533 ::: python/mozbuild/mozbuild/frontend/emitter.py:675 (Diff revision 2) > + if isinstance(f, SourcePath): Oh, just thought of something else: we should probably reject non SourcePaths for FinalTargetPreprocessedFiles.

Comment on attachment 8694209 [details] MozReview Request: bug 1160185 - support GENERATED_FILES in EXPORTS. r?glandium I didn't mean for the r+ to be removed.

https://reviewboard.mozilla.org/r/26705/#review24533 > Oh, just thought of something else: we should probably reject non SourcePaths for FinalTargetPreprocessedFiles. Added a check + exception for this and a test for it.

https://reviewboard.mozilla.org/r/26705/#review24527 > Note, if the _tests install_manifest was _tests, we could use path.replace('/', '_'). That's been bugging me for a while... Followup? Good call! That wasn't hard to fix so I just did it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ca0a1e37264624f4304aec23918fb9e0f4910e2 bug 1160185 - support GENERATED_FILES in EXPORTS. r=glandium