Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(40)

Issue 10694002: Remove unecessary headers following NDK r10c update. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 9 months ago by fdegans
Modified:
6 years, 9 months ago
Reviewers:
rmcilroy, primiano
Base URL:
https://chromium.googlesource.com/external/google-breakpad/src.git@master
Visibility:
Public.

Description

Remove unecessary headers following NDK r10c update.

BUG=chromium:358831

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
D common/android/include/sys/ucontext.h View 1 chunk +0 lines, -255 lines 0 comments Download
D common/android/include/sys/user.h View 1 chunk +0 lines, -229 lines 0 comments Download

Messages

Total messages: 6
fdegans
PTAL This will have to be rolled in with the NDK r10c update in Chromium ...
6 years, 9 months ago #1
fdegans
Also since there is no CQ, I should add someone who is a committer to ...
6 years, 9 months ago #2
primiano
LGTM cause I like cleanups. However I'd like Ross to confirm as he might have ...
6 years, 9 months ago #3
fdegans
This is not only cleanup, NDK r10c introduces breaking changes. Chromium does not build with ...
6 years, 9 months ago #4
rmcilroy
On 2014/10/20 17:29:08, fdegans wrote: > This is not only cleanup, NDK r10c introduces breaking ...
6 years, 9 months ago #5
primiano
6 years, 9 months ago #6
On 2014/10/20 17:53:19, rmcilroy wrote:
> On 2014/10/20 17:29:08, fdegans wrote:
> > This is not only cleanup, NDK r10c introduces breaking changes.
> > Chromium does not build with NDK r10c without this patch. And the other way
> > around, Chromium will not build with this patch applied and without NDK r10c
> so
> > the NDK and breakpad will both need to be rolled in Chromium at the same
time.
> > 
> > I was planning on rolling the NDK this week as it would be nice to have the
> > Lollipop NDK in Chromium quickly and there are some toolchain improvements
we
> > could use.
> 
> Nice to get rid of some of these :). I think this should be fine. Did you
check
> it builds on Arm64 / x86_64?  If so, lgtm.

Fabrice and I had a long offline discussion.
We can't just remove these files, as doing so breaks the compability with NDK <
r10c.
It is my understanding (looking at README.ANDROID in breakpad) that Chrome is
not the only Breakpad+Android client and I think we cannot just make breaking
changes like this.

Fabrice is going to send out a newer CL which is backwards compatible, where he
will conditionally (on  NDK > r10c) mask out / #include_next the system's
user.h, and keep compatibility with older NDKs.

For sake of records, this change was:
- landed in r1394
- reverted in r1395

Please mark this issue as closed.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted