Show detailed difference on unexpected difference.

Show detailed difference on unexpected difference.

Show object file difference to make it easy to distinguish what
caused the difference.

BUG=314403
Committed: https://crrev.com/83bdd08debd6f7691d61ba8b827146642f09d8ec
Cr-Commit-Position: refs/heads/master@{#414979}

[Yoshisato Yanagisawa, 2016-08-24 10:01:17]

yyanagisawa@chromium.org changed reviewers:
+ dpranke@chromium.org, maruel@chromium.org

[Yoshisato Yanagisawa, 2016-08-24 10:01:17]

Currently, it is difficult for me to go into details on why two binaries are
different.
Let me show difference of object files that are used for making targets to make
it easy to understand why.

[Dirk Pranke, 2016-08-24 18:30:18]

lgtm

[M-A Ruel, 2016-08-24 18:47:27]

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
File tools/determinism/compare_build_artifacts.py (right):

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:505: out =
subprocess.check_output(['ninja', '-C', build_dir,
There's a problem as the directory was renamed, so calling ninja after the
directory was renamed will likely not result in the expected outcome, as the
ninja files may have absolute paths. So it is important to rename the directory
:/ before calling ninja.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:527: deps =
set(first_deps).union(set(second_deps))
the difference should be empty ?

[Yoshisato Yanagisawa, 2016-08-25 08:47:29]

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
File tools/determinism/compare_build_artifacts.py (right):

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:505: out =
subprocess.check_output(['ninja', '-C', build_dir,
On 2016/08/24 18:47:27, M-A Ruel wrote:
> There's a problem as the directory was renamed, so calling ninja after the
> directory was renamed will likely not result in the expected outcome, as the
> ninja files may have absolute paths. So it is important to rename the
directory
> :/ before calling ninja.

I have never seen absolute object file path names but I am not confident.  Let
me rename to possibly original directory, and warn and skip if absolute object
file path is used.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:527: deps =
set(first_deps).union(set(second_deps))
On 2016/08/24 18:47:27, M-A Ruel wrote:
> the difference should be empty ?

I am not confident.  However, I will simplify the code expecting no difference
between first_deps and second_deps.

[M-A Ruel, 2016-08-25 15:46:26]

lgtm anyhow, the rest can be done later.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
File tools/determinism/compare_build_artifacts.py (right):

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:505: out =
subprocess.check_output(['ninja', '-C', build_dir,
On 2016/08/25 08:47:29, Yoshisato Yanagisawa wrote:
> On 2016/08/24 18:47:27, M-A Ruel wrote:
> > There's a problem as the directory was renamed, so calling ninja after the
> > directory was renamed will likely not result in the expected outcome, as the
> > ninja files may have absolute paths. So it is important to rename the
> directory
> > :/ before calling ninja.
> 
> I have never seen absolute object file path names but I am not confident.  Let
> me rename to possibly original directory, and warn and skip if absolute object
> file path is used.

zforman@ worked on making this work on linux but this is likely not the case on
OSX and Windows, probably worth double checking on at least one of the other
platform eventually.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:527: deps =
set(first_deps).union(set(second_deps))
On 2016/08/25 08:47:29, Yoshisato Yanagisawa wrote:
> On 2016/08/24 18:47:27, M-A Ruel wrote:
> > the difference should be empty ?
> 
> I am not confident.  However, I will simplify the code expecting no difference
> between first_deps and second_deps.

I guess my question was more to print out the differences to be able to monitor
when this happens.

[Yoshisato Yanagisawa, 2016-08-29 05:44:50]

thanks for the review, I am going to run CQ.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
File tools/determinism/compare_build_artifacts.py (right):

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:505: out =
subprocess.check_output(['ninja', '-C', build_dir,
On 2016/08/25 15:46:26, M-A Ruel wrote:
> On 2016/08/25 08:47:29, Yoshisato Yanagisawa wrote:
> > On 2016/08/24 18:47:27, M-A Ruel wrote:
> > > There's a problem as the directory was renamed, so calling ninja after the
> > > directory was renamed will likely not result in the expected outcome, as
the
> > > ninja files may have absolute paths. So it is important to rename the
> > directory
> > > :/ before calling ninja.
> > 
> > I have never seen absolute object file path names but I am not confident. 
Let
> > me rename to possibly original directory, and warn and skip if absolute
object
> > file path is used.
> 
> zforman@ worked on making this work on linux but this is likely not the case
on
> OSX and Windows, probably worth double checking on at least one of the other
> platform eventually.

I checked the code seems to work on Win.

https://codereview.chromium.org/2278493002/diff/1/tools/determinism/compare_b...
tools/determinism/compare_build_artifacts.py:527: deps =
set(first_deps).union(set(second_deps))
On 2016/08/25 15:46:26, M-A Ruel wrote:
> On 2016/08/25 08:47:29, Yoshisato Yanagisawa wrote:
> > On 2016/08/24 18:47:27, M-A Ruel wrote:
> > > the difference should be empty ?
> > 
> > I am not confident.  However, I will simplify the code expecting no
difference
> > between first_deps and second_deps.
> 
> I guess my question was more to print out the differences to be able to
monitor
> when this happens.

I think current code still help us to find this kind of difference, not clearly
mentions which one is larger, though.
Let me update the code when we actually see this case.

[Yoshisato Yanagisawa, 2016-08-29 05:44:53]

The CQ bit was checked by yyanagisawa@chromium.org

[Yoshisato Yanagisawa, 2016-08-29 05:44:54]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2278493002/#ps20001
(title: "rename before ninja, do not expect deps are different.")

[commit-bot: I haz the power, 2016-08-29 05:45:15]

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, 2016-08-29 06:40:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-29 06:43:51]

Description was changed from

==========
Show detailed difference on unexpected difference.

Show object file difference to make it easy to distinguish what
caused the difference.

BUG=314403
==========

to

==========
Show detailed difference on unexpected difference.

Show object file difference to make it easy to distinguish what
caused the difference.

BUG=314403

Committed: https://crrev.com/83bdd08debd6f7691d61ba8b827146642f09d8ec
Cr-Commit-Position: refs/heads/master@{#414979}
==========

[commit-bot: I haz the power, 2016-08-29 06:43:52]

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