Closed Bug 934739 Opened 7 years ago Closed 7 years ago
New support-files not noticed when using wildcards
The App Manager's chrome.ini 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? : 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
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)
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+
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
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
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+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.