"alwayslink=1" is broken on Windows and VS 2017

[snnn]

Please provide the following information. The more we know about your system and use case, the more easily and likely we can help.

Description of the problem / feature request / question:

"alwayslink=1" doesn't work anymore.

If possible, provide a minimal example to reproduce the problem:

Build file:

cc_binary(
    name = "h",
    srcs = ["main.cpp"],
    deps = [":a1"]
)

cc_library(
name="a1",
srcs=["a1.cpp"],
visibility=["//visibility:public"],
alwayslink=1,
linkstatic=1
)

main.cpp

#include <stdio.h>

int main(){
	printf("main\n");
	return 0;
}

a1.cpp

#include <stdlib.h>
#include <stdio.h>

class Hello{
public:
Hello(){
	printf("hello\n");
}
};

static Hello h;

When you run "h.exe", the expected output should be

---

hello
main

---

Environment info

• Operating System:
  Windows 10, Visual Studio 2017(15.4.1)

• Bazel version (output of bazel info release):
  0.7.0

Have you found anything relevant by searching the web?

no

Anything else, information or logs or outputs that would be helpful?

I've found the root cause: In "h.exe-2.params", the file path after "/WHOLEARCHIVE:" should use '\' as path separator, not '/'

[meteorcloudy]

@snnn It does produce the correct output on my machine

pcloudy@pcloudy0-w MSYS ~/workspace/my_tests/always_link_test
$ bazel build :h
INFO: Analysed target //:h.
INFO: Found 1 target...
Target //:h up-to-date:
  C:/tmp/_bazel_pcloudy/otvst_nr/execroot/__main__/bazel-out/msvc_x64-fastbuild/bin/h.exe
INFO: Elapsed time: 0.777s, Critical Path: 0.29s
INFO: Build completed successfully, 6 total actions

pcloudy@pcloudy0-w MSYS ~/workspace/my_tests/always_link_test
$ ./bazel-bin/h
hello
main

pcloudy@pcloudy0-w MSYS ~/workspace/my_tests/always_link_test
$ cat ./bazel-out/msvc_x64-fastbuild/bin/h.exe-2.params
bazel-out/msvc_x64-fastbuild/bin/_objs/h/main.o
/WHOLEARCHIVE:bazel-out/msvc_x64-fastbuild/bin/liba1.lo

Hmm,,
I believe the compiler will understand /, it shouldn't cause any problem.
Which visual C++ build tool version are you using?

[meteorcloudy]

Can you run the link command manually to see if it works

link.exe /OUT:h.exe /MACHINE:X64 @bazel-out/msvc_x64-fastbuild/bin/h.exe-2.params /DEFAULTLIB:msvcrt.lib /DEBUG:FASTLINK /INCREMENTAL:NO

[snnn]

D:\bazel_test>link.exe /OUT:h.exe /MACHINE:X64 @bazel-out/msvc_x64-fastbuild/bin/h.exe-2.params /DEFAULTLIB:msvcrt.lib /DEBUG:FASTLINK /INCREMENTAL:NO
Microsoft (R) Incremental Linker Version 14.11.25547.0
Copyright (C) Microsoft Corporation.  All rights reserved.

bazel-out/msvc_x64-fastbuild/bin/_objs/h/main.o
/WHOLEARCHIVE:bazel-out/msvc_x64-fastbuild/bin/liba1.lo

D:\bazel_test>h.exe
main

[snnn]

Hi @meteorcloudy
My VS version is : Visual Studio 2017 (15.4.1). What's yours?
I've tried to copy h.exe-2.params to another position and replace '/' with '\', then the link command can generate a correct exe.

[meteorcloudy]

I installed Visual C++ Build Tool 2015,
The linker version is:

C:\tools\msys64\home\pcloudy\workspace\my_tests\always_link_test>link
Microsoft (R) Incremental Linker Version 14.00.24210.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Interesting, I'll test this with Visual Studio 2017

[snnn]

I got the same result as you with VS 2015.

[meteorcloudy]

I can also reproduce this with VS 2017, it's surprising that VS 2017 stopped supporting / in parameter file for /WHOLEARCHIVE..
@laszlocsomor Is there an easy way to switch to backslash?

[meteorcloudy]

Will anything go wrong if we use \\ as primary separator?

bazel/src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java

