1309015 - Move MOZ_MAINTENANCE_SERVICE and MOZ_STUB_INSTALLER to Python configure

Status: RESOLVED DUPLICATE of bug 1283830

(Firefox Build System :: General, task)

Description

[Chris Manchester (limited bugmail, email directly)]

browser/confvars.sh is pretty almost entirely standalone assignments with these moved over.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1309015 - Move MOZ_MAINTENANCE_SERVICE to Python configure.

This was force-enabled on Windows, and disallowed anywhere else by
old-configure, so the option was removed and only the forced value
is reflected in Python configure.

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

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1309015 - Move MOZ_STUB_INSTALLER to Python configure.

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

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1309015 - Move MOZ_BUNDLED_FONTS to Python configure.

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

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8799611 [details]
Bug 1309015 - Move MOZ_MAINTENANCE_SERVICE to Python configure.

https://reviewboard.mozilla.org/r/84750/#review84364

It was still possible to disable it in the past. Whether this is something that is necessary or not is not something I can tell. But this is clearly a behavior change. As it is apparently updater code, I guess the following would be questions for rstrong:
- is disabling maintenance service on windows worth supporting?
- is there ever going to be maintenance service on non-windows?
- by extension of the above two, can't we just remove MOZ_MAINTENANCE_SERVICE tests and replaces them with platform tests? (or completely remove them for obviously windows-only stuff)

Comment 5

[Mike Hommey [:glandium]]

See comment 4.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8799612 [details]
Bug 1309015 - Move MOZ_STUB_INSTALLER to Python configure.

https://reviewboard.mozilla.org/r/84752/#review84366

::: browser/moz.configure:24
(Diff revision 1)
> +    if all([target.os == 'WINNT',
> +            not have_64_bit,
> +            not debug,
> +            update_channel in ('nightly', 'aurora', 'beta',
> +                               'beta-dev', 'release', 'release-dev')]):
> +        return True

You could make that:

return (target.os == 'WINNT' and
        not have_64_bit and
        ...) or None

Comment 7

[Robert Strong (they/them - no direct email)]

(In reply to Mike Hommey [:glandium] from comment #4)
> Comment on attachment 8799611 [details]
> Bug 1309015 - Move MOZ_MAINTENANCE_SERVICE to Python configure.
> 
> https://reviewboard.mozilla.org/r/84750/#review84364
> 
> It was still possible to disable it in the past. Whether this is something
> that is necessary or not is not something I can tell. But this is clearly a
> behavior change. As it is apparently updater code, I guess the following
> would be questions for rstrong:
> - is disabling maintenance service on windows worth supporting?
For anyone creating distributions it is. iirc SeaMonkey doesn't use it.

> - is there ever going to be maintenance service on non-windows?
I have no plans to create something similar for other platforms but it isn't possible for me to know what the future holds.

> - by extension of the above two, can't we just remove
> MOZ_MAINTENANCE_SERVICE tests and replaces them with platform tests? (or
> completely remove them for obviously windows-only stuff)
Considering that other apps don't use it I don't think so.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8799613 [details]
Bug 1309015 - Move MOZ_BUNDLED_FONTS to Python configure.

https://reviewboard.mozilla.org/r/84754/#review84368

::: browser/confvars.sh
(Diff revision 1)
>  MOZ_UPDATER=1
>  MOZ_PHOENIX=1
>  
> -if test "$OS_ARCH" = "WINNT" -o \
> -        "$OS_ARCH" = "Linux"; then
> -  MOZ_BUNDLED_FONTS=1

This is also in b2g/**/confvars.sh

::: browser/moz.configure:33
(Diff revision 1)
> +@depends(target)
> +def windows_or_linux(target):
> +    if target.os in ('WINNT', 'GNU'):
> +        return True

Come to think of it (reading back bug 1231701), I think this is the wrong test. Specifically, I think the intent is that all platforms except Android and Darwin should have it enabled by default.

::: browser/moz.configure:47
(Diff revision 1)
> +bundled_fonts = depends_if('--enable-bundled-fonts')(lambda _: True)
> +
> +set_define('MOZ_BUNDLED_FONTS', bundled_fonts)

Soon, it will be set_define('MOZ_BUNDLED_FONTS', True, when='--enable-bundled-fonts') :)

