1335309 - Make FileFinder's find_executables False by default

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Back when the class was written, for the packaging code, it made sense that the default was True. But now that it's used all over the place, and that the vast majority of uses are with find_executables=False, it makes more sense for that to be the default.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1335309 - Add explicit find_executables arguments to every use of FileFinder.

And make it an error not to give it. While the default is True, we do
pass a value of False wherever it makes sense, which happens to be, in
most places.

This is an intermediate step to flip the default from True to False,
ensuring that we don't unwantedly switch some calls to False.

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

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1335309 - Change the default for find_executables to False.

Back when the class was written, for the packaging code, it made sense
that the default was True. But now that it's used all over the place,
and that the vast majority of uses are with find_executables=False, it
makes more sense for that to be the default.

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

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8831947 [details]
Bug 1335309 - Add explicit find_executables arguments to every use of FileFinder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108418/diff/1-2/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8831948 [details]
Bug 1335309 - Change the default for find_executables to False.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108420/diff/1-2/

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8831947 [details]
Bug 1335309 - Add explicit find_executables arguments to every use of FileFinder.

https://reviewboard.mozilla.org/r/108418/#review109640

I think this looks good. It's a little hard to verify that for the cases where we switched from the default of True to an explicit False that they don't end up relying on ExecutableFile somewhere, but as you say it looks like that should only be the packager and strip.

Comment 6

[Michael Shal [:mshal]]

Comment on attachment 8831948 [details]
Bug 1335309 - Change the default for find_executables to False.

https://reviewboard.mozilla.org/r/108420/#review109642

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41b0870f79dc
Add explicit find_executables arguments to every use of FileFinder. r=mshal
https://hg.mozilla.org/integration/autoland/rev/e9ba8f265022
Change the default for find_executables to False. r=mshal

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/41b0870f79dc
https://hg.mozilla.org/mozilla-central/rev/e9ba8f265022