1024471 - Make run/debug commands use a scratch profile by default

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Josh Matthews [:jdm]]

I think that requiring people to add a profile argument is a gotcha that we can avoid, and we can make the experience for first-time users nicer. My argument is that in general, preserving data between testing sessions is not a necessity, and it's easier to teach people how to use an explicit profile later than it is to explain to new users why they're having trouble running a dev build at the same time as their regular Firefox instance. I'd like to make ./mach run/debug without any profile argument created a scratch profile dir in /tmp and run with that.

Comment 1

[(not currently active) Ted Mielczarek]

Yes, let's do this. As long as we have an optional profile argument so you can pass in an existing profile, creating a fresh profile for each run by default sounds like a great idea.

Comment 2

[Gregory Szorc [:gps]]

As much as I support this, I suspect there will be objection to it. I really wish we had proper config files for mach to make this kind of choice configurable. Config files go a long way towards silencing the mob.

That being said, I will r+ a patch that changes this as long as we can specify the profile via extra arguments.

Comment 3

[Josh Matthews [:jdm]]

I have to say, I would expect the people who care about this behaviour to already be using the -P argument explicitly, rather than having set the default profile for Firefox and depending on that being used when -P is not present.

Comment 4

[Mike Hommey [:glandium]]

Why not create a scratch profile in $objdir, that would be clobbered by a clobber, but kept otherwise?
The main reason _I_ need a scratch profile is not to have it cleaned up every time, but, simply, to have it *not* being my firefox profile, which I'm running firefox on anyways. I guess most people are in this case.

Comment 5

[(no longer active)]

(In reply to comment #4)
> Why not create a scratch profile in $objdir, that would be clobbered by a
> clobber, but kept otherwise?
> The main reason _I_ need a scratch profile is not to have it cleaned up every
> time, but, simply, to have it *not* being my firefox profile, which I'm running
> firefox on anyways. I guess most people are in this case.

Yes please!

Comment 6

[philippe]

Attachment: Default user is now created in objdir/tmp/scratch_user

Comment 7

[philippe]

Made a mistake in my comment. The default user is made in objdir/tmp/scratch_user

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8444052 [details] [diff] [review]
Default user is now created in objdir/tmp/scratch_user

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

Please change the title in the commit message to objdir/tmp/scratch_user to avoid mislead.

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8444052 [details] [diff] [review]
Default user is now created in objdir/tmp/scratch_user

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

I could bikeshed about the name and path of the profile directory. But I think this is fine.

We really need config file support for mach so people can customize the default behavior. Not your problem to solve.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +751,5 @@
> +        if '-profile' not in params and '-P' not in params:
> +            if not os.path.isdir(self.topobjdir + "/tmp/scratch_user"):
> +                os.makedirs(self.topobjdir + "/tmp/scratch_user")
> +            args.append('-profile')
> +            args.append(self.topobjdir + "/tmp/scratch_user")

Does the forward slash work on Windows?

Also, please using os.path.join() or mozpack.path.join() (the latter always uses / as opposed to platform-dependent) for joining filenames. It's how you do path manipulation in python, for better or worse.

@@ +845,5 @@
> +        if '-profile' not in params and '-P' not in params:
> +            if not os.path.isdir(self.topobjdir + "/tmp/scratch_user"):
> +                os.makedirs(self.topobjdir + "/tmp/scratch_user")
> +            args.append('--profile')
> +            args.append(self.topobjdir + "/tmp/scratch_user")

Seeing this redundant block has me wondering how different debug() and run() actually are. It seems like one should be implemented in terms of the other. Not your problem to fix though.

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8444052 [details] [diff] [review]
Default user is now created in objdir/tmp/scratch_user

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

r+ on condition of fixing commit message and using appropriate path joining.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +751,5 @@
> +        if '-profile' not in params and '-P' not in params:
> +            if not os.path.isdir(self.topobjdir + "/tmp/scratch_user"):
> +                os.makedirs(self.topobjdir + "/tmp/scratch_user")
> +            args.append('-profile')
> +            args.append(self.topobjdir + "/tmp/scratch_user")

Does the forward slash work on Windows?

Also, please using os.path.join() or mozpack.path.join() (the latter always uses / as opposed to platform-dependent) for joining filenames. It's how you do path manipulation in python, for better or worse.

@@ +845,5 @@
> +        if '-profile' not in params and '-P' not in params:
> +            if not os.path.isdir(self.topobjdir + "/tmp/scratch_user"):
> +                os.makedirs(self.topobjdir + "/tmp/scratch_user")
> +            args.append('--profile')
> +            args.append(self.topobjdir + "/tmp/scratch_user")

Seeing this redundant block has me wondering how different debug() and run() actually are. It seems like one should be implemented in terms of the other. Not your problem to fix though.

Comment 11

[philippe]

Attachment: modified the path to use os.path.join()

Comment 12

[Josh Matthews [:jdm]]

Let's get this merged! There's no need for a try run since this isn't used by any automated tests.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This doesn't apply to inbound. Please rebase.

Comment 14

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: I think this feature is convenient. Let me help combine the two patches, and rebase it.

Comment 15

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Default user is now created in objdir/tmp/scratch_user.

Comment 16

[Carsten Book [:Tomcat]]

checkin done!
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0885143017d0

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0885143017d0