660687 - Configure options to change MOZ_APP_BASENAME and disable system extensions

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Zack Weinberg (:zwol)]

Attachment: patch

This patch adds configure options that can change MOZ_APP_BASENAME (like the existing option that changes MOZ_APP_NAME, only more so) and disable searching any directories outside the current profile for extensions.

I have had a local patch that does these things unconditionally for some time (because of the inappropriate WONTFIXing of bug 443078) but recently that's started to make try server builds break (see bug 657448).  This way I can get the same effect with .mozconfig options that affect only my local builds, not the try server; and for the same reason, I hope it will be uncontroversial enough that it can be landed and I can stop carrying it around.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #0)
> This way I can get the same effect with .mozconfig options that affect only my
> local builds, not the try server;

You don't need to modify MOZ_APP_BASENAME to achieve that, you can already just set MOZ_APP_PROFILE in your mozconfig to a custom value.

Comment 2

[Zack Weinberg (:zwol)]

(In reply to comment #1)
> 
> You don't need to modify MOZ_APP_BASENAME to achieve that, you can already
> just set MOZ_APP_PROFILE in your mozconfig to a custom value.

That's good to know, but it doesn't do everything I need.  Most importantly, the startup process (under X11 at least) looks for windows belonging to gAppData->name to decide whether there's a running instance that it should delegate to.  So to ensure that "./dist/bin/minefield TESTCASE" in my build directory opens the test case in the thing I just built and not some other browser that happens to be running, I gotta change the Name= field of application.ini, and that's what changing MOZ_APP_BASENAME does.

(Yes, I know about -no-remote, but I do not think I should have to remember to type that every time.)

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> That's good to know, but it doesn't do everything I need.  Most importantly,
> the startup process (under X11 at least) looks for windows belonging to
> gAppData->name to decide whether there's a running instance that it should
> delegate to.

MOZ_ENABLE_XREMOTE= ?

Comment 4

[Zack Weinberg (:zwol)]

... I'd really rather not play whack-a-mole with an ever growing list of things that have to be turned off or tweaked.  Changing MOZ_APP_BASENAME covers everything that needs covered now, and offers confidence that it will also cover any new thing that comes up in the future.

Is there a reason you don't want this option?  We already have control over MOZ_APP_NAME from configure, so it seems like a logical extension to me.

Comment 5

[Zack Weinberg (:zwol)]

Also, do you have an opinion on the other half of the patch?

Comment 6

[Mike Hommey [:glandium]]

If I'm not mistaken, you should be able to set these variables with mk_add_options, which wouldn't require a configure option for each of them.

Comment 7

[Zack Weinberg (:zwol)]

I'll see if setting MOZ_APP_BASENAME with mk_add_options works when I get home (cycles take too long on this laptop).  The other option triggers #ifdeffage, I don't think that can be done without a configure switch.

Comment 8

[Mike Hommey [:glandium]]

The only branding variable that I expect not to work with mk_add_options is MOZ_APP_UA_NAME.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

I am generally averse to adding more configure flags unnecessarily, and your use case seems rather uncommon.

I don't have any opinion about the other hunk of the patch.

Comment 10

[Mike Hommey [:glandium]]

The other hunk of the patch might be more useful as a runtime switch, though I don't have a strong opinion there.

Comment 11

[Zack Weinberg (:zwol)]

(In reply to comment #10)
> The other hunk of the patch might be more useful as a runtime switch, though
> I don't have a strong opinion there.

I can see the value there, and I'd be happy to change the patch around if there's consensus, but I'd still want a way to control the default for that runtime switch at build time.

Comment 12

[Zack Weinberg (:zwol)]

(In reply to comment #8)
> The only branding variable that I expect not to work with mk_add_options is
> MOZ_APP_UA_NAME.

mk_add_options MOZ_APP_BASENAME=Minefield

has no effect.  MOZ_APP_BASENAME does make it into .mozconfig.mk, but <builddir>/config/autoconf.mk still has MOZ_APP_BASENAME=Firefox, as does config.status. If the proper incantation is different please let me know what it is.

Comment 13

[Mike Hommey [:glandium]]

Somehow, I thought the mk_add_options variables were passed to the make process in charge of building, but it's not true. I wonder if that wouldn't be useful to include .mozconfig.mk from config/config.mk... (Kyle? Ted?)
Anyways, you should still be able to do what I was thinking of, just a bit differently:
mk_add_options MOZ_MAKE_FLAGS="MOZ_APP_BASENAME=Minefield"

Comment 14

[Zack Weinberg (:zwol)]

You'll forgive me if I think the additional configure option is more elegant.

Comment 15

[Mike Hommey [:glandium]]

(In reply to comment #14)
> You'll forgive me if I think the additional configure option is more elegant.

There are just better ways to do that, not involving the somehow awful config I'm suggesting. One is to include .mozconfig.mk from config/config.mk as I wrote in comment 13. Another is to avoid branding confvars.sh to override preexisting values of the variables it sets.

Comment 16

[(not currently active) Ted Mielczarek]

Comment on attachment 536144 [details] [diff] [review]
patch

Review of attachment 536144 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I've left this sitting around because I wasn't sure about it, but it's harmless enough and if it makes your life easier then let's just take it.

Comment 17

[Zack Weinberg (:zwol)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb466cf5e01

Comment 18

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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

Comment 19

[Ben Bucksch (:BenB)]

Can't depend, because it's already fixed.
Bug 723493 is about giving this without recompilation.

Comment 20

[Zack Weinberg (:zwol)]

For the record, I don't actually care about bug 723493 except insofar as I was worried that bug 686466 might have *regressed* the feature I added here, which it hasn't.