Update linux static initializer check in sizes.py

Update linux static initializer check in sizes.py

Newer versions of gcc put "frame_dummy" in the .init_array
when it was previously called from _init.  Since sizes.py
uses the size of the .init_array to determine the number
of static initializers we need to take this extra element
into account.

BUG=701478
Review-Url: https://codereview.chromium.org/2765983002
Cr-Commit-Position: refs/heads/master@{#458807}
Committed: https://chromium.googlesource.com/chromium/src/+/55844bc1348ddf628a1559a2ed4532fe6f614478

[Sam Clegg, 2017-03-21 21:01:31]

sbc@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Sam Clegg, 2017-03-21 21:01:32]

I'm not sure how i'm supposed to actually run or test this script locally, or on
the bots.   Any tips?

[Nico, 2017-03-21 21:04:24]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2017-03-21 21:04:24]

lgtm

I think I used to just run this locally with  build_directory mocked out
locally, the script is fairly stand-alone. But testing it live is probably fine
too.

This seems like a nice approach.

https://codereview.chromium.org/2765983002/diff/1/infra/scripts/legacy/script...
File infra/scripts/legacy/scripts/slave/chromium/sizes.py (right):

https://codereview.chromium.org/2765983002/diff/1/infra/scripts/legacy/script...
infra/scripts/legacy/scripts/slave/chromium/sizes.py:305: # but we don't want to
count this entry, since it alwasy present and nothing
typos "it alwasy"

[Sam Clegg, 2017-03-21 21:08:03]

On 2017/03/21 21:04:24, Nico wrote:
> lgtm
> 
> I think I used to just run this locally with  build_directory mocked out
> locally, the script is fairly stand-alone. But testing it live is probably
fine
> too.
> 
> This seems like a nice approach.
> 
>
https://codereview.chromium.org/2765983002/diff/1/infra/scripts/legacy/script...
> File infra/scripts/legacy/scripts/slave/chromium/sizes.py (right):
> 
>
https://codereview.chromium.org/2765983002/diff/1/infra/scripts/legacy/script...
> infra/scripts/legacy/scripts/slave/chromium/sizes.py:305: # but we don't want
to
> count this entry, since it alwasy present and nothing
> typos "it alwasy"

OK, I'l give that a try.

For the record, out of the box, if I try to run it via testing/scripts/sizes.py
I get this:

sbc.sbc (~/dev/chromium/src) $ testing/scripts/sizes.py  run --output=foo.txt
Traceback (most recent call last):
  File "testing/scripts/sizes.py", line 77, in <module>
    sys.exit(common.run_script(sys.argv[1:], funcs))
  File "/usr/local/google/home/sbc/dev/chromium/src/testing/scripts/common.py",
line 63, in run_script
    return args.func(args)
  File "testing/scripts/sizes.py", line 22, in main_run
    '--json', tempfile_path])
  File "/usr/local/google/home/sbc/dev/chromium/src/testing/scripts/common.py",
line 100, in run_runtest
    cmd_args.paths['runit.py'],
KeyError: 'runit.py'

Running it directly gives:

sbc.sbc (~/dev/chromium/src) $
infra/scripts/legacy/scripts/slave/chromium/sizes.py
Traceback (most recent call last):
  File "infra/scripts/legacy/scripts/slave/chromium/sizes.py", line 25, in
<module>
    from slave import build_directory
ImportError: No module named slave

[Nico, 2017-03-21 21:09:11]

On Tue, Mar 21, 2017 at 5:08 PM, <sbc@chromium.org> wrote:

> On 2017/03/21 21:04:24, Nico wrote:
> > lgtm
> >
> > I think I used to just run this locally with build_directory mocked out
> > locally, the script is fairly stand-alone. But testing it live is
> probably
> fine
> > too.
> >
> > This seems like a nice approach.
> >
> >
> https://codereview.chromium.org/2765983002/diff/1/infra/
> scripts/legacy/scripts/slave/chromium/sizes.py
> > File infra/scripts/legacy/scripts/slave/chromium/sizes.py (right):
> >
> >
> https://codereview.chromium.org/2765983002/diff/1/infra/
> scripts/legacy/scripts/slave/chromium/sizes.py#newcode305
> > infra/scripts/legacy/scripts/slave/chromium/sizes.py:305: # but we
> don't want
> to
> > count this entry, since it alwasy present and nothing
> > typos "it alwasy"
>
> OK, I'l give that a try.
>
> For the record, out of the box, if I try to run it via
> testing/scripts/sizes.py
> I get this:
>
> sbc.sbc (~/dev/chromium/src) $ testing/scripts/sizes.py run
> --output=foo.txt
> Traceback (most recent call last):
> File "testing/scripts/sizes.py", line 77, in <module>
> sys.exit(common.run_script(sys.argv[1:], funcs))
> File "/usr/local/google/home/sbc/dev/chromium/src/testing/
> scripts/common.py",
> line 63, in run_script
> return args.func(args)
> File "testing/scripts/sizes.py", line 22, in main_run
> '--json', tempfile_path])
> File "/usr/local/google/home/sbc/dev/chromium/src/testing/
> scripts/common.py",
> line 100, in run_runtest
> cmd_args.paths['runit.py'],
> KeyError: 'runit.py'
>
> Running it directly gives:
>
> sbc.sbc (~/dev/chromium/src) $
> infra/scripts/legacy/scripts/slave/chromium/sizes.py
> Traceback (most recent call last):
> File "infra/scripts/legacy/scripts/slave/chromium/sizes.py", line 25, in
> <module>
> from slave import build_directory
> ImportError: No module named slave
>

Right, comment out that line and change callers to point to your local
build dir.


>
>
> https://codereview.chromium.org/2765983002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sam Clegg, 2017-03-21 21:30:56]

managed to test locally and updated script the make this a little easier.

ptal

[Sam Clegg, 2017-03-21 21:31:02]

The CQ bit was checked by sbc@chromium.org to run a CQ dry run

[Nico, 2017-03-21 21:33:42]

Meh, I liked patch set 1 (+ fix) better, the new version now runs some code at
module import time (i.e. it adds a static initialize to the python script, so to
speak), and it's fairly easy to change for debugging. But up to you.

[Sam Clegg, 2017-03-21 21:53:06]

On 2017/03/21 21:33:42, Nico wrote:
> Meh, I liked patch set 1 (+ fix) better, the new version now runs some code at
> module import time (i.e. it adds a static initialize to the python script, so
to
> speak), and it's fairly easy to change for debugging. But up to you.

Fair enough.  good to keep this change focus anyway.

[Sam Clegg, 2017-03-21 21:53:46]

The CQ bit was checked by sbc@chromium.org

[Sam Clegg, 2017-03-21 21:53:46]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2765983002/#ps40001
(title: "cleanup")

[commit-bot: I haz the power, 2017-03-21 21:54:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 22:03:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 22:03:02]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Paweł Hajdan Jr., 2017-03-22 16:58:12]

LGTM

[Sam Clegg, 2017-03-22 17:15:21]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2017-03-22 17:15:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 18:22:03]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490202921673020,
"parent_rev": "a6d343f911171278376b4509f7da9d90592b03ee", "commit_rev":
"55844bc1348ddf628a1559a2ed4532fe6f614478"}

[commit-bot: I haz the power, 2017-03-22 18:22:50]

Description was changed from

==========
Update linux static initializer check in sizes.py

Newer versions of gcc put "frame_dummy" in the .init_array
when it was previously called from _init.  Since sizes.py
uses the size of the .init_array to determine the number
of static initializers we need to take this extra element
into account.

BUG=701478
==========

to

==========
Update linux static initializer check in sizes.py

Newer versions of gcc put "frame_dummy" in the .init_array
when it was previously called from _init.  Since sizes.py
uses the size of the .init_array to determine the number
of static initializers we need to take this extra element
into account.

BUG=701478

Review-Url: https://codereview.chromium.org/2765983002
Cr-Commit-Position: refs/heads/master@{#458807}
Committed:
https://chromium.googlesource.com/chromium/src/+/55844bc1348ddf628a1559a2ed45...
==========

[commit-bot: I haz the power, 2017-03-22 18:22:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/55844bc1348ddf628a1559a2ed45...

[Lei Zhang, 2017-04-07 21:06:52]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2017-04-07 21:06:53]

Can we drop this after r462181?

[Sam Clegg, 2017-04-07 21:30:42]

On 2017/04/07 21:06:53, Lei Zhang wrote:
> Can we drop this after r462181?

Kinda.. we need to keep the -= 1, but we can make it unconditional perhaps?  I'm
tempted to update the comment and remove the TODO, but keep the conditional just
in case.