Comment 9

[Mike Hommey [:glandium]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #7)
> (In reply to Mike Hommey [:glandium] from comment #4)
> > Comment on attachment 8799611 [details]
> > Bug 1309015 - Move MOZ_MAINTENANCE_SERVICE to Python configure.
> > 
> > https://reviewboard.mozilla.org/r/84750/#review84364
> > 
> > It was still possible to disable it in the past. Whether this is something
> > that is necessary or not is not something I can tell. But this is clearly a
> > behavior change. As it is apparently updater code, I guess the following
> > would be questions for rstrong:
> > - is disabling maintenance service on windows worth supporting?
> For anyone creating distributions it is. iirc SeaMonkey doesn't use it.

What about Firefox? Is disabling maintenance service on Windows for Firefox worth supporting?

Comment 10

[Robert Strong (they/them - no direct email)]

I don't know of any reason to support disabling it on Firefox except for possibly distributions. For example, I don't know if Tor or any other Windows distributions disables building it.

Comment 11

[Mike Hommey [:glandium]]

Come to think of it, aren't stub installer and maintenance service things that should be opt-in rather than opt-out, and opted-in from the mozconfigs?

Comment 12

[Robert Strong (they/them - no direct email)]

That would be ideal.

Comment 13

[Chris Manchester (limited bugmail, email directly)]

(In reply to Mike Hommey [:glandium] from comment #8)
> Comment on attachment 8799613 [details]
> Bug 1309015 - Move MOZ_BUNDLED_FONTS to Python configure.
> 
> https://reviewboard.mozilla.org/r/84754/#review84368
> 
> ::: browser/confvars.sh
> (Diff revision 1)
> >  MOZ_UPDATER=1
> >  MOZ_PHOENIX=1
> >  
> > -if test "$OS_ARCH" = "WINNT" -o \
> > -        "$OS_ARCH" = "Linux"; then
> > -  MOZ_BUNDLED_FONTS=1
> 
> This is also in b2g/**/confvars.sh
> 
> ::: browser/moz.configure:33
> (Diff revision 1)
> > +@depends(target)
> > +def windows_or_linux(target):
> > +    if target.os in ('WINNT', 'GNU'):
> > +        return True
> 
> Come to think of it (reading back bug 1231701), I think this is the wrong
> test. Specifically, I think the intent is that all platforms except Android
> and Darwin should have it enabled by default.

I'm not finding the part of that bug that suggests that... and I see some code in the packager that has a related test for `#if defined(XP_WIN) || defined(XP_LINUX)`. Can you elaborate?

> 
> ::: browser/moz.configure:47
> (Diff revision 1)
> > +bundled_fonts = depends_if('--enable-bundled-fonts')(lambda _: True)
> > +
> > +set_define('MOZ_BUNDLED_FONTS', bundled_fonts)
> 
> Soon, it will be set_define('MOZ_BUNDLED_FONTS', True,
> when='--enable-bundled-fonts') :)

Comment 14

[Mike Hommey [:glandium]]

There is no reason for *BSD, Solaris, etc. to be treated any different from Linux.

Comment 15

[Chris Manchester (limited bugmail, email directly)]

Bug 1491419 seems to have taken care of MOZ_MAINTENANCE_SERVICE. Not sure if there's a reason MOZ_STUB_INSTALLER is still in old-configure, but I'm probably not going to get back to this.

Comment 16

[Mike Hommey [:glandium]]

MOZ_STUB_INSTALLER is only an AC_SUBST in old-configure.in, set via confvars.sh, so it would be handled by bug 1283830.