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

Issue 6764002: Update breakpad to support Android NDK r10c (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 5 months ago by fdegans
Modified:
9 years, 5 months ago
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk
Visibility:
Public.

Description

Update breakpad to support Android NDK r10c

Prior to NDK r10c, Breakpad was privately backporting these system headers.
This change is now unforking and removing those headers and using the ones from
the NDK.
Rationale:
 - They are finally available in the NDK, so there is no need to keep
maintaining a fork (which was necessary up until recently to support arm64).
 - These forked headers, as they are today, are not compatible with the latest
NDK (i.e. this change is required to roll NDK r10c). The forked
ucontext.h, being removed by this CL, depends on some transitional features
which are not compatible with the NDK release being targeted here.

After this change, the NDK r10c is now required to build Breakpad on Android.
Note that NDK releases are backwards compatible and contain all the previous
API
levels, so this change is NOT effectively enforcing to build against any
particular Android SDK.

BUG=chromium:358831

Patch Set 1 #

Total comments: 2

Patch Set 2 : Remove file from gyp #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M README.ANDROID View 2 chunks +3 lines, -2 lines 0 comments Download
D src/common/android/include/sys/ucontext.h View 1 chunk +0 lines, -255 lines 0 comments Download
M src/common/android/include/sys/user.h View 1 chunk +6 lines, -189 lines 0 comments Download
M src/common/common.gyp View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 7
fdegans
9 years, 5 months ago #1
primiano
LGTM. Can you just please reword a bit the commit message and put some of ...
9 years, 5 months ago #2
Mark Mentovai
https://breakpad.appspot.com/6764002/diff/1/src/common/android/include/sys/ucontext.h File src/common/android/include/sys/ucontext.h (left): https://breakpad.appspot.com/6764002/diff/1/src/common/android/include/sys/ucontext.h#oldcode1 src/common/android/include/sys/ucontext.h:1: // Copyright (c) 2012, Google Inc. Remove this file ...
9 years, 5 months ago #3
fdegans
Thanks for the quick review, that should do it. https://breakpad.appspot.com/6764002/diff/1/src/common/android/include/sys/ucontext.h File src/common/android/include/sys/ucontext.h (left): https://breakpad.appspot.com/6764002/diff/1/src/common/android/include/sys/ucontext.h#oldcode1 src/common/android/include/sys/ucontext.h:1: ...
9 years, 5 months ago #4
Mark Mentovai
LGTM
9 years, 5 months ago #5
rmcilroy
On 2014/10/21 13:22:53, Mark Mentovai wrote: > LGTM lgtm.
9 years, 5 months ago #6
primiano
9 years, 5 months ago #7
On 2014/10/21 13:25:00, rmcilroy wrote:
> On 2014/10/21 13:22:53, Mark Mentovai wrote:
> > LGTM
> 
> lgtm.

Landed Patch Set 2 on fdegans's behalf as r1396 
https://code.google.com/p/google-breakpad/source/detail?r=1396

Please mark this issue as closed (and have fun rolling this :D)
Sign in to reply to this message.

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