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

RESOLVED FIXED in Firefox 68

Status

enhancement
RESOLVED FIXED
8 months ago
5 months ago

People

(Reporter: jkratzer, Assigned: jkratzer)

Tracking

Trunk
mozilla68

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments)

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?
Needinfo for :ted because he implemented the original generated-files archive from what I can see :)
Flags: needinfo?(ted)
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.
Flags: needinfo?(ted)

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

Assignee: nobody → jkratzer

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

Flags: needinfo?(nfroyd)

(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?

Flags: needinfo?(nfroyd)

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.

(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!

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?

Flags: needinfo?(jkratzer)
Flags: needinfo?(jkratzer)
Keywords: checkin-needed

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c1cef5b46db
Include CSS2Properties.webidl in generated-files required by DOM fuzzer. r=froydnj

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.