Issue 4677 - Add support for x86 builds in adblockbrowser-build

Issue 4677 - Add support for x86 builds in adblockbrowser-build

[anton, 2016-12-01 07:03:58]

https://codereview.adblockplus.org/29365816/diff/29365817/build.py
File build.py (right):

https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode60
build.py:60: build_environment["MOZCONFIG"] = generate_mozconfig(
it might be more reliable not just remove `find_mozconfig` and `check_mozconfig`
and assume it's valid, but genereate_mozconfig and use `check_mozconfig` for
just generated mozconfig. It would be easier to merge build.py from upstream and
be sure generated config is still valid. Just an idea though.

https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode130
build.py:130: sys.exit(4)
From integration point of view (f.e. CI server - let's say someone has
configured CI server and it starts working incorrect one day) it's better not to
change existing return codes but just add new ones instead. Not a significant
remark, but anyway. It means that we should use `6` here and still use `4` and
`5` below.

[diegocarloslima, 2016-12-01 10:35:59]

https://codereview.adblockplus.org/29365816/diff/29365817/build.py
File build.py (right):

https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode60
build.py:60: build_environment["MOZCONFIG"] = generate_mozconfig(
On 2016/12/01 07:03:58, anton wrote:
> it might be more reliable not just remove `find_mozconfig` and
`check_mozconfig`
> and assume it's valid, but genereate_mozconfig and use `check_mozconfig` for
> just generated mozconfig. It would be easier to merge build.py from upstream
and
> be sure generated config is still valid. Just an idea though.

I understand your point of view, but my reasons to remove mozconfig validation
is to avoid the addition of a lot of boilerplate code as we increase our build
variants. Instead, I added comments to each mozconfig stating that they
shouldn't be changed. For adding custom configuration, I have provided the
mozconfig-custom file. Actually, even with the checks, the user can easily
bypass it just by commenting out the check function calls. So none of the
approaches can prevent the user from screwing up the build process if he wants
to do so.

https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode130
build.py:130: sys.exit(4)
On 2016/12/01 07:03:58, anton wrote:
> From integration point of view (f.e. CI server - let's say someone has
> configured CI server and it starts working incorrect one day) it's better not
to
> change existing return codes but just add new ones instead. Not a significant
> remark, but anyway. It means that we should use `6` here and still use `4` and
> `5` below.

Acknowledged.

[anton, 2016-12-12 15:24:17]

On 2016/12/01 10:35:59, diegocarloslima wrote:
> https://codereview.adblockplus.org/29365816/diff/29365817/build.py
> File build.py (right):
> 
> https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode60
> build.py:60: build_environment["MOZCONFIG"] = generate_mozconfig(
> On 2016/12/01 07:03:58, anton wrote:
> > it might be more reliable not just remove `find_mozconfig` and
> `check_mozconfig`
> > and assume it's valid, but genereate_mozconfig and use `check_mozconfig` for
> > just generated mozconfig. It would be easier to merge build.py from upstream
> and
> > be sure generated config is still valid. Just an idea though.
> 
> I understand your point of view, but my reasons to remove mozconfig validation
> is to avoid the addition of a lot of boilerplate code as we increase our build
> variants. Instead, I added comments to each mozconfig stating that they
> shouldn't be changed. For adding custom configuration, I have provided the
> mozconfig-custom file. Actually, even with the checks, the user can easily
> bypass it just by commenting out the check function calls. So none of the
> approaches can prevent the user from screwing up the build process if he wants
> to do so.
> 
> https://codereview.adblockplus.org/29365816/diff/29365817/build.py#newcode130
> build.py:130: sys.exit(4)
> On 2016/12/01 07:03:58, anton wrote:
> > From integration point of view (f.e. CI server - let's say someone has
> > configured CI server and it starts working incorrect one day) it's better
not
> to
> > change existing return codes but just add new ones instead. Not a
significant
> > remark, but anyway. It means that we should use `6` here and still use `4`
and
> > `5` below.
> 
> Acknowledged.

LGTM

[Felix Dahlke, 2016-12-20 15:34:38]

Pretty much LGTM, but isn't `MOZILLA_OFFICIAL=1` missing?

[diegocarloslima, 2016-12-21 13:39:35]

On 2016/12/20 15:34:38, Felix Dahlke wrote:
> Pretty much LGTM, but isn't `MOZILLA_OFFICIAL=1` missing?

Yes, it is. I'll fix that