Don't depend on download_build_install.py in instrumented library targets.

Don't depend on download_build_install.py in instrumented library targets.

I'm still tweaking the script to accommodate specific libraries. Rebuilding the entire list of libraries with every change is less than fun.

R=glider@chromium.org
BUG=313751, 343515
NOTRY=true

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

[earthdok, 2014-02-12 20:26:21]

PTAL. This is what was causing time-consuming rebuilds of every instrumented
library every time I edited the script. I believe this behavior is wrong. If we
want to retain the ability to force such rebuilds, we could have them depend on
a dummy file.

If we want to keep this dependency out of principle (i.e. depend on what you
use) then we should at least factor out library-specific code into separate
files.

[Alexander Potapenko, 2014-02-13 10:34:47]

Editing download_build_install.py may easily break everything if you
change the compilation flags or the library locations or whatever else
in the script.
A special file that needs to be touched in order to rebuild the
libraries will be probably ignored.
How often is download_build_install.py updated and are you going to
update it at the same pace when MSan is stable?
I suggest to comment this dependency out for now and file a bug to
enable it back once you're done with the libraries.

On Thu, Feb 13, 2014 at 12:26 AM,  <earthdok@chromium.org> wrote:
> Reviewers: Alexander Potapenko,
>
> Message:
> PTAL. This is what was causing time-consuming rebuilds of every instrumented
> library every time I edited the script. I believe this behavior is wrong. If
> we
> want to retain the ability to force such rebuilds, we could have them depend
> on
> a dummy file.
>
> If we want to keep this dependency out of principle (i.e. depend on what you
> use) then we should at least factor out library-specific code into separate
> files.
>
> Description:
> Don't depend on download_build_install.py in instrumented library targets.
>
> R=glider@chromium.org
> BUG=313751
>
> Please review this at https://codereview.chromium.org/161133002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+0, -1 lines):
>   M
> third_party/instrumented_libraries/standard_instrumented_library_target.gypi
>
>
> Index:
> third_party/instrumented_libraries/standard_instrumented_library_target.gypi
> diff --git
>
a/third_party/instrumented_libraries/standard_instrumented_library_target.gypi
>
b/third_party/instrumented_libraries/standard_instrumented_library_target.gypi
> index
>
2bc2de20b4d0c4f5fe056737d79b7e43144af0f7..11ee92dad5f220a3778765e96502094dad11465b
> 100644
> ---
>
a/third_party/instrumented_libraries/standard_instrumented_library_target.gypi
> +++
>
b/third_party/instrumented_libraries/standard_instrumented_library_target.gypi
> @@ -12,7 +12,6 @@
>      {
>        'action_name': '<(_library_name)',
>        'inputs': [
> -        'download_build_install.py',
>        ],
>        'outputs': [
>
>
'<(PRODUCT_DIR)/instrumented_libraries/<(_sanitizer_type)/<(_library_name).txt',
>
>



-- 
Alexander Potapenko
Software Engineer
Google Moscow

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[earthdok, 2014-02-13 11:12:12]

On 2014/02/13 10:34:47, Alexander Potapenko wrote:
> Editing download_build_install.py may easily break everything if you
> change the compilation flags or the library locations or whatever else
> in the script.
> A special file that needs to be touched in order to rebuild the
> libraries will be probably ignored.
> How often is download_build_install.py updated 

4 times in the last 3 weeks. It's especially painful when you're working on the
patch and each incremental change to the script causes a rebuild.

> and are you going to
> update it at the same pace when MSan is stable?

Hopefully not.

[earthdok, 2014-02-13 11:28:01]

updated

[earthdok, 2014-02-13 11:28:01]

updated

[Alexander Potapenko, 2014-02-13 11:48:33]

lgtm

[earthdok, 2014-02-13 11:57:25]

The CQ bit was checked by earthdok@chromium.org

[commit-bot: I haz the power, 2014-02-13 11:57:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/earthdok@chromium.org/161133002/60001

[commit-bot: I haz the power, 2014-02-13 16:19:50]

Change committed as 251050