Configure options to change MOZ_APP_BASENAME and disable system extensions

RESOLVED FIXED in mozilla10

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: zwol, Assigned: zwol)

Tracking

Trunk
mozilla10
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 536144 [details] [diff] [review]
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.
Attachment #536144 - Flags: review?(ted.mielczarek)
(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.
(Assignee)

Comment 2

6 years ago
(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.)
(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= ?
(Assignee)

Comment 4

6 years ago
... 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.
(Assignee)

Comment 5

6 years ago
Also, do you have an opinion on the other half of the patch?
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.
(Assignee)

Comment 7

6 years ago
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.
The only branding variable that I expect not to work with mk_add_options is MOZ_APP_UA_NAME.
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.
The other hunk of the patch might be more useful as a runtime switch, though I don't have a strong opinion there.
(Assignee)

Comment 11

6 years ago
(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.
(Assignee)

Comment 12

6 years ago
(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.
Assignee: nobody → zackw
Status: NEW → ASSIGNED
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"
(Assignee)

Comment 14

6 years ago
You'll forgive me if I think the additional configure option is more elegant.
(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 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.
Attachment #536144 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 17

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb466cf5e01
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/fcb466cf5e01
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 723493

Comment 19

5 years ago
Can't depend, because it's already fixed.
Bug 723493 is about giving this without recompilation.
No longer depends on: 723493
(Assignee)

Comment 20

5 years ago
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.
You need to log in before you can comment on or make changes to this bug.