Refactoring unpack_lib_posix and obj_int_extract.

Refactoring unpack_lib_posix and obj_int_extract.

- Created gypi files for common features
- Make them be ready for reusing by webrtc audio_processing module.

BUG=334447
R=johannkoenig@google.com, tomfinegan@chromium.org

Committed: 247345

[Tom Finegan, 2014-01-27 17:36:35]

On 2014/01/27 17:29:40, michaelbai wrote:

obj_int_extract.c change is NOT lgtm: You have to upstream that.

Other stuff looks fine. I'll unblock once you get that upstreamed and roll.

[Tom Finegan, 2014-01-27 17:37:34]

On 2014/01/27 17:36:35, Tom Finegan wrote:
> On 2014/01/27 17:29:40, michaelbai wrote:
> 
> obj_int_extract.c change is NOT lgtm: You have to upstream that.
> 
> Other stuff looks fine. I'll unblock once you get that upstreamed and roll.

and then we can roll... need coffee... agh

[Johann, 2014-01-27 18:22:45]

Please submit the obj_int_extract.c change to
http://www.webmproject.org/code/contribute/

It looks like this is based on the older checkout though because it still uses
the .sh file and requires cygwin.

[michaelbai, 2014-01-27 20:31:53]

On 2014/01/27 18:22:45, Johann wrote:
> Please submit the obj_int_extract.c change to
> http://www.webmproject.org/code/contribute/
> 
> It looks like this is based on the older checkout though because it still uses
> the .sh file and requires cygwin.

Was the change rolled in chromium?

[Johann, 2014-01-27 20:38:55]

On 2014/01/27 20:31:53, michaelbai wrote:
> On 2014/01/27 18:22:45, Johann wrote:
> > It looks like this is based on the older checkout though because it still
uses
> > the .sh file and requires cygwin.
> 
> Was the change rolled in chromium?

I believe you can submit on top of un-merged changes by manually updating
src/DEPS to 247061 for libvpx and syncing. In any case, that should be in soon:

https://codereview.chromium.org/147563002/

[michaelbai, 2014-01-27 21:48:48]

Synced and removed the obj_int_extract.c, PTAL

[Tom Finegan, 2014-01-27 21:56:47]

lgtm % nits

https://codereview.chromium.org/130563008/diff/70001/obj_int_extract.gypi
File obj_int_extract.gypi (right):

https://codereview.chromium.org/130563008/diff/70001/obj_int_extract.gypi#new...
obj_int_extract.gypi:8: # The following gyp variables must be set before include
this gypi.
s/include/including/
s/gypi./gypi:/

Ident 2 for each variable name. 

i.e.: s/# output_format/#   output_format/

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi
File unpack_lib_posix.gypi (right):

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:5: # This action takes archived .a file and unpack it unto
.o files.
s/archived .a/an archive (.a)/
s/unpack/unpacks/
s/.o/object (.o)/

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:6: # input gyp variables are:
Optional or required?

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:7: # unpack_lib_output_dir, the output directory of
extrated object file
Ditto on variable name indent comment from last file.

[michaelbai, 2014-01-27 22:46:34]

https://codereview.chromium.org/130563008/diff/70001/obj_int_extract.gypi
File obj_int_extract.gypi (right):

https://codereview.chromium.org/130563008/diff/70001/obj_int_extract.gypi#new...
obj_int_extract.gypi:8: # The following gyp variables must be set before include
this gypi.
On 2014/01/27 21:56:48, Tom Finegan wrote:
> s/include/including/
> s/gypi./gypi:/
> 
> Ident 2 for each variable name. 
> 
> i.e.: s/# output_format/#   output_format/

Done.

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi
File unpack_lib_posix.gypi (right):

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:5: # This action takes archived .a file and unpack it unto
.o files.
On 2014/01/27 21:56:48, Tom Finegan wrote:
> s/archived .a/an archive (.a)/
> s/unpack/unpacks/
> s/.o/object (.o)/

Done.

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:6: # input gyp variables are:
On 2014/01/27 21:56:48, Tom Finegan wrote:
> Optional or required?

Done.

https://codereview.chromium.org/130563008/diff/70001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:7: # unpack_lib_output_dir, the output directory of
extrated object file
On 2014/01/27 21:56:48, Tom Finegan wrote:
> Ditto on variable name indent comment from last file.

Done.

[Tom Finegan, 2014-01-27 22:57:17]

still lgtm % nits

A couple new ones that I didn't spot the first time: Sorry.

