Re: [meta-browser][PATCH 2/4] chromium: gcc-6 support

[Khem Raj <raj.khem@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3259 bytes --]


> On Jul 20, 2016, at 1:13 PM, Trevor Woerner <twoerner@gmail.com> wrote:
> 
> On Wed 2016-07-20 @ 12:59:21 PM, Khem Raj wrote:
>> On Wed, Jul 20, 2016 at 12:51 PM, Trevor Woerner <twoerner@gmail.com> wrote:
>>> Allow chromium to build with gcc-6.
>> 
>> bunch of patches are deleted. Are they upstreamed ? or how do they
>> help compiling chromium with gcc6?
> 
> At some point in the past (I could figure out who, but I don't want to give
> the impression that I'm pointing fingers) someone added the following line to
> the recipe:
> 
> 	-Ddisable_fatal_linker_warnings=1
> 
> The *intention* was to keep the build from failing due to compiler -Werror
> issues. For whatever reason this line has no effect on the build; the -Werror
> is still used, and builds still fail for annoying, non-critical reasons.
> Therefore we've amassed a large set of patches that try to fix these
> warnings by either fixing the code itself:

which is the right thing. We should try to make packages better and send patches upstream as much
as we can. This helps us ensure that the patches get reviewed even if they are rejected. Improves
our metadata quality.

> 
> 
> 	> > ---- a/third_party/ots/src/name.cc
> 	> > -+++ b/third_party/ots/src/name.cc
> 	> > -@@ -79,7 +79,7 @@ bool ots_name_parse(Font *font, const uint8_t* data, size_t length) {
> 	> > -   const char* string_base = reinterpret_cast<const char*>(data) +
> 	> > -       string_offset;
> 	> > -
> 	> > --  NameRecord prev_record;
> 	> > -+  NameRecord prev_record(0, 0, 0, 0);
> 	> > -   bool sort_required = false;
> 	> > -
> 
> or by (more likely) some l337 #pragma tricks:
> 

these are still workarounds but alright in some cases we need them. I would still encourage to run
then by chromium community though.

> 
> 	> > ---- a/third_party/WebKit/Source/wtf/dtoa/bignum.cc
> 	> > -+++ b/third_party/WebKit/Source/wtf/dtoa/bignum.cc
> 	> > -@@ -102,7 +102,10 @@
> 	> > -     }
> 	> > -
> 	> > -
> 	> > -+#pragma GCC diagnostic push
> 	> > -+#pragma GCC diagnostic warning "-Wstrict-overflow"
> 	> > -     void Bignum::AssignDecimalString(Vector<const char> value) {
> 	> > -+#pragma GCC diagnostic pop
> 	> > -         // 2^64 = 18446744073709551616 > 10^19
> 	> > -         const int kMaxUint64DecimalDigits = 19;
> 	> > -         Zero();
> 	> > -@@ -696,7 +696,10 @@
> 	> > -     }
> 
> What I've done here is to patch the various BUILD.gn files and *gyp files so
> that -Werror is actually removed from our build (which was the original
> intent), thereby making a number of subsequent patches superfluous.


I see that and its fine. However we are going to hide issues if we lower the guards.

> 
> Given the chromium developers use a completely different compiler and,
> therefore, are unlikely to be tripping over the same warning/error problems we
> keep seeing, I believe this is a sensible thing to do. Especially since, going
> forward, is reduces the maintanance burden, and (in my opinion) the upstream
> developers are probably unlikely to ever accept these sorts of patches anyway.

If you fix a genuine case that is being caught by gcc and clang is missing to catch and vice
versa it its definitely of interest

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]