New support-files not noticed when using wildcards

RESOLVED FIXED in mozilla29

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: jryans, Assigned: gps)

Tracking

(Blocks 1 bug)

Trunk
mozilla29
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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
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.
Component: mach → Build Config
Duplicate of this bug: 946945
Blocks: clobber
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.
Attachment #8344481 - Flags: review?(mh+mozilla)
Assignee: nobody → gps
Status: NEW → ASSIGNED
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
Attachment #8344512 - Flags: review?(mh+mozilla)
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.
Attachment #8344481 - Flags: review?(mh+mozilla) → review+
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]?
Attachment #8344512 - Flags: review?(mh+mozilla) → review+
Blocks: 947879
No longer blocks: 947879
Duplicate of this bug: 947879
Part 2 and 2b with fix for test failure. Uploading for checkin-needed
because the tree is closed.
Attachment #8344512 - Attachment is obsolete: true
Attachment #8344481 - Flags: checkin+
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.
Attachment #8345016 - Flags: review+
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/7300b41e4b1f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Blocks: 949205
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.