1337391 - certdata.c isn't generated when building the export tier

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(no longer active)]

STR:

$ ./mach clobber
$ ./mach configure &$& ./mach build-backend
$ make -C objdir pre-export && make -C objdir export
$ ls objdir/security/nss/lib/ckfw/builtins/builtins_nssckbi/certdata.c # The file isn't found.
$ cd objdir/security/nss/lib/ckfw/builtins/builtins_nssckbi && make export
$ ls certdata.c # The file is found now.

As far as I can tell, this is caused by "security/nss/lib/ckfw/builtins/builtins_nssckbi/target" not existing in the export_dirs variable in root.mk, and only in compile_dirs.

Ted, can you please take a look?  Thanks!

Comment 1

[(not currently active) Ted Mielczarek]

We map the gyp action directly to a moz.build GENERATED_FILES:
https://dxr.mozilla.org/mozilla-central/rev/f4f374622111022d41dd8d5eb9220624135c534a/python/mozbuild/mozbuild/frontend/gyp_reader.py#94

And GENERATED_FILES are supposed to be built in the export tier. The recursivemake backend has code specifically to look at GENERATED_FILES to do this:
https://dxr.mozilla.org/mozilla-central/rev/f4f374622111022d41dd8d5eb9220624135c534a/python/mozbuild/mozbuild/backend/recursivemake.py#510

Unfortunately I'm not super familiar with the way the recursivemake backend works beyond that these days, so I don't know what exactly is going wrong there. glandium or one of the other build peers would be better equipped to handle that question.

Comment 2

[Mike Hommey [:glandium]]

Relying on tiers is not future-proof. Nothing guarantees that something will be built in a specific tier forever. As a matter of fact, this is what is happening here. If you need that file to be built, use mach build security/nss/lib/cfkw/builtins/builtins_nssckbi/certdata.c, but even that is not really future-proof either: the path might (and will likely) change.

Comment 3

[(no longer active)]

The use case here is to ensure that all of the headers and the generated sources are current (basically everything that a C++ compiler consumes.  The third command in comment 0 achieves this, except for this one file in the whole tree...

Comment 4

[Mike Hommey [:glandium]]

Turns out there's actually a nasty bug hidden, triggered by the gyp code. Never was a problem before nss got gypped.

Comment 5

[Mike Hommey [:glandium]]

This is actually a regression from bug 1308982.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1337391 - Don't skip all directories that aren't traversed with a DIRS in a moz.build file.

The way directory traversal is computed relies on the
RecursiveMakeTraversal class, which is used to reproduce the old
traversal order from the old entirely-in-make traversal with DIRS,
PARALLEL_DIRS, etc. because of the undeclared intra-directory
dependencies that are looming here and there.

It's fed through DirectoryTraversal objects emitted by the frontend.
Normally, DirectoryTraversal objects are emitted for a directory,
possibly giving the subdirectories defined in DIRS/TEST_DIRS its
moz.build. But in the case of gyp processing, nothing places the gyp
objdirs in some virtual DIRS of some parent moz.build since bug 1308982.

As a consequence, the corresponding entries in the
RecursiveMakeTraversal instance attached to the backend are not attached
to any parent directory. When subsequently traversing the tree from the
root, they are never found, and end up being skipped, irregarding of
their actual _no_skip status.

It would probably be possible to revert the changes from bug 1308992,
but we might as well not rely on remains from the old ways. So instead,
we make the RecursiveMakeTraversal consider directories without a
declared parent attached directly to the root directory. They don't need
to depend on any other directory anyways.

Review commit: https://reviewboard.mozilla.org/r/111000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111000/

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8835310 [details]
Bug 1337391 - Don't skip all directories that aren't traversed with a DIRS in a moz.build file.

https://reviewboard.mozilla.org/r/111000/#review112546

Regarding the commit message, this was broken prior to bug 1308982 -- the patch there codified the existing (incorrect) behavior.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8857aa03d1a5
Don't skip all directories that aren't traversed with a DIRS in a moz.build file. r=chmanchester

Comment 9

[(no longer active)]

Thanks for the quick fix!

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8857aa03d1a5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Looks like this needs an Aurora approval request as well.

Comment 12

[Mike Hommey [:glandium]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #11)
> Looks like this needs an Aurora approval request as well.

It doesn't _actually_ matter enough for an uplift. It only affects intermediary files in NSS, and those files are being depended upon through other means, and are built correctly through a full build.

(That is, except if Ehsan is planning on trying to do the same thing with aurora)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Ehsan, how much do you care about whether this reaches Aurora or not?

Comment 14

[(no longer active)]

No not at all, it's OK to not backport this.