Lines 53 to 58 in 755e176

	private static class Helper extends PathFragment.Helper {
	private static final char SEPARATOR_CHAR = '/';
	// TODO(laszlocsomor): Lots of internal PathFragment operations, e.g. getPathString, use the
	// primary separator char and do not use this.
	private static final char EXTRA_SEPARATOR_CHAR = '\\';

[meteorcloudy]

Looks like so, after switching the two, I got:

pcloudy@pcloudy0-w MSYS ~/workspace/my_tests/always_link_test                                                                                                                                                                                   
$ bazel build :h -s                                                                                                                                                                                                                             
Extracting Bazel installation...                                                                                                                                                                                                                
...........                                                                                                                                                                                                                                     
ERROR: C:\tmp\_bazel_pcloudy\otvst_nr\external\bazel_tools\tools\jdk\BUILD:1:1: package names may contain only A-Z, a-z, 0-9, '/', '-', '.', ' ', '$', '(', ')' and '_'                                                                         
ERROR: C:\tmp\_bazel_pcloudy\otvst_nr\external\bazel_tools\tools\jdk\BUILD:74:1: @bazel_tools//tools\jdk:ijar: invalid label 'ijar\ijar.exe' in element 0 of attribute 'srcs' in 'filegroup' rule: invalid target name 'ijar\ijar.exe': target n
ames may not contain '\'                                                                                                                                                                                                                        
ERROR: C:\tmp\_bazel_pcloudy\otvst_nr\external\bazel_tools\tools\jdk\BUILD:85:1: @bazel_tools//tools\jdk:singlejar: invalid label 'singlejar\SingleJar_deploy.jar' in element 0 of attribute 'srcs' in 'filegroup' rule: invalid target name 'si
nglejar\SingleJar_deploy.jar': target names may not contain '\'                                                                                                                                                                                 
ERROR: C:\tmp\_bazel_pcloudy\otvst_nr\external\bazel_tools\tools\jdk\BUILD:203:1: @bazel_tools//tools\jdk:package-srcs: invalid label 'ijar\ijar.exe' in element 8 of attribute 'srcs' in 'filegroup' rule: invalid target name 'ijar\ijar.exe':
 target names may not contain '\'                                                                                                                                                                                                               

[laszlocsomor]

@meteorcloudy : I believe that would break things, but I don't know. For the record I didn't add that TODO, @haxorz did so he may have more context.

[laszlocsomor]

@meteorcloudy : Maybe update this logic to be Windows-aware?
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java#L1107

[snnn]

I found a temporary solution for me:

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 6af0118e7..1cc913ae4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -1891,6 +1891,7 @@ public class CppLinkActionBuilder {
           name = Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString();
         } else {
           name = inputArtifact.getExecPathString();
+            name = name.replace('/','\\');
         }

         librariesToLink.addValue(

[tmandry]

I feel somewhat strongly that this should be a P1 issue. I'm not fully aware of how your priorities are assigned, but Visual Studio 2017 is the latest version and has been out for awhile. I think it should be fully supported if Bazel advertises full Windows support for C++.

Is there any way to apply @snnn's fix for only Windows? It may be a hack, but it gets this working for now!

[snnn]

Hi @tmandry

Could you should me your Visual Studio version? with the minor version numbers.

[tmandry]

@snnn To be clear, I'm not suffering from this problem yet. I'm commenting because I'm in the process of transitioning our dev team to bazel, and some of them use Visual Studio. I wrote lavender for this purpose.

It makes it harder to build up support for a change like this when the docs say the latest version of Visual Studio isn't supported. This issue actually seems like really low-hanging fruit, which is why I commented. That said, I know the team has a lot of work to do!

[snnn]

Hi @tmandry

I have reported this problem to VS team, they should have done some fix in their side. So, if you are using the latest visual studio 2017, it should be ok. They said they had fixed it, I haven't verified.

[meteorcloudy]

@snnn Thanks for the reporting! I can confirm alwayslink now works with the latest VS 2017.
My MSVC is located at
C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.13.261280
14.13.261280 is the version number right?

[meteorcloudy]

I'll still try to make Bazel emit paths with \, but this issue should be fixed by upgrading VS 2017.

[snnn]

Yes, exactly. That is the toolset version number. You may install multiple VS toolsets inside the same VS installation. Version number of VS is something like "15.6.6".