1513071 - CSS2Properties.webidl not present in public/build/target.generated-files.tar.gz

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Jason Kratzer [:jkratzer]]

The CSS2Properties.webidl file is generated as part of the build process.  Would it be possible to have this file included in the "generated-files" archive?

Comment 1

[Christian Holler (:decoder)]

Needinfo for :ted because he implemented the original generated-files archive from what I can see :)

Comment 2

[(not currently active) Ted Mielczarek]

Sure. Most of the files that wind up in the generated sources archive get there by being passed to _handle_generated_sources in the common build backend. We have logic here that handles `GENERATED_FILES`:
https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/python/mozbuild/mozbuild/backend/common.py#174

It currently only captures things that set the `required_for_compile` property, which I suspect is reserved for source files. If you can figure out a sensible way to handle this file there that'd be fine.

The purpose of the generated-sources archive is to make those files available for when they show up in crash reports, which is why it's focused on files that actually get compiled.

Comment 3

[Jason Kratzer [:jkratzer]]

Attachment: Bug 1513071 - Included generated webidl files in generated sources-archive

This was easier than expected. Adding the webidl extension to the GeneratedFile class includes the CSS2Properties IDL in the archive.

Comment 4

[Jason Kratzer [:jkratzer]]

:nfroyd, any chance this is something you could review or happen to know who this might fall under?

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Jason Kratzer [:jkratzer] from comment #4)

:nfroyd, any chance this is something you could review or happen to know who this might fall under?

I can review this, sure. The patch doesn't strike me as exactly right; as Ted said in comment 2, this functionality is really only intended for files that get compiled and can therefore show up in crash reports, and .webidl files are not exactly compiled in the traditional sense.

That being said, I see that we also count .profdata (probably not compiled) and node.stub (also...not compiled) files as "compiled" sources, so I suppose there's prior art in this area. And so the patch seems fine on its own.

What I don't understand is why you want to do this; comment 0 isn't illuminating in this respect. Can you explain?

Comment 6

[Jason Kratzer [:jkratzer]]

Sure. We use the webidl files to automatically update some of our DOM fuzzers. Currently, we pull all of the static webidls from git/hg. The CSS2Properties.webidl however, would require us to build each time we wanted to update. Adding this to generated-files archive seemed like a good compromise due to the low increase and file size and reduced complexity on our end.

I'm not sure if it's even possible but if so, we could ifdef this only for fuzzing builds.

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to Jason Kratzer [:jkratzer] from comment #6)

Sure. We use the webidl files to automatically update some of our DOM fuzzers. Currently, we pull all of the static webidls from git/hg. The CSS2Properties.webidl however, would require us to build each time we wanted to update. Adding this to generated-files archive seemed like a good compromise due to the low increase and file size and reduced complexity on our end.

OK, this seems reasonable. Please submit a patch in phab with the above explanation or something like it in the commit message, and r? me. Thanks!

Comment 8

[Jason Kratzer [:jkratzer]]

Attachment: Bug 1513071 - Include CSS2Properties.webidl in generated-files required by DOM fuzzer. r?froydnj

Comment 9

[BugBot [:suhaib / :marco]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jkratzer, could you have a look please?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c1cef5b46db
Include CSS2Properties.webidl in generated-files required by DOM fuzzer. r=froydnj

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9c1cef5b46db