Closed Bug 1660396 Opened 4 months ago Closed 3 months ago

Wildcards in FinalTargetFiles are broken in the recursivemake backend

Categories

(Firefox Build System :: General, defect)

defect

Tracking

(firefox82 fixed)

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: glandium, Assigned: mhentges)

Details

Attachments

(2 files)

Bug 1643776 was adding something like:

EXTRA_JS_MODULES.themes['alpenglow'] += [
    '*.svg',
]

in browser/themes/addons/alpenglow/moz.build

This ended up installing the files under dist/bin/modules/themes/alpenglow on normal builds, which is wrong because of the moz.build being under browser/. They should have ended in dist/bin/browser/modules/themes/alpenglow, which is what happens with the fastermake backend, meaning artifact builds are not affected.

While looking into this, I also spotted that handling of FinalTargetFiles in fastermake doesn't ensure the wildcard is on the file part of the path (which the recursivemake backend does)

Mitch, do you have cycles to look into this?

Flags: needinfo?(mhentges)
Assignee: nobody → mhentges
Status: NEW → ASSIGNED
Flags: needinfo?(mhentges)

I'll add it to the stack, I bet I can dig in within the next week

Relative targets (e.g.: on top of "dist/bin") were being ignored when
the target path had a wildcard.

While looking into this, I also spotted that handling of FinalTargetFiles in fastermake doesn't ensure the wildcard is on the file part of the path (which the recursivemake backend does)

Digging in, this may be by design.
When a JARManifest is encountered, recursivemake just writes the manifest path while fastermake will interpret the manifest, handling the contents.
There are paths within the manifest that are both handled as FinalTargetFiles and have directory (non-file) wildcards, such as here (the handled path here is en-US/security/**/*.ftl)
In this case, fastermake has to handle such a situation, while recursivemake can safely ignore it.

Due to this^, the correct change to make here isn't clear to me. Is there still something I can solve here :glandium?

Flags: needinfo?(mh+mozilla)

There's a usage of mozpath.join(...) found while investigating
wildcard-management. Since the return value isn't here, and the
parameters here aren't mutated, this function call doesn't do anything.

Pushed by mhentges@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b264fdabef48
RecursiveMake should support wildcards with reltargets r=firefox-build-system-reviewers,glandium

(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #3)

While looking into this, I also spotted that handling of FinalTargetFiles in fastermake doesn't ensure the wildcard is on the file part of the path (which the recursivemake backend does)

Digging in, this may be by design.
When a JARManifest is encountered, recursivemake just writes the manifest path while fastermake will interpret the manifest, handling the contents.
There are paths within the manifest that are both handled as FinalTargetFiles and have directory (non-file) wildcards, such as here (the handled path here is en-US/security/**/*.ftl)
In this case, fastermake has to handle such a situation, while recursivemake can safely ignore it.

Due to this^, the correct change to make here isn't clear to me. Is there still something I can solve here :glandium?

Oh nice find, you're totally right.

Flags: needinfo?(mh+mozilla)
Pushed by mhentges@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ea188261f16
Removes no-op path operation r=firefox-build-system-reviewers,glandium
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.