1440879 - generated-sources.json doesn't contain all the generated files that it should

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

If I look in my $OBJDIR/generated-sources.json file, it contains 4 entries under toolkit/components/telemetry/:

        "toolkit/components/telemetry/TelemetryEventEnums.h",
        "toolkit/components/telemetry/TelemetryHistogramEnums.h",
        "toolkit/components/telemetry/TelemetryProcessEnums.h",
        "toolkit/components/telemetry/TelemetryScalarEnums.h",

However looking at the moz.build file, there are other files that are generated, e.g. TelemetryEventData.h [1]. Why does this file not get listed in generated-sources.json?

Another example is the 4 generated files at [2], none of which show up in my generated-sources.json either.

[1] https://searchfox.org/mozilla-central/rev/88e89261ea81860f44a44ed068cf5839aea5def6/toolkit/components/telemetry/moz.build#166
[2] https://searchfox.org/mozilla-central/rev/88e89261ea81860f44a44ed068cf5839aea5def6/security/apps/moz.build#41-44,48

Comment 1

[Nathan Froyd [:froydnj]]

Probably because the four entries you listed are explicitly mentioned in EXPORTS:

https://searchfox.org/mozilla-central/rev/88e89261ea81860f44a44ed068cf5839aea5def6/toolkit/components/telemetry/moz.build#48-51

And EXPORTS has special handling for generated headers:

https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/backend/common.py#157-161

but we don't have anything for generic generated things that aren't exported or otherwise built.  We probably should...

Comment 2

[(not currently active) Ted Mielczarek]

Yeah, I noted that case when I implemented this: bug 1384568 comment 1. The code handling this is not very complicated, as froydnj points out. It would probably be fine to have a whitelist of extensions that look like source code and treat anything in `GENERATED_FILES` with one of those extensions as a generated source file.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Blindly groping my way around the build system... https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee8b8628ffd9c30387440c21fa8d3373fe371af5

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Ted, does the patch in the above try push looking like it's in the right direction? If so, do you know how to address the build error? The GeneratedFile entry for the certdata.c file it's complaining about seems to be coming from a .gyp file and I don't know where the actual file ends up or how to add its path to generated-sources.json.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Nathan, any thoughts here? This is pretty much the last thing blocking me from expanding support to include macOS code in searchfox. (i.e. this blocks bug 1425597 which blocks bug 1487583)

Comment 6

[Nathan Froyd [:froydnj]]

Applying your patch locally, I see:

        "security/nss/lib/ckfw/builtins/builtins_nssckbi/certdata.c", 
        "security/nss/lib/ckfw/builtins/certdata.c", 

in generated-sources.json.  The first one is correct; there's an actual backend.mk file in said directory that generates that file.  The second one is an utter lie.  I don't really understand how that works, because the bit that actually generates certdata.c:

https://searchfox.org/mozilla-central/source/security/nss/lib/ckfw/builtins/builtins.gyp#29-48

doesn't make any mention of this builtins_nssckbi directory.  I guess that directory is coming from within the gyp reader?

I think the problem is in your backend/common.py hunk.  You're creating a generic Path object, which somehow resolves relative to the relative srcdir, i.e. security/nss/lib/ckfw/builtins/.  Instead, what you want is for it to resolve relative to the relative objdir, so what I think you want is:

+        elif isinstance(obj, GeneratedFile):
+            if obj.required_for_compile:
+                for f in obj.required_for_compile:
+                    fullpath = ObjDirPath(obj._context, '!' + f).full_path
+                    relpath = mozpath.relpath(fullpath, obj._context.config.topobjdir)
+                    self._handle_generated_sources([relpath])
+            return False

Notice the use of the ObjDirPath there, and relpath'ing things relative to the topobjdir.  This change appears to DTRT for me as far as certdata.c goes, and other paths in generated-sources.json come out looking OK.  Try that instead?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Thanks! That's looking much better. Try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee64db93dcc149da9313460317257b8c42eec5b2&selectedJob=196937398

The files that are still "missing" (i.e. that we have C++ analysis data for but are not included in the generated-files tarball) are:

./dist/include/buildid.h
./dist/include/mozilla-config.h
./dist/stl_wrappers/cstring
./dist/stl_wrappers/ios
./dist/stl_wrappers/cstdlib
./dist/stl_wrappers/iosfwd
./dist/stl_wrappers/climits
./dist/stl_wrappers/iterator
./dist/stl_wrappers/memory
./dist/stl_wrappers/algorithm
./dist/stl_wrappers/atomic
./dist/stl_wrappers/unordered_set
./dist/stl_wrappers/cstdarg
./dist/stl_wrappers/type_traits
./dist/stl_wrappers/istream
./dist/stl_wrappers/limits
./dist/stl_wrappers/ostream
./dist/stl_wrappers/stack
./dist/stl_wrappers/cassert
./dist/stl_wrappers/tuple
./dist/stl_wrappers/functional
./dist/stl_wrappers/utility
./dist/stl_wrappers/vector
./dist/stl_wrappers/cwchar
./dist/stl_wrappers/cstdio
./dist/stl_wrappers/iostream
./dist/stl_wrappers/set
./dist/stl_wrappers/unordered_map
./dist/stl_wrappers/cmath
./dist/stl_wrappers/thread
./dist/stl_wrappers/list
./dist/stl_wrappers/string
./dist/stl_wrappers/deque
./dist/stl_wrappers/map