https://codereview.chromium.org/130563008/diff/90001/obj_int_extract.gypi
File obj_int_extract.gypi (right):

https://codereview.chromium.org/130563008/diff/90001/obj_int_extract.gypi#new...
obj_int_extract.gypi:8: # The following gyp variables must be set before
includes this gypi:
s/includes/including/

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi
File unpack_lib_posix.gypi (right):

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:6: # The required input gyp variables are:
s/required input gyp variable are/following gyp variables are required/

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:7: #   unpack_lib_output_dir, the output directory of
extrated object file
s/extrated/extracted/

[michaelbai, 2014-01-27 23:09:58]

https://codereview.chromium.org/130563008/diff/90001/obj_int_extract.gypi
File obj_int_extract.gypi (right):

https://codereview.chromium.org/130563008/diff/90001/obj_int_extract.gypi#new...
obj_int_extract.gypi:8: # The following gyp variables must be set before
includes this gypi:
On 2014/01/27 22:57:18, Tom Finegan wrote:
> s/includes/including/

It seemed I missed this one in previous patch

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi
File unpack_lib_posix.gypi (right):

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:6: # The required input gyp variables are:
On 2014/01/27 22:57:18, Tom Finegan wrote:
> s/required input gyp variable are/following gyp variables are required/

Done.

https://codereview.chromium.org/130563008/diff/90001/unpack_lib_posix.gypi#ne...
unpack_lib_posix.gypi:7: #   unpack_lib_output_dir, the output directory of
extrated object file
On 2014/01/27 22:57:18, Tom Finegan wrote:
> s/extrated/extracted/

Done.

[Johann, 2014-01-27 23:19:58]

LGTM, but with a request. Is it possible to force regeneration of these files
when: the sources files they depend on change or when the build configuration
(release/debug/32/64bit) changes?

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp
File libvpx.gyp (right):

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp#newcode522
libvpx.gyp:522: '<(INTERMEDIATE_DIR)/vpx_scale_asm_offsets.o',
Is there a way to mark this file as dirty whenever the source or configuration
change? It's unrelated to your change, but we've had issues with stale files
causing issues.

[michaelbai, 2014-01-27 23:55:21]

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp
File libvpx.gyp (right):

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp#newcode522
libvpx.gyp:522: '<(INTERMEDIATE_DIR)/vpx_scale_asm_offsets.o',
Which files or configuration changed?


On 2014/01/27 23:19:58, Johann wrote:
> Is there a way to mark this file as dirty whenever the source or configuration
> change? It's unrelated to your change, but we've had issues with stale files
> causing issues.

[Johann, 2014-01-28 00:03:56]

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp
File libvpx.gyp (right):

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp#newcode522
libvpx.gyp:522: '<(INTERMEDIATE_DIR)/vpx_scale_asm_offsets.o',
On 2014/01/27 23:55:21, michaelbai wrote:
> Which files or configuration changed?

Any of the sources for libvpx_asm_offsets_*:
<(libvpx_source)/vp8/encoder/vp8_asm_enc_offsets.c
<(libvpx_source)/vpx_scale/vpx_scale_asm_offsets.c

or any configuration change where structure sizes could change - In one case,
the Debug directory will retain artifacts when switching between 32 and 64 bit
builds: b/12230993 This wasn't an issue for the Release directories because they
were different for 32 and 64 bit.

[michaelbai, 2014-01-28 00:13:48]

Committed patchset #4 manually as r247345 (presubmit successful).

[michaelbai, 2014-01-28 00:15:39]

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp
File libvpx.gyp (right):

https://codereview.chromium.org/130563008/diff/110001/libvpx.gyp#newcode522
libvpx.gyp:522: '<(INTERMEDIATE_DIR)/vpx_scale_asm_offsets.o',
You may put a list of potential .a files as 'input' of action.


On 2014/01/28 00:03:57, Johann wrote:
> On 2014/01/27 23:55:21, michaelbai wrote:
> > Which files or configuration changed?
> 
> Any of the sources for libvpx_asm_offsets_*:
> <(libvpx_source)/vp8/encoder/vp8_asm_enc_offsets.c
> <(libvpx_source)/vpx_scale/vpx_scale_asm_offsets.c
> 
> or any configuration change where structure sizes could change - In one case,
> the Debug directory will retain artifacts when switching between 32 and 64 bit
> builds: b/12230993 This wasn't an issue for the Release directories because
they
> were different for 32 and 64 bit.