Closed Bug 1253431 Opened 4 years ago Closed 4 years ago

Port SDK related install targets to moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

I think this requires a new SdkFiles object derived from FinalTargetFiles in order to install things into dist/sdk. Some of the INSTALL_TARGETS also install things generated during the compile tier, so we depend on bug 1253430.
This adds support for an SDK_FILES variable in moz.build, which creates
a FinalTargetFiles object to install files into dist/sdk/

Review commit: https://reviewboard.mozilla.org/r/38307/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38307/
Attachment #8726911 - Flags: review?(gps)
We can just generate xpidllex.py/xpidlyacc.py in the current directory
rather than one directory higher, and specify this directory as an
include path to xpidl-process.py

Review commit: https://reviewboard.mozilla.org/r/38311/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38311/
Attachment #8726913 - Flags: review?(gps)
We only ever execute this in one place, so we can just have the main
action do the --regen --cachedir=. mode of operation.

Review commit: https://reviewboard.mozilla.org/r/38313/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38313/
Attachment #8726914 - Flags: review?(gps)
It has no effect anyway, since it is set after including config/rules.mk

Review commit: https://reviewboard.mozilla.org/r/38317/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38317/
Attachment #8726916 - Flags: review?(gps)
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e92169592069
(Note this patchset requires bug 1253430 to build successfully)
Attachment #8726911 - Flags: review?(gps) → review+
Comment on attachment 8726911 [details]
MozReview Request: Bug 1253431 part 1 - Add SDK_FILES to moz.build; r?gps

https://reviewboard.mozilla.org/r/38307/#review35079

Nit: you don't need to write "part N" in commit messages in MozReview because MozReview preserves the commit ordering in the UI since it is basing things off version control.
Attachment #8726912 - Flags: review?(gps) → review+
Comment on attachment 8726912 [details]
MozReview Request: Bug 1253431 part 2 - Use SDK_FILES instead of INSTALL_TARGETS; r?gps

https://reviewboard.mozilla.org/r/38309/#review35083

The patch reminds me how much I hate xpcom/idl-parser/xpidl/Makefile.in.
Attachment #8726913 - Flags: review?(gps) → review+
Comment on attachment 8726913 [details]
MozReview Request: Bug 1253431 part 3 - Move SDK_BINARY files in xpcom/idl-parser/xpidl to moz.build; r?gps

https://reviewboard.mozilla.org/r/38311/#review35093

Very nice.
Attachment #8726914 - Flags: review?(gps)
Comment on attachment 8726914 [details]
MozReview Request: Bug 1253431 part 4 - Convert header.py to a GENERATED_FILES script; r?gps

https://reviewboard.mozilla.org/r/38313/#review35097

::: xpcom/idl-parser/xpidl/header.py
(Diff revision 1)
> -    idl = p.parse(open(file).read(), filename=file)
> -    idl.resolve(options.incdirs, p)
> -    print_header(idl, outfd, file)
> -
> -    if closeoutfd:
> -        outfd.close()
> -
> -    if options.depfile is not None:
> -        dirname = os.path.dirname(options.depfile)
> -        if dirname:
> -            try:
> -                os.makedirs(dirname)
> -            except:
> -                pass
> -        depfd = open(options.depfile, 'w')
> -        deps = [dep.replace('\\', '/') for dep in idl.deps]
> -
> -        print >>depfd, "%s: %s" % (options.outfile, " ".join(deps))
> -        for dep in deps:
> -            print >>depfd, "%s:" % dep

Is all this code really dead?! I guess we have mozbuild.action.xpidl_process, so I guess so!
Comment on attachment 8726914 [details]
MozReview Request: Bug 1253431 part 4 - Convert header.py to a GENERATED_FILES script; r?gps

https://reviewboard.mozilla.org/r/38313/#review35099
Attachment #8726914 - Flags: review+
Comment on attachment 8726915 [details]
MozReview Request: Bug 1253431 part 5 - Remove build/unix/Makefile.in; r?gps

https://reviewboard.mozilla.org/r/38315/#review35101
Attachment #8726915 - Flags: review?(gps) → review+
Comment on attachment 8726916 [details]
MozReview Request: Bug 1253431 part 6 - Remove SDK_BINARY from js/src; r?gps

https://reviewboard.mozilla.org/r/38317/#review35103

Wow.
Attachment #8726916 - Flags: review?(gps) → review+
Attachment #8726917 - Flags: review?(gps) → review+
Comment on attachment 8726917 [details]
MozReview Request: Bug 1253431 part 7 - Remove SDK_BINARY; r?gps

https://reviewboard.mozilla.org/r/38319/#review35105

Awesome series.

::: python/mozbuild/mozbuild/frontend/emitter.py:833
(Diff revision 1)
> +        generated_files.update(['%s%s' % (k, self.config.substs.get('BIN_SUFFIX', '')) for k in self._binaries.keys()])

I'm kinda surprised this doesn't invalidate any tests!
(In reply to Gregory Szorc [:gps] from comment #9)
> Nit: you don't need to write "part N" in commit messages in MozReview
> because MozReview preserves the commit ordering in the UI since it is basing
> things off version control.

Thanks for the heads up! I'll do that from now on.
(In reply to Gregory Szorc [:gps] from comment #12)
> Is all this code really dead?! I guess we have
> mozbuild.action.xpidl_process, so I guess so!

Apparently so - we only run header.py once in the whole build.
Depends on: 1304131
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.