New issue
Advanced search Search tips

Issue 761849 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 3
Type: Bug

Blocked on:
issue 762072

Blocking:
issue 495204



Sign in to add a comment

Reorder_imports doesn't work in linux->win cross builds

Project Member Reported by thakis@chromium.org, Sep 4 2017

Issue description

[1/321] ACTION //chrome:reorder_imports(//build/toolchain/win:win_clang_x86)
FAILED: chrome.exe chrome.exe.pdb 
python ../../build/win/reorder-imports.py -i initialexe -o . -a x86
Traceback (most recent call last):
  File "../../build/win/reorder-imports.py", line 57, in <module>
    sys.exit(main(sys.argv[1:]))
  File "../../build/win/reorder-imports.py", line 54, in main
    return reorder_imports(opts.input, opts.output, opts.arch)
  File "../../build/win/reorder-imports.py", line 34, in reorder_imports
    subprocess.check_call(args)
  File "/usr/lib/python2.7/subprocess.py", line 535, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 522, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
    raise child_exception
OSError: [Errno 20] Not a directory


The problem is that reorder-imports.py uses a syzygy binary, which is only checked in as .exe (which can't run on non-win hosts). We already have pefile.py in the tree, and swapimport.exe is small (https://github.com/google/syzygy/blob/master/syzygy/swapimport/swapimport_app.cc), so probably easiest to just reimplement it in python.


This is the only thing missing for getting chrome.exe to build.
 
Status: started (was: Untriaged)
https://chromium-review.googlesource.com/c/chromium/src/+/649810
Blockedon: 762072
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf357e4d70c3f537994bbbce771b678bc6ef2d3e

commit cf357e4d70c3f537994bbbce771b678bc6ef2d3e
Author: Nico Weber <thakis@chromium.org>
Date: Wed Sep 06 01:01:04 2017

Make reorder-imports.py not depend on .exe files.

This attempts to port https://github.com/google/syzygy/blob/master/syzygy/swapimport/swapimport_app.cc#L143

The motivation is to be able to run this on non-Windows hosts. This is
the last thing that's required to be able to link chrome.exe (with some
other local changes).
Not relying on a checked-in binary also makes changing the script easier.

Bug:  761849 
Change-Id: I9956ce5c0929dcb3fd1309a8f0b2acc80b7b92ce
Reviewed-on: https://chromium-review.googlesource.com/649810
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499824}
[modify] https://crrev.com/cf357e4d70c3f537994bbbce771b678bc6ef2d3e/build/win/reorder-imports.py

Comment 4 by thakis@chromium.org, Sep 10 2017

pefile.py in third_party is currently checked out only on Windows. So, missing here:

* Decide if we want `target_os = ['win']` in .gclient when targeting win. Probably a good idea anyhow as prerequisite for more automatic toolchain setup. (I've removed cygwin and psyco from the win deps, so those deps shouldn't be too burdensome.)

* standardize on just the pefile in tools/symsrc (bugs 762072). The prior fix is better, but bug 762072 is a nice cleanup anyhow, and it'd fix this bug here.

Comment 5 by thakis@chromium.org, Oct 12 2017

Status: Fixed (was: Started)
We want the target_os bit, so this is done. Still might want to do the cleanup.

Sign in to add a comment