Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(130)

Issue 295313002: libvpx: Fix generated asm offsets. (Closed)

Created:
6 years, 7 months ago by fgalligan
Modified:
6 years, 7 months ago
CC:
chromium-reviews, wwcv, jzern, fgalligan1
Visibility:
Public.

Description

libvpx: Fix generated asm offsets. The first issue was that unpack_lib_posix target did not have the correct input set. The next major issue was if the generated asm offset file was changed, the assembly files dependent on the generated file were not being assembled. This change also removes two search paths, where the generated asm offsets were being written. The two paths removed were for older targets, that are not valid anymore. BUG=377062 TBR=tomfinegan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272641

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -34 lines) Patch
M libvpx.gyp View 8 chunks +23 lines, -25 lines 0 comments Download
M unpack_lib_posix.gypi View 4 chunks +12 lines, -9 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
fgalligan
Committed patchset #1 manually as r272641 (presubmit successful).
6 years, 7 months ago (2014-05-24 00:01:03 UTC) #1
fgalligan1
6 years, 7 months ago (2014-05-24 00:01:11 UTC) #2
Tom Finegan
On 2014/05/24 00:01:11, fgalligan1 wrote: lgtm [rubber stamp]
6 years, 7 months ago (2014-05-24 00:21:16 UTC) #3
michaelbai
The WebRTC also use unpack_lib_posix.gypi https://code.google.com/p/chromium/codesearch#search/&q=unpack_lib_posix.gypi%20package:%5Echromium$%20file:(%5Esrc/third_party/webrtc/)&type=cs Did you update them? https://codereview.chromium.org/295313002/diff/1/unpack_lib_posix.gypi File unpack_lib_posix.gypi (left): https://codereview.chromium.org/295313002/diff/1/unpack_lib_posix.gypi#oldcode39 ...
6 years, 7 months ago (2014-05-24 00:29:09 UTC) #4
fgalligan1
6 years, 7 months ago (2014-05-24 00:48:11 UTC) #5
Message was sent while issue was closed.
I didn't know about the other projects using the unpack target. I will update
them as well.

https://codereview.chromium.org/295313002/diff/1/unpack_lib_posix.gypi
File unpack_lib_posix.gypi (left):

https://codereview.chromium.org/295313002/diff/1/unpack_lib_posix.gypi#oldcode39
unpack_lib_posix.gypi:39: 'unpack_lib_posix.sh',
On 2014/05/24 00:29:10, michaelbai wrote:
> I believe the unpack_lib_posix.sh should also be here, otherwise the target
will
> not be regenerated if the  unpack_lib_posix.sh changed.

OK I will add this back.

Powered by Google App Engine
This is Rietveld 408576698