Create an eslint-build tester (tier 3) that depends on xpt artifacts - for checking the use of idl constants to ensure they are valid
Categories
(Firefox Build System :: Task Configuration, task)
Tracking
(firefox109 fixed)
Tracking | Status | |
---|---|---|
firefox109 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
(Blocks 2 open bugs)
Details
Attachments
(5 files, 3 obsolete files)
For a while now, we have had a linter rule to validate that any idl constants used from Javascript are defined. This is currently run manually, and tends to pick up something that's broken once every couple of months.
We can't include this as part of the automatic ESLint, as it depends on the generated xpidl output, so would typcially require an opt build.
We would like to include this as a tier-2 task, that is run once a day. Picking up the failing patches is normally quite easy.
The basic process to run this would be:
- Get the source.
- Do an opt build, or get an opt build where we have the generated xpidl output.
- Run ESLint with a special command that enables the extra rule.
Comment 1•4 years ago
|
||
:kats, I'd be interested in getting this over the line. I'm assuming searchfox has similar needs in terms of the xpt files - are these already exposed somewhere?
Comment 2•4 years ago
|
||
I'm unfamiliar with xpt files specifically, and I don't see any in searchfox. I do see some in my objdir/config/makefiles/xpidl/
folder - is that what you're referring to?
In general the searchfox task (e.g. this one) has a generated-files artifact that contains generated files that we want to index. But I don't see the xpt files in there, so maybe that's not useful to you. Searchfox also runs some sort of xpidl parsing directly, in this script but I suspect that is also not useful to you.
Comment 3•4 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
I'm unfamiliar with xpt files specifically, and I don't see any in searchfox. I do see some in my
objdir/config/makefiles/xpidl/
folder - is that what you're referring to?
Yep.
In general the searchfox task (e.g. this one) has a generated-files artifact that contains generated files that we want to index. But I don't see the xpt files in there, so maybe that's not useful to you. Searchfox also runs some sort of xpidl parsing directly, in this script but I suspect that is also not useful to you.
Hm, yeah - but it would seem that we could maybe add something to https://searchfox.org/mozilla-central/rev/35d927df97900a57ecb562ad13909e392440b0fb/toolkit/mozapps/installer/packager.mk#77-93 to package up the xpt files so they too appear as artifacts on this job? With that it should be possible to do this, I think.
Mark, in comment #0 you explicitly mentioned opt builds, but afaict these are debug builds - does that make a difference for the linter? I wouldn't have thought so, but I'm probably missing something...
Comment 4•4 years ago
|
||
Alternatively, I guess with a src dir only generating the xpidl xpt info and nothing else shouldn't actually be that difficult - the xpidl thing is just a python parser, IIRC?
Assignee | ||
Comment 5•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #3)
Mark, in comment #0 you explicitly mentioned opt builds, but afaict these are debug builds - does that make a difference for the linter? I wouldn't have thought so, but I'm probably missing something...
Oh, I think I was thinking "opt" vs "artifact". I don't think it would really matter.
(In reply to :Gijs (he/him) from comment #4)
Alternatively, I guess with a src dir only generating the xpidl xpt info and nothing else shouldn't actually be that difficult - the xpidl thing is just a python parser, IIRC?
If we can do that, it'd probably be nice, easy and quick-ish.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 6•2 years ago
|
||
Nick says he's thought about this a while ago.
Some more context about where we're at:
- We now have an ESLint command that can be run locally with an optimised build:
MOZ_OBJDIR=objdir-ff-opt ./mach eslint --rule "mozilla/valid-services-property: error" --rule="mozilla/valid-ci-uses: error" *
- We would like to get this run as a tier 2 builder on CI, so that new issues cause bugs to be filed for regressions.
- For these rules to run, we need access to the
xpt
files in <objdir>/config/makefiles/xpidl/`
Hence I think we should package those xpt files either as part of the existing tests artifact, or as a separate artifact which is downloaded when necessary. If it is tests artifact, then we might be able to make these rules work locally for artifact builds as well.
Comment 7•2 years ago
|
||
This allows to check the executable mode of iterated entries.
Comment 8•2 years ago
|
||
Depends on D161639
Comment 9•2 years ago
|
||
For the specific case of .xpt
artifacts, we could place them
directly into $topobjdir/config/makefiles/xpidl
, but I'm wary of
modifying the object directory in this manner. I'd like the initial
round of tooling building on this capability to look in
dist/xpt_artifacts
for artifact builds and be able to give detailed
feedback in error situations. We can revisit the placement of
artifacts in the future as more use cases and shortcomings are
identified.
In the future, this mechanism might be used to include Java code
generated at build-time that exposes IDL constants to GeckoView.
Depends on D161640
Comment 10•2 years ago
|
||
Mark: this seems to be working fine in automation; see, for example,
remote: https://treeherder.mozilla.org/jobs?repo=try&revision=c7e2509ad5cd63e97cfa6fde247c661380d82660
To test, use MOZ_ARTIFACT_TASK=<task ID> ./mach [build|artifact install] -v
, etc.
As noted in the commit message, out of an abundance of caution (who owns what part of the object directory when can be subtle) and to ease implementation (there are many assumptions that artifacts are written to dist
), the .xpt
files are available in dist/xpt_artifacts
. (I used xpt_artifacts
as the easy-to-grep sentinel for this idea.) Tooling can either look there and fall back to $topobjdir/config/makefiles/xpidl
, or recognize that this is an artifact build in some manner and look only in that location. As we grow usages, we can think more about populating the object directory directly.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 11•2 years ago
|
||
Here's a better try build:
https://treeherder.mozilla.org/jobs?repo=try&revision=3eb43f63b350ac8acc558162ddbf6ea085f6d4a0
I see target.xpt_artifacts.zip
that looks sensible produced by a B
job and the corresponding BA
job has:
[task 2022-11-09T00:50:36.697Z] 00:50:36 INFO - "/builds/worker/.mozbuild/package-frontend/4ac53502595d36ea-target.xpt_artifacts.zip" is a recognized extra archive (XPT Artifacts)
[task 2022-11-09T00:50:36.697Z] 00:50:36 INFO - Adding xpt_artifacts/captivedetect.xpt to processed archive
Assignee | ||
Comment 12•2 years ago
|
||
I've not been able to get it to download locally (possibly as I'm on a different head?), but it looks good on try.
Would you also be able to help with getting a task running (or know who to ask)? I tried asking on CI on matrix, but didn't get a response.
I took a look, but it appears that we'd need a new mozharness test type along with associated scripts, to do basically the equivalent of https://searchfox.org/mozilla-central/rev/6f77213807eb5359c8afe458ac5628f973e92a25/taskcluster/ci/source-test/mozlint.yml#92-100, which seems a little excessive. So maybe I got something wrong there.
Comment 13•2 years ago
|
||
On macOS, this allows the ResignJarWriter
to handle ZIP (JAR) files
correctly.
Comment 14•2 years ago
|
||
The ESlint plugin can look for XPT artifacts:
- in the location specified by the environment variable
MOZ_XPT_ARTIFACTS_DIR
(for automation pretending to be a build); - in
dist/xpt_artifacts
for builds known to be artifact builds; - in
$TOPOBJDIR/config/makefiles/xpidl
for non-artifact builds.
Depends on D161641
Comment 15•2 years ago
|
||
It's possible that the processed file has not been created.
Comment 16•2 years ago
|
||
On macOS, this allows the ResignJarWriter
to handle ZIP (JAR) files
correctly.
Depends on D162073
Comment 17•2 years ago
|
||
The ESlint plugin can look for XPT artifacts:
- in the location specified by the environment variable
MOZ_XPT_ARTIFACTS_DIR
(for automation pretending to be a build); - in
dist/xpt_artifacts
for builds known to be artifact builds; - in
$TOPOBJDIR/config/makefiles/xpidl
for non-artifact builds.
Depends on D161641
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 18•2 years ago
|
||
To help with making the landing simpler (not needing sheriff sign-offs etc), I'm going to do the initial landing as a tier 3, I'll use bug 1800874 to promote this up to a tier 2.
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 19•2 years ago
|
||
Comment 20•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2bc50f7ab10a
https://hg.mozilla.org/mozilla-central/rev/e7c65861b7ae
https://hg.mozilla.org/mozilla-central/rev/dd5f6e80ff30
https://hg.mozilla.org/mozilla-central/rev/a1e47fa15919
https://hg.mozilla.org/mozilla-central/rev/e20483a63756
Description
•