New issue
Advanced search Search tips

Issue 774193 link

Starred by 1 user
Status: Fixed
Owner:
thakis@chromium.org
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 3
Type: Bug

Blocking:
issue 495204



Sign in to add a comment

come up with concrete plan for rc files for win cross builds

Project Member Reported by thakis@chromium.org, Oct 12 2017 
rc.exe is Windows-only.

I wrote a rc reimplementation that for chrome's rc files produces bit-for-bit identical outputs (https://github.com/nico/hack/tree/master/res, https://codereview.chromium.org/2628573003/). We could hook that up. However, currently rc is called as a gn tool("rc") and those can't currently have deps. So the options here are:

1. Put a prebuilt rc binary somewhere, pull via hook. A lot of procedure for a 110kB binary.

2. Allow gn tools to have deps, build my rc as part of the regular build.


We also had an option try and port my rc thing to LLVM. Sadly, that work isn't complete.


So probably prebuilt + my thing for now.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 19 2017 

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

commit 0c0ade8e414850c6cf64fc724344c6ba350cbe6d
Author: Nico Weber <thakis@chromium.org>
Date: Thu Oct 19 22:34:43 2017

Add a cross-platform resource compiler.

Also a stepping stone in getting deps from .rc files correct automatically.

The driver script rc.py works like rc.exe; it supports printing resource
names on /showIncludes:

$ cat test.rc
4 ICON "icon.ico"
$ build/toolchain/win/rc/rc.py /showIncludes test.rc
Note: including file: ./foo.h
Note: including file: /Users/thakis/src/chrome/src/icon.ico
$ file test.res
test.res: MSVC .res

The driver script behind the scenes calls a deps'd-in "rc" binary.
This CL includes a script to build new versions of that binary.
It's ~150kB on Mac and Linux, and ~600kb on Windows.

The compiler isn't hooked up to anything yet. I plan to change
tool_wrapper.py to call both it and rc.exe in ExecRcWrapper() when
on Windows (and compare the two outputs and make sure they're identical),
and to call only rc.py when on non-Windows.

Eventually I also want to make ExecRcWrapper() use rc.py's /showIncludes
to get dependencies for .rc files right.

The README.md has some more details. We're in the process of slowly
upstreaming this to LLVM, but it looks like that'll take a while longer,
so in the meantime let's do this until the LLVM version is ready.

Bug:  774193 , 132660 
Change-Id: I9a9ee1e271bf403ddb7f8d18ecd97730762c6cbc
Reviewed-on: https://chromium-review.googlesource.com/727110
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510244}
[modify] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/DEPS
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/.gitignore
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/README.md
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/linux64/rc.sha1
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/mac/rc.sha1
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/rc.py
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/upload_rc_binaries.sh
[add] https://crrev.com/0c0ade8e414850c6cf64fc724344c6ba350cbe6d/build/toolchain/win/rc/win/rc.exe.sha1
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 20 2017 

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

commit 82f6080f7d0e71eabce5af2e363d375db323bea7
Author: Nico Weber <thakis@chromium.org>
Date: Fri Oct 20 20:00:46 2017

Hook up new rc.py.

Two advantages:
* lets ninja track dependencies on resources referenced
  from .rc files
* allows building targets that use .rc files when
  cross-compiling chrome/win on non-Win hosts, e.g.
  gfx_unittests can now build on linux

Since we now get deps right automatically, remove a few manually-added
deps on .ico files.

Bug:  132660 , 774193 
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: Ia23ec5cba2bb06146650c40acf5b7754bbfe9a99
Reviewed-on: https://chromium-review.googlesource.com/729536
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510532}
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/build/toolchain/win/BUILD.gn
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/build/toolchain/win/rc/rc.py
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/build/toolchain/win/tool_wrapper.py
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/chrome/installer/mini_installer/BUILD.gn
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/chrome/installer/setup/BUILD.gn
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/docs/win_cross.md
[modify] https://crrev.com/82f6080f7d0e71eabce5af2e363d375db323bea7/sandbox/win/BUILD.gn

Comment 3 by thakis@chromium.org, Oct 20 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment