Add modified time checking to strip_library_for_apk.py

Add modified time checking to strip_library_for_apk.py

Often this action is triggered and only actually needs to strip few/no
native libraries (i.e if only base.so changes there is no need to strip
all the other libraries). Add modified time checking to this action.

Since the input files are very large, using md5 checking is too slow
(i.e. it is not much, if any, faster than just doing the strip).

BUG=158821
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193003

[newt (away), 2013-04-04 02:18:49]

could this logic be moved to gyp instead of python?  having multiple systems
doing dependency analysis caused so many problems with ant. (granted, we have
better control over this python script)

if each .so needs to be stripped, can we strip them when they're each built?

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:106: # For a symlink, the modified time
should be the greater of the link's
indent 2

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:107: # modified time and the modified time
of the target.
why?

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:116: input_time = max(map(GetModifiedTime,
inputs))
functional programming FTW :)

in practice, if inputs is a long list, this could be sped up by exiting early:

for input in inputs:
  if GetModifiedTime(input) > output_time:
    return True
return False

(not sure if you can dream up a functional way to do this...)

[cjhopman, 2013-04-04 15:56:17]

On 2013/04/04 02:18:49, newt wrote:
> could this logic be moved to gyp instead of python?
Not really. Or rather, not without rather extensive changes to gyp/gyp
generators. Extending gyp to better support some of the things the Android build
does is a longer-term goal.

>  having multiple systems
> doing dependency analysis caused so many problems with ant. (granted, we have
> better control over this python script)

First, there are two relevant concepts that are involved with build actions:
dependency analysis (i.e. between actions)
output staleness (i.e. within one action)

With ant, we had problems with the way it does both of these, but the real
problems were that it did them incorrectly and in ways that were difficult to
understand, control, and fix. Also, gyp has good support for dependency analysis
(but little support for interesting output staleness checking), but its concept
of what an "action" should do did not really reflect what our ant actions were
doing.

We have now moved all (technically, almost all) dependency analysis into gyp.
This is what gyp is for.

Now, output staleness checking. Note, that our tools already do advanced
checking (that is not reflected in gyp). For example, goma checks contents of
inputs (via md5 I believe) vs some stamp. Ninja does some rather advanced
checking to determine if a shared library needs to relink.

Output staleness is a much simpler thing than dependency analysis. With
gyp/ninja the best checking that can be done for these actions is a simple
modified time check on all outputs vs all inputs. That is much, much coarser
than required.

> 
> if each .so needs to be stripped, can we strip them when they're each built?

That would be nice. Gyp doesn't support any way of doing this, though I have
some ideas/proposals that would work that I'll be proposing to g/gyp-developer.

[cjhopman, 2013-04-04 15:59:13]

On 2013/04/04 15:56:17, cjhopman wrote:
> On 2013/04/04 02:18:49, newt wrote:
> > could this logic be moved to gyp instead of python?
> Not really. Or rather, not without rather extensive changes to gyp/gyp
> generators. Extending gyp to better support some of the things the Android
build
> does is a longer-term goal.
> 
> >  having multiple systems
> > doing dependency analysis caused so many problems with ant. (granted, we
have
> > better control over this python script)
> 
> First, there are two relevant concepts that are involved with build actions:
> dependency analysis (i.e. between actions)
> output staleness (i.e. within one action)
> 
> With ant, we had problems with the way it does both of these, but the real
> problems were that it did them incorrectly and in ways that were difficult to
> understand, control, and fix. Also, gyp has good support for dependency
analysis
> (but little support for interesting output staleness checking), but its
concept
> of what an "action" should do did not really reflect what our ant actions were
> doing.
> 
> We have now moved all (technically, almost all) dependency analysis into gyp.

That is, into .gyp files.

> This is what gyp is for.
> 
> Now, output staleness checking. Note, that our tools already do advanced
> checking (that is not reflected in gyp). For example, goma checks contents of
> inputs (via md5 I believe) vs some stamp. Ninja does some rather advanced
> checking to determine if a shared library needs to relink.
> 
> Output staleness is a much simpler thing than dependency analysis. With
> gyp/ninja the best checking that can be done for these actions is a simple
> modified time check on all outputs vs all inputs. That is much, much coarser
> than required.
> 
> > 
> > if each .so needs to be stripped, can we strip them when they're each built?
> 
> That would be nice. Gyp doesn't support any way of doing this, though I have
> some ideas/proposals that would work that I'll be proposing to
g/gyp-developer.

[newt (away), 2013-04-04 22:52:06]

thanks for the thorough analysis.  I agree that it'd be ideal for the stripping
to happen in gyp, but can't be done for the time being.

nits + a philosophical question about symlinks.

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:107: # modified time and the modified time
of the target.
On 2013/04/04 02:18:49, newt wrote:
> why?

this can't hurt, but I can't think of a situation where the build would create a
symlink to an old file that hasn't been rebuilt.

[cjhopman, 2013-04-05 17:42:24]

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:106: # For a symlink, the modified time
should be the greater of the link's
On 2013/04/04 02:18:49, newt wrote:
> indent 2

Done.

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:107: # modified time and the modified time
of the target.
On 2013/04/04 22:52:06, newt wrote:
> On 2013/04/04 02:18:49, newt wrote:
> > why?
> 
> this can't hurt, but I can't think of a situation where the build would create
a
> symlink to an old file that hasn't been rebuilt.

Why need symlink time:
The case that I think could actually happen is that the target of a symlink
changes. I.e. I change a build action from creating a symlink pointing to
AndroidManifest1.xml to one pointing at AndroidManifest2.xml. In this case the
symlink modified time could be much more recent than the target modified time.

Why need target time:
If I depend on AndroidManifest.xml via AndroidManifestLink.xml, then modifying
AndroidManifest.xml should trigger me. This could particularly happen when the
symlink is not created by the build system (because if it were, it should be
touched by whatever build action creates it).

https://codereview.chromium.org/13504003/diff/1/build/android/gyp/util/build_...
build/android/gyp/util/build_utils.py:116: input_time = max(map(GetModifiedTime,
inputs))
On 2013/04/04 02:18:49, newt wrote:
> functional programming FTW :)
> 
> in practice, if inputs is a long list, this could be sped up by exiting early:
> 
> for input in inputs:
>   if GetModifiedTime(input) > output_time:
>     return True
> return False

This works for me. 

> 
> (not sure if you can dream up a functional way to do this...)

This is very close:
return reduce(lambda stale, input: stale or GetModifiedTime(input) >
output_time, inputs, False)

To get true early exit, you pretty much have to deal with continuations and then
it becomes a real mess.

[newt (away), 2013-04-08 20:39:35]

lgtm

[commit-bot: I haz the power, 2013-04-08 20:41:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/13504003/8001

[commit-bot: I haz the power, 2013-04-08 21:07:19]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-09 02:01:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/13504003/8001

[commit-bot: I haz the power, 2013-04-09 04:27:25]

Change committed as 193003