New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 848134 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

link.exe fails to link content_public_common_mojo_bindings_shared due to speech_recognition_error_code_shared.dll.lib not existing

Project Member Reported by yoichio@chromium.org, May 31 2018

Issue description

There is a build error on win with following gn args:
target_cpu = "x86"
enable_nacl = false
is_component_build = true
is_debug = true
use_lld = false
win_console_app = true
use_goma = true

Here is output:
$ autoninja webkit_unit_tests
[86/12905] LINK(DLL) content_public_common_mojo_bindings_shared
LINK : fatal error LNK1104: cannot open file 'speech_recognition_error_code_shared.dll.lib'

Though I can build the file, there is still no
speech_recognition_error_code_shared.dll.lib:
$ autoninja speech_recognition_error_code_shared.dll.lib
$ ls speech_recognition_error_code_shared.*
speech_recognition_error_code_shared.dll     speech_recognition_error_code_shared.ilk
speech_recognition_error_code_shared.dll.pdb

I guess if we doesn't have any class/struct, some generator
doesn't create *dll.lib but someone requres it.
Here is generated mojom.h:
https://cs.chromium.org/chromium/src/out/Debug/gen/content/public/common/speech_recognition_error_code.mojom.h?q=speech_recognition_error_code+package:chromium&dr=CSs

Following is discussion log from 
https://chromium-review.googlesource.com/c/chromium/src/+/1078027

Nico Weber:
This is probably more an llvm-lib than an lld thing.
I guess llvm-lib allows this only when building thin archives (?) Else we could use llvm-lib even if use_lld is false.
It'd be good to have a bug to discuss options in. If we want to support this, the mojo compiler should probably write this dummy struct if there are no symbols, or it should use source_sets instead of static libs.

Reid Kleckner:
My first guess is that the Visual C++ linker doesn't make import libraries (.lib files) when the DLL has zero exports. This enum-only mojom file results in a binding DLL that doesn't export anything. LLD appears to always create an accompanying .lib file when it makes a DLL, even if there are no exports.
I think llvm-lib is only used to make static archives, so I don't think it's related to this issue.
Perhaps a proper fix would be for the mojo C++ binding generator to always export at least one dummy symbol that nobody uses when no structs are involved.

Ken Rockot:
The mojom GN template spits out component targets, so I guess we could just set static_component_type = "source_set" in those. I don't know the reasons why component() even defaults to static_library though, so maybe I'm overlooking something. If it works, I'd probably prefer that over generating a dummy symbol.

Peter Collingbourne:
Or we can change our link.exe wrapper to create an empty import library before running the linker.

Nico Weber:
component used to use source_set, but then all .obj files in there get linked in instead of just the needed ones, making binaries larger and links slower, so we changed that a while ago. If this generates components, then the dummy symbol is probably the way to go if we want to care about link.exe (which we probably do want to care about).


 
I don't know appropriate person/team/component/way.
Nico, please reassign it if you aren't.

Thanks.

Comment 2 by thakis@chromium.org, May 31 2018

Labels: -Pri-1 Pri-2
I like Peter's idea.

But since lld is the default linker now, I don't think this is p1.
Also visible on https://ci.chromium.org/buildbot/chromium.clang/CrWinClngLLDdbg/716 (which now uses link.exe)
Summary: link.exe fails to link content_public_common_mojo_bindings_shared due to speech_recognition_error_code_shared.dll.lib not existing (was: Fix build a mojo file w/o class/struct on win)
So content/public/common/resource_type.mojom has been around for a long time and also only declares an enum. Why does that not result in the same error?

speech_recognition_error_code sets this:

  export_class_attribute = "CONTENT_EXPORT"
  export_define = "CONTENT_IMPLEMENTATION=1"
  export_header = "content/common/content_export.h"

Maybe that triggers some magic in the mojo compiler?

...yeah, if that's set then https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/mojom.gni?type=cs&q=export_class_attribute+file:%5C.gni&sq=package:chromium&g=0&l=531 sets shared_component_output_name and only if that's set then https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/mojom.gni?type=cs&q=export_class_attribute+file:%5C.gni&sq=package:chromium&g=0&l=672 ends up creating a component() for the mojom thingy.


So I think the Right Fix is to remove these 3 lines from the gn file.

Maybe the mojo compiler could have an error where if you only define enums, it shouts at you if you create a component(), so that we don't have to rely on link.exe for this.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2018

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

commit e8568ea75e73a6cc1c91c91f42a3b567fec66591
Author: Nico Weber <thakis@chromium.org>
Date: Fri Jun 01 16:51:01 2018

win: Fix component build with link.exe.

speech_recognition_error_code.mojom only declares an enum, so there's no need
to create a component for it (which is triggered by setting the export attribs).
And link.exe doesn't create an import lib for a dll without exports, so a link
further down fails with link.exe if we do create such a component -- so just
don't set the export attribs. See bug for details.

Bug:  848134 
Change-Id: I0c3608caa7ee943690422968766fb892f5ab6ca6
Reviewed-on: https://chromium-review.googlesource.com/1082575
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563679}
[modify] https://crrev.com/e8568ea75e73a6cc1c91c91f42a3b567fec66591/content/public/common/BUILD.gn

Status: Fixed (was: Untriaged)

Sign in to add a comment