I get frequently frustrated when I use mach run and then get things like the interstitial warning in about:config. It would be nice if I could have a personal setup somewhere on my machine that mach clones when creating the temporary profile. Is this something that would be accepted if I wrote a patch?

Yes. The preferred way to do this is to create mach config files. But that is scope bloat. We settle for environment variables until a hero implements mach config files.

Looks like |mach run| should just use mozprofile. Mozprofile can: 1) Create a temporary profile 2) Clone existing profile 3) Use + modify an existing profile or 4) Use + restore an existing profile In all cases you can use it to set prefs and optionally remove them again at the end. p.s mach configs have existed for awhile now, we could even allow setting prefs in there.

(In reply to Andrew Halberstadt [:ahal] from comment #2) > p.s mach configs have existed for awhile now, we could even allow setting > prefs in there. What are mach configs?

(In reply to Dave Townsend [:mossop] from comment #3) > What are mach configs? You can define them in ~/.mozbuild/machrc, or else point $MACHRC to it. See ./mach settings for a list of accepted settings (currently only alias).

The attached patch allows both cloning profiles (with --profile) and setting prefs, either via --setpref or machrc: [runprefs] layout.css.servo.enabled=true Prefs will be ignored if -P or -profile (with one dash) is used though. Passing in --profile will cause the profile to be cloned and then deleted at the end of the run, whereas not passing --profile will create a persistent profile under $objdir/tmp/scratch-user that will be re-used on subsequent runs. This patch isn't finished, it probably needs some tweaking on exactly what happens when which profile related parameters are used.. the possibilities are quite complicated. If anyone wants to grab this and run with it, be my guest!

(In reply to Andrew Halberstadt [:ahal] from comment #6) > The attached patch allows both cloning profiles (with --profile) and setting > prefs, either via --setpref or machrc: I tried this and --set-pref works as expected, thank you for putting this up! One issue with profile cloning is that `--profile` currently gets passed along to Firefox which creates a new profile at the directory (or reuses an existing one). So without the patch applied `./mach run --profile /tmp/foo` works, but with it applied `./mach run --profile /tmp/foo` fails with `OSError: [Errno 2] No such file or directory: '/tmp/foo'`

(In reply to Andrew Halberstadt [:ahal] from comment #6) > The attached patch allows both cloning profiles (with --profile) and setting > prefs, either via --setpref or machrc: > > [runprefs] > layout.css.servo.enabled=true > > Prefs will be ignored if -P or -profile (with one dash) is used though. > Passing in --profile will cause the profile to be cloned and then deleted at > the end of the run, whereas not passing --profile will create a persistent > profile under $objdir/tmp/scratch-user that will be re-used on subsequent > runs. > > This patch isn't finished, it probably needs some tweaking on exactly what > happens when which profile related parameters are used.. the possibilities > are quite complicated. If anyone wants to grab this and run with it, be my > guest! IMO we could pull profile cloning out into another CLI argument / bug - it seems useful but adds to number of possibilities. Another thing that may simplify things quite a bit is if we only wrote preferences when being called without any profile argument (so only when using the scratch_user).

Comment on attachment 8891352 [details] Bug 1172574 - Allow for setting prefs in mach run for scratch_user; https://reviewboard.mozilla.org/r/162548/#review167808 ::: python/mozbuild/mozbuild/mach_commands.py:1334 (Diff revision 1) > + prefs = { > + 'browser.shell.checkDefaultBrowser': False, > + 'general.warnOnAboutConfig': False, > + } This is probably scope bloat, but what if on first run (e.g when ~/.mozbuild gets created), we had mach_bootstrap.py create a machrc with these prefs pre-populated under [runprefs]? Then people could remove them if they don't like them. The downside is that no one knows about machrc, so they would be essentially hidden. Though |mach run| could print all the prefs it's setting or something. Would be good follow-up fodder. ::: python/mozbuild/mozbuild/mach_commands.py:1342 (Diff revision 1) > - if not os.path.isdir(path): > + profile = Profile(path, preferences=prefs, restore=False) > - os.makedirs(path) > args.append('-profile') > - args.append(path) > + args.append(profile.profile) Thanks, yeah this is a much simpler first step (though it doesn't address the title of this bug, which is more about cloning than prefs). In the future we could properly wrap -profile/--profile and either clone or restore the passed in profile.

(In reply to Andrew Halberstadt [:ahal] from comment #10) > Comment on attachment 8891352 [details] > Bug 1172574 - Add ability to clone profiles and set prefs to |mach run| > > https://reviewboard.mozilla.org/r/162548/#review167808 > > ::: python/mozbuild/mozbuild/mach_commands.py:1334 > (Diff revision 1) > > + prefs = { > > + 'browser.shell.checkDefaultBrowser': False, > > + 'general.warnOnAboutConfig': False, > > + } > > This is probably scope bloat, but what if on first run (e.g when ~/.mozbuild > gets created), we had mach_bootstrap.py create a machrc with these prefs > pre-populated under [runprefs]? Then people could remove them if they don't > like them. > > The downside is that no one knows about machrc, so they would be essentially > hidden. Though |mach run| could print all the prefs it's setting or > something. > > Would be good follow-up fodder. I was wondering about that too. For now someone could at least override these prefs since the runprefs are loaded after the hardcoded defaults (although I just tested and it seems to lowercase the prefname after loading from machrc so that will need to be fixed) [runprefs] general.warnOnAboutConfig=true > ::: python/mozbuild/mozbuild/mach_commands.py:1342 > (Diff revision 1) > > - if not os.path.isdir(path): > > + profile = Profile(path, preferences=prefs, restore=False) > > - os.makedirs(path) > > args.append('-profile') > > - args.append(path) > > + args.append(profile.profile) > > Thanks, yeah this is a much simpler first step (though it doesn't address > the title of this bug, which is more about cloning than prefs). > > In the future we could properly wrap -profile/--profile and either clone or > restore the passed in profile. Mossop, would something like this patch solve your original request? It doesn't do any cloning, but instead allows for setting arbitrary prefs on the scratch_user (either via CLI argument or machrc file).

(In reply to Brian Grinstead [:bgrins] from comment #11) > I was wondering about that too. For now someone could at least override > these prefs since the runprefs are loaded after the hardcoded defaults > (although I just tested and it seems to lowercase the prefname after loading > from machrc so that will need to be fixed) I think if you add `self._config.optionxform = str` to this location, it'll work: https://dxr.mozilla.org/mozilla-central/source/python/mach/mach/config.py#303

(In reply to Andrew Halberstadt [:ahal] from comment #12) > (In reply to Brian Grinstead [:bgrins] from comment #11) > > I was wondering about that too. For now someone could at least override > > these prefs since the runprefs are loaded after the hardcoded defaults > > (although I just tested and it seems to lowercase the prefname after loading > > from machrc so that will need to be fixed) > > I think if you add `self._config.optionxform = str` to this location, it'll > work: > https://dxr.mozilla.org/mozilla-central/source/python/mach/mach/config.py#303 Yes that fixed it, thanks!

Comment on attachment 8891352 [details] Bug 1172574 - Allow for setting prefs in mach run for scratch_user; https://reviewboard.mozilla.org/r/162548/#review167864 I wholeheartedly approve of the feature. The implementation needs a little work. ::: python/mozbuild/mozbuild/locale/en_US/LC_MESSAGES/runprefs.po:5 (Diff revision 2) > +msgid "runprefs.*.short" > +msgstr "Pass a pref into Firefox when using `mach run`" > + > +msgid "runprefs.*.full" It isn't clear to me how these vary. There is no code to treat them differently. Is mozprofile doing magic? ::: python/mozbuild/mozbuild/mach_commands.py:1269 (Diff revision 2) > help='Do not pass the --profile argument by default.') > @CommandArgument('--disable-e10s', action='store_true', group=prog_group, > help='Run the program with electrolysis disabled.') > @CommandArgument('--enable-crash-reporter', action='store_true', group=prog_group, > help='Run the program with the crash reporter enabled.') > + @CommandArgument('--setpref', action='append', default=[], group=prog_group, Why not just `--pref`? ::: python/mozbuild/mozbuild/mach_commands.py:1293 (Diff revision 2) > def run(self, params, remote, background, noprofile, disable_e10s, > - enable_crash_reporter, debug, debugger, debugger_args, > - dmd, mode, stacks, show_dump_stats): > + enable_crash_reporter, setpref, debug, debugger, > + debugger_args, dmd, mode, stacks, show_dump_stats): > While you're here, could you please add a docstring that calls out how you can define prefs in a mach config file? That will appear in `mach help run`. ::: python/mozbuild/mozbuild/mach_commands.py:1335 (Diff revision 2) > + prefs.update([p.split('=', 1) for p in setpref]) > + prefs.update(self._mach_context.settings.runprefs) This is the wrong application order. CLI arguments should take precedence over global config files.

(In reply to Gregory Szorc [:gps] from comment #16) > ::: python/mozbuild/mozbuild/mach_commands.py:1269 > (Diff revision 2) > > help='Do not pass the --profile argument by default.') > > @CommandArgument('--disable-e10s', action='store_true', group=prog_group, > > help='Run the program with electrolysis disabled.') > > @CommandArgument('--enable-crash-reporter', action='store_true', group=prog_group, > > help='Run the program with the crash reporter enabled.') > > + @CommandArgument('--setpref', action='append', default=[], group=prog_group, > > Why not just `--pref`? This matches the CLI argument name for mochitests: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/mochitest_options.py#402.

Comment on attachment 8891409 [details] Bug 1172574 - Prevent the default browser check and the about:config warning by default for the scratch_user; https://reviewboard.mozilla.org/r/162574/#review167896

(In reply to Brian Grinstead [:bgrins] from comment #11) > Mossop, would something like this patch solve your original request? It > doesn't do any cloning, but instead allows for setting arbitrary prefs on > the scratch_user (either via CLI argument or machrc file). Certainly solves the cases I was thinking of.

(In reply to Gregory Szorc [:gps] from comment #16) > Comment on attachment 8891352 [details] > Bug 1172574 - Allow for setting prefs in mach run for scratch_user; > > https://reviewboard.mozilla.org/r/162548/#review167864 > > I wholeheartedly approve of the feature. The implementation needs a little > work. > > ::: python/mozbuild/mozbuild/locale/en_US/LC_MESSAGES/runprefs.po:5 > (Diff revision 2) > > +msgid "runprefs.*.short" > > +msgstr "Pass a pref into Firefox when using `mach run`" > > + > > +msgid "runprefs.*.full" > > It isn't clear to me how these vary. There is no code to treat them > differently. Is mozprofile doing magic? I'm not sure. If I do `./mach settings` I see the `alias.*.full` message here: https://dxr.mozilla.org/mozilla-central/source/python/mach/mach/locale/en_US/LC_MESSAGES/alias.po#5-9. Andrew, do you know if/when the *.short messages are used?

The i18n of the settings is over-engineered. If it is easier to delete that code, just do it. Although I still insist on a requirement that every setting be documented and discoverable. It is horrible UX to have an invisible feature.

Another oddity with the localization is that the files have to be in a new python/mozbuild/mozbuild/locale/en_US folder even though there's already a python/mozbuild/mozbuild/locale/en-US (with a dash). When I tried to move it into the existing folder the docs don't show up with `./mach settings`

Comment on attachment 8891352 [details] Bug 1172574 - Allow for setting prefs in mach run for scratch_user; https://reviewboard.mozilla.org/r/162548/#review167912 ::: python/mozbuild/mozbuild/mach_commands.py:1293 (Diff revision 2) > def run(self, params, remote, background, noprofile, disable_e10s, > - enable_crash_reporter, debug, debugger, debugger_args, > - dmd, mode, stacks, show_dump_stats): > + enable_crash_reporter, setpref, debug, debugger, > + debugger_args, dmd, mode, stacks, show_dump_stats): > I assume you mean to change the 'help' text in the @CommandArgument('--setpref') above? I'll update that to include instructions for machrc unless if I hear otherwise.

Yeah, I'd whole heartedly support removing the i18n stuff.. I don't remember what the difference between short and full is. I think that's just something the gettext module expects. I think instead of doing that we could just add it as a required parameter on the SettingsProvider: @SettingsProvider class RunSettings(object): config_settings = [('runprefs.*', 'string', 'Prefs to be used with `mach run`')] This is probably best left to a follow-up though.

Alright I've updated the review to address the feedback in Comment 16, with the exception of the CLI arg name (see Comment 17) and the i18n stuff (see Comment 24).

Comment on attachment 8891352 [details] Bug 1172574 - Allow for setting prefs in mach run for scratch_user; https://reviewboard.mozilla.org/r/162548/#review167960 ::: python/mozbuild/mozbuild/mach_commands.py:1337 (Diff revision 3) > + prefs = { } > + prefs.update(self._mach_context.settings.runprefs) > + prefs.update([p.split('=', 1) for p in setpref]) > + > path = os.path.join(self.topobjdir, 'tmp', 'scratch_user') > - if not os.path.isdir(path): > + profile = Profile(path, preferences=prefs, restore=False) Actually, you should remove the 'restore' parameter (my mistake). Setting this to False means the prefs will keep being appended to `scratch_user/user.js` on each `mach run` invocation. Mozprofile is smart enough to only reset prefs that it created (so if you manually add prefs to user.js, those will be preserved).

Comment on attachment 8891352 [details] Bug 1172574 - Allow for setting prefs in mach run for scratch_user; https://reviewboard.mozilla.org/r/162548/#review168018

Just pushed a fix for boolean preferences (needed to call Preferences.cast as in https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1809-1810)

Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5a8f9c5240a2 Allow for setting prefs in mach run for scratch_user;r=gps https://hg.mozilla.org/integration/autoland/rev/15dc693f7bb1 Prevent the default browser check and the about:config warning by default for the scratch_user;r=mossop

https://hg.mozilla.org/mozilla-central/rev/5a8f9c5240a2 https://hg.mozilla.org/mozilla-central/rev/15dc693f7bb1

This section should be updated: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/mach#Does_mach_have_its_own_configuration_file (it doesn't mention the machrc file at all at the moment)

(In reply to Brian Grinstead [:bgrins] from comment #37) > This section should be updated: > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/ > mach#Does_mach_have_its_own_configuration_file (it doesn't mention the > machrc file at all at the moment) Alright, I've added a bit of information about the ~/.mozbuild/machrc file and a pointer to `./mach settings`. I didn't detail the individual settings since they seem more likely to be kept up to date in the `./mach settings` command than on that page.