934739 - New support-files not noticed when using wildcards

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

The App Manager's chrome.ini[1] adds an entire directory and all subdirectories as support-files:

[DEFAULT]
support-files =
  hosted_app.manifest
  validator/*

The "validator" directory itself contains 3 more directories.  I already had a fully built tree, and then I added a new fourth directory under "validator" (with its own files).

Both "mach build" and "mach mochitest-chrome" failed to copy the new directory into the test path until I touched the chrome.ini file.

Is this something that can be fixed so it just works without touching chrome.ini, or should we not be using this syntax so you are forced to add each file to support-files?

[1]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/app-manager/test/chrome.ini

Comment 1

[Gregory Szorc [:gps]]

If you want all recursive entries in a directory tree, use **. See the documentation at https://ci.mozilla.org/job/mozilla-central-docs/Build_Documentation/test_manifests.html.

But, there is likely a bug here. Wildcards are expanded at build config time, not run-time. So if you add a new file, the build config won't know about this file until the build config is rescanned. Build config is rescanned automatically if an input file (such as the chrome.ini) changes. And this is why wildcards aren't fun: they require run-time scanning, which incurs a perf penalty. Enumerating each file is highly recommended.

To work around this problem, touch chrome.ini or run |mach build-backend| to force a build config scan.

We should consider offering a build mode that always rescans the build config.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Part 1: Make moz.build backend generation depend on all Python modules

I plan to solve this by updating mozpack to support wildcards in install
manifest entries. This will require bumping the manifest version number.
I discovered that changes to mozpack.* modules weren't requiring a
backend regeneration. This patch fixes that by ensuring all loaded
Python modules are listed as config.status dependencies.

Comment 4

[Gregory Szorc [:gps]]

Attachment: Part 2: Add pattern matches to manifests, resolve at run time

This patch adds pattern matching entries to install manifests. We store
metadata necessary to construct a pattern match at a later point in
time. When we convert the install manifest to a file registry, we
resolve the patterns using FileFinder.

The build config logic has been updated to store support-files values as
pattern entries. This should resolve the clobber needed issue and make
the local development experience more pleasant as well.

https://tbpl.mozilla.org/?tree=Try&rev=c56fca460890

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8344481 [details] [diff] [review]
Part 1: Make moz.build backend generation depend on all Python modules

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

I had roughly the same thing locally.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8344512 [details] [diff] [review]
Part 2: Add pattern matches to manifests, resolve at run time

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

::: python/mozbuild/mozpack/manifests.py
@@ +79,5 @@
>      COPY = 2
>      REQUIRED_EXISTS = 3
>      OPTIONAL_EXISTS = 4
> +    PATTERN_SYMLINK = 5
> +    PATTERN_COPY = 6

I'm not convinced we need copy here.

@@ +219,5 @@
>  
> +    def add_pattern_symlink(self, base, pattern, dest):
> +        """Add a pattern match that results in symlinks being created.
> +
> +        A ``FileFinder`` will be created with it's base set to ``base``

s/it's/its/

@@ +272,5 @@
>              if install_type == self.OPTIONAL_EXISTS:
>                  registry.add(dest, ExistingFile(required=False))
>                  continue
>  
> +            if install_type in (self.PATTERN_SYMLINK, self.PATTERN_COPY):

_, base, pattern, dest = entry, and replace entry[n]?

Comment 7

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a58048028f70
https://hg.mozilla.org/integration/mozilla-inbound/rev/17a5f2e554e8

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/572624d76edf

bustage fix

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a58048028f70

Part 2/2b were backed out due to bug 947879.
https://hg.mozilla.org/mozilla-central/rev/ff4ad5485e45

Comment 11

[Gregory Szorc [:gps]]

Attachment: Part 2: Add pattern matches to install manifests; r=glandium

Part 2 and 2b with fix for test failure. Uploading for checkin-needed
because the tree is closed.

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8345016 [details] [diff] [review]
Part 2: Add pattern matches to install manifests; r=glandium

Carry over r+. IRC r+ for bustage fix.

Comment 13

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7300b41e4b1f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7300b41e4b1f