1419055 - Fix FasterMake backend handling of FINAL_TARGET_FILES with wildcards in filename

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[(not currently active) Ted Mielczarek]

The patch in bug 1407374 adds `LOCALIZED_FILES` entries with wildcards in the name, which are handled by FasterMake the same way as `FINAL_TARGET_FILES`. It turns out that FasterMake's handling of wildcards in filenames is broken.

I guess this isn't a construct we actually use in-tree currently. It's broken with entries like:
```
FINAL_TARGET_FILES += ['*.xyz']
```

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: bug 1419055 - Fix FasterMake backend handling of FINAL_TARGET_FILES with wildcards in filename.

This patch fixes a bug in the FasterMake backend and adds a new
test_fastermake.py file to add a test for the behavior.

The FasterMake backend didn't handle wildcards in file names present in
FINAL_TARGET_FILES properly. For an entry like:

FINAL_TARGET_FILES.foo += ['*.xyz']

It would wind up trying to install the files to `dist/bin/foo/*.xyz/`, a
path with a literal asterisk in it. The code seems to have been written
assuming that wildcards would only be present in directory components of
the path. This change fixes this specific case, although it's possible that
it still doesn't handle all permutations of wildcards properly.

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

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8930124 [details]
bug 1419055 - Fix FasterMake backend handling of FINAL_TARGET_FILES with wildcards in filename.

https://reviewboard.mozilla.org/r/201292/#review206450

lgtm.  Just a case that wasn't consider, I guess.

Comment 3

[Nathan Froyd [:froydnj]]

Do we want to make the emitter smarter about these kinds of cases, so it's more difficult to botch things in the backend?  Surely the FasterMake backend will not be the only one to run into problems like this.

Comment 4

[(not currently active) Ted Mielczarek]

I am not opposed to that, I just don't know what it'd look like precisely. I guess having some sort of `Pattern` class would be useful.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8930124 [details]
bug 1419055 - Fix FasterMake backend handling of FINAL_TARGET_FILES with wildcards in filename.

https://reviewboard.mozilla.org/r/201292/#review206820

::: python/mozbuild/mozbuild/backend/fastermake.py:82
(Diff revision 1)
>  
> +                        if '*' in f.target_basename:
> +                            target = path
> +                        else:
> +                            target = mozpath.join(path, f.target_basename)
> +                        mozpath.join(path, f.target_basename)

Dead code

Comment 6

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bca7bf9ad9bf54e1c0e96c2dd8be27f61c50cfbd
bug 1419055 - Fix FasterMake backend handling of FINAL_TARGET_FILES with wildcards in filename. r=nalexander

Comment 7

[(not currently active) Ted Mielczarek]

(In reply to :froydnj (on leave until 2018, ni? or email if necessary) from comment #3)
> Do we want to make the emitter smarter about these kinds of cases, so it's
> more difficult to botch things in the backend?  Surely the FasterMake
> backend will not be the only one to run into problems like this.

I filed bug 1419489 to discuss this.

Comment 8

[Cristina Coroiu [:ccoroiu]]

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