I was asked to help with the review and on a first glimpse agree with
Robie c#62.
TL;DR:
1. munching various changes together makes it hard to review
2. several changes are not clear why they were added to the debdiff
2. We are in Final Freeze already, so SRU Rules apply -> lets fix the FTBFS and
drop all not needed changes which make the change easier to grasp, better to
review and safer to apply.
If you could adapt a git based workflow for complex uploads that would
help review and changes as well.
I'll rework the patch in a way I assume it might work.
Then I ask Jorge to review if this is still ok with him.
I can recreate the build errors on the last upload.
It matches those in [1].
I checked Robies comments above and referencing his changes I did:
1. really not needed atm - dropping
2. really not needed atm - dropping
3. the effect of this depends on defines like IB_STRONG_MEMORY_MODEL
For Intel this is still a no-op (as the other defines), just now a defined
instead of an undefined no-op define.
=> keep this change
4. Yes this is like "compiler please check nothing" which I think is too much
The code actually already takes a very detailed list of ignores to run with
-Werror despite
some warnings. If you look at [1] you see e.g. -Wno-error=nonnull-compare and
-Wno-error=unused-result in combination with -Werror. That way it is safe,
but just allows
a few.
Dropping -Werror in general and even setting fpermissive [2] is much more
than needed
This is new in gcc7 [3] and that is what we look for.
=> use -Wno-error=implicit-fallthrough which will keep the warning, but make
it non fatal
=> tests showed a more cases that are needed like format-overflow, ...
(added as needed)
=> See below for details on permissive
5. nice cleanup but not needed
=> drop
6. This might be to make the defines available earlier?
I tested to build without and it was fine on x86.
7. I'll have to rewrite the changelog anyway, so that will be cleaned
This exercise (other than review) of rewriting the fix was mostly disabling the
new errors, but one was special. It now better detects ISO standard violations.
That causes:
"error: ISO C++ forbids comparison between pointer and integer
[-fpermissive]"#
Now one might think, lets add that because the doc says:
-fpermissive
Downgrade some diagnostics about nonconformant code from errors
to warnings. Thus, using -fpermissive allows some nonconforming
code to compile.
But that makes something that is an error a warning, which still is an error
with -Werror.
You can't further downgrade this with -Wno-... [4]
Instead these errors need to be fixed in code - otherwise we would have to drop
-Werrror which I would not like.
Test Program:
#include <iostream>
using namespace std;
int main()
{
const char *str = "Foo";
if (str != '\0')
{
cout << "Hello, World!";
}
return 0;
}
I fixed it with a in-source fix "fix-gcc7-compiler-errors.patch".
I'd ask you Jorge, to please discuss to upstream this.
After quite some iterations I was successful with that on a local build.
Switching to cross arch build on Launchpad to be entirely sure - this is still
building, so further respins might be needed if something shows up there.
@Jorge - If a test would fast and the build succeeds, please consider
testing the ppa at [5].
I upload this for review (Jorge if you want, Rbasak for sure but it should be
easier to look at now) and sponsoring (Rbasak).
Please look at the changes suggested in [6], I wasn't sure if you want a normal
MP after so much review was on the debdiff. But if you want there would be one
in [7] - that would give us commentary functions of LP.
[1]:
https://launchpadlibrarian.net/336371437/buildlog_ubuntu-artful-amd64.percona-xtradb-cluster-5.6_5.6.34-26.19-0ubuntu3_BUILDING.txt.gz
[2]:
https://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/C_002b_002b-Dialect-Options.html#index-fpermissive-140
[3]:
https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/
[4]:
https://stackoverflow.com/questions/19214645/how-to-suppress-warnings-for-void-to-foo-conversions-reduced-from-errors
[5]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/2994
[6]:
https://code.launchpad.net/~paelzer/ubuntu/+source/percona-xtradb-cluster-5.6/+git/percona-xtradb-cluster-5.6/+ref/bug-1657256-FTBFS-cleanup
[7]:
https://code.launchpad.net/~paelzer/ubuntu/+source/percona-xtradb-cluster-5.6/+git/percona-xtradb-cluster-5.6/+merge/332357
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1657256
Title:
Percona crashes when doing a a 'larger' update
To manage notifications about this bug go to:
https://bugs.launchpad.net/charm-test-infra/+bug/1657256/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
