Only include unique lines when patching orderfile.

Only include unique lines when patching orderfile.

Also factor out prefix stripping.

Committed: https://crrev.com/81322057a224333cb87afaed0aca693af0a690f4
Cr-Commit-Position: refs/heads/master@{#312644}

[azarchs, 2015-01-22 16:39:16]

azarchs@chromium.org changed reviewers:
+ lizeb@chromium.org, pasko@chromium.org

[azarchs, 2015-01-22 16:39:17]

PTAL.  This should clean up the output a bit, and set the stage for what I want
to do next.

[Benoit L, 2015-01-22 16:49:42]

On 2015/01/22 16:39:17, azarchs wrote:
> PTAL.  This should clean up the output a bit, and set the stage for what I
want
> to do next.

lgtm

[Benoit L, 2015-01-22 16:49:54]

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:119: return line  # Unprefixed case
nit: You can also use the

for prefix in _PREFIXES:
  ...
else:
  return line

idiom to remove the comment. Up to your personal preference.

[azarchs, 2015-01-22 17:10:12]

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:119: return line  # Unprefixed case
On 2015/01/22 16:49:53, lizeb wrote:
> nit: You can also use the
> 
> for prefix in _PREFIXES:
>   ...
> else:
>   return line
> 
> idiom to remove the comment. Up to your personal preference.

Maybe it's because I'm used to C, C++, Java, C#, Basic, and Visual Basic, but
that idiom just looks weird to me.

[pasko, 2015-01-22 17:37:14]

makes sense! thank you!

a few minor comments and enhancing a test

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:105: def _StripPrefixes(line):
it strips only one prefix, so maybe name it _StripPrefix?

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:106: """Get the symbol from a line with a
linker section name
nit: full stop at the end of the sentence. (here and below)

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:110: .text.SymbolName
nit: 4 chars shift to the right

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:137: if name == '' or name == '*' or name ==
'.text':
please enhance testGetSymbolsFromStream to verify that lines ".text" and 
".text.*" are filtered out from the input.

Also would be nice to mention that a prefixed start expansion would be filtered
out.

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile_unittest.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile_unittest.py:106: self.output = self.output + s
did the old version not work? if so, why?

[azarchs, 2015-01-22 17:44:30]

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:105: def _StripPrefixes(line):
On 2015/01/22 17:37:14, pasko wrote:
> it strips only one prefix, so maybe name it _StripPrefix?

Done.

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:106: """Get the symbol from a line with a
linker section name
On 2015/01/22 17:37:14, pasko wrote:
> nit: full stop at the end of the sentence. (here and below)

Done.

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:110: .text.SymbolName
On 2015/01/22 17:37:14, pasko wrote:
> nit: 4 chars shift to the right

Done.

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile.py:137: if name == '' or name == '*' or name ==
'.text':
On 2015/01/22 17:37:14, pasko wrote:
> please enhance testGetSymbolsFromStream to verify that lines ".text" and 
> ".text.*" are filtered out from the input.
> 
> Also would be nice to mention that a prefixed start expansion would be
filtered
> out.

Done.

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
File tools/cygprofile/patch_orderfile_unittest.py (right):

https://codereview.chromium.org/859303005/diff/1/tools/cygprofile/patch_order...
tools/cygprofile/patch_orderfile_unittest.py:106: self.output = self.output + s
On 2015/01/22 17:37:14, pasko wrote:
> did the old version not work? if so, why?

The old one resulted in extra '\n's between output lines.  This did not cause a
test failure in the old version because only one call was made to write().

[pasko, 2015-01-22 17:49:04]

lgtm, thank you, Adam!

[azarchs, 2015-01-22 17:49:26]

The CQ bit was checked by azarchs@chromium.org

[commit-bot: I haz the power, 2015-01-22 17:50:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/859303005/20001

[commit-bot: I haz the power, 2015-01-22 18:50:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-22 18:51:14]

Patchset 2 (id:??) landed as
https://crrev.com/81322057a224333cb87afaed0aca693af0a690f4
Cr-Commit-Position: refs/heads/master@{#312644}