I notice in particular that dist/stl_wrappers/new *is* included in the generated files tarball (presumably from [1]). I'll try and figure out why the first two entries files aren't ending up in the generated sources.

[1] https://searchfox.org/mozilla-central/rev/43969de34f5d4b113133d090f024e5eed7a82af0/python/mozbuild/mozbuild/frontend/data.py#1171

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Hm, we do get a buildid.h and mozilla-config.h at the root of the tree in the generated-files tarball, just not in dist/include. It looks like those files exist both in $OBJDIR/ and in $OBJDIR/dist/include/ for some reason.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Add GeneratedFile entries for source-ish files into generated-sources.json

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Include all the stl_wrappers headers in the generated-files tarball

Comment 11

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) (parental leave) from comment #8)
> Hm, we do get a buildid.h and mozilla-config.h at the root of the tree in
> the generated-files tarball, just not in dist/include. It looks like those
> files exist both in $OBJDIR/ and in $OBJDIR/dist/include/ for some reason.

I can deal with this on the searchfox side easily enough. Although ideally the build system wouldn't generate two copies of these files into the objdir.

Comment 12

[Nathan Froyd [:froydnj]]

Comment on attachment 9005814 [details] [diff] [review]
Part 2 - Include all the stl_wrappers headers in the generated-files tarball

Review of attachment 9005814 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/frontend/data.py
@@ +1171,2 @@
>          )
> +        self.required_for_compile = [f for f in self.outputs if f.endswith(suffixes) or 'stl_wrappers/' in f]

This is a hack, but I don't have better ideas on how to address this currently.  Can you add a comment in config/moz.build hereish:

https://searchfox.org/mozilla-central/source/config/moz.build#49-60

saying that the stl_wrappers bit is known to the build system, so it needs to be changed in two places?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/71d569322700
Emit source-like generated files into generated-sources.json. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/93892cfed015
Ensure all the stl_wrappers end up in the generated-files tarball. r=froydnj

Comment 14

[Narcis Beleuzu [:NarcisB]]

Backed out for build bustages on mozbuild\test\backend.

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=197385372&repo=mozilla-inbound&lineNumber=44350

Comment 15

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/765f2d8c3340
Backed out 2 changesets for build bustages on mozbuild\test\backend. CLOSED TREE

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Snippet of failure output from the log above:

15:31:31     INFO - ..\python\mozbuild\mozbuild\backend\common.py:322: in _handle_generated_sources
15:31:31     INFO -     self._generated_sources.update(mozpath.relpath(f, self.environment.topobjdir) for f in files)
15:31:31     INFO - ..\python\mozbuild\mozbuild\backend\common.py:322: in <genexpr>
15:31:31     INFO -     self._generated_sources.update(mozpath.relpath(f, self.environment.topobjdir) for f in files)
15:31:31     INFO - ..\python\mozbuild\mozpack\path.py:31: in relpath
15:31:31     INFO -     rel = normsep(os.path.relpath(path, start))
15:31:31     INFO - _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
15:31:31     INFO - path = 'bar.h', start = 'c:/users/task_1536069441/appdata/local/temp/tmpuxfklt'

This seems like self.environment.topobjdir is some temp folder on c: at [1] which seems wrong. The objdir should be on z: which is where the builder is doing things and where the source checkout lives. Unless the test runs with some temporary objdir? In that case I think the temp objdir should be on z: as well because that's what the "current drive" is and what the path 'bar.h' is interpreted as being on.

[1] https://hg.mozilla.org/integration/mozilla-inbound/file/71d569322700/python/mozbuild/mozbuild/backend/common.py#l322

Comment 17

[Kartikaya Gupta (email:[e-mail])]

But I guess maybe bar.h should be an absolute path. Other places that call _handle_generated_sources seem to always pass an absolute path. I'll try that.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Yeah, that seems to have worked: https://treeherder.mozilla.org/#/jobs?repo=try&bugfiler=&group_state=expanded&revision=2e4ee165e8beba898a35442877ed06b1f54c8fc5

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8a8bc1f8bc1
Emit source-like generated files into generated-sources.json. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/d547743a6b80
Ensure all the stl_wrappers end up in the generated-files tarball. r=froydnj

Comment 20

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f8a8bc1f8bc1
https://hg.mozilla.org/mozilla-central/rev/d547743a6b80