Closed Bug 1440879 Opened 2 years ago Closed Last year

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

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files)

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
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...
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.
Product: Core → Firefox Build System
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.
Flags: needinfo?(ted)
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)
Flags: needinfo?(nfroyd)
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?
Flags: needinfo?(nfroyd)
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
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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) (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.
Attachment #9005812 - Flags: review?(nfroyd) → review+
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?
Attachment #9005814 - Flags: review?(nfroyd) → review+
Pushed by kgupta@mozilla.com:
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
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
Flags: needinfo?(kats)
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/765f2d8c3340
Backed out 2 changesets for build bustages on mozbuild\test\backend. CLOSED TREE
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
Flags: needinfo?(kats)
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.
Pushed by kgupta@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/f8a8bc1f8bc1
https://hg.mozilla.org/mozilla-central/rev/d547743a6b80
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.