Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: support msvc object files #20982

Open
ghost opened this issue Jul 11, 2017 · 195 comments
Open

cmd/link: support msvc object files #20982

ghost opened this issue Jul 11, 2017 · 195 comments

Comments

@ghost
Copy link

@ghost ghost commented Jul 11, 2017

I understand that the go linker cannot currently link msvc object files and also recognize that this issue is likely to be a low priority. However, it would be nice to support this because it would somewhat simplify windows workflow. This issue is mainly to understand how much effort this would be and/or what would be required.

@bradfitz bradfitz changed the title cgo/ support msvc object files using go linker cmd/link: support msvc object files Jul 11, 2017
@bradfitz bradfitz added the OS-Windows label Jul 11, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 11, 2017

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 12, 2017

@xoviat what is the problem that you are having? I need to be able to reproduce it here. So, please, provide all steps I will need to follow to reproduce this.

Thank you

Alex

PS: I won't have computer until end of July. I will have a look at this then.

@ghost
Copy link
Author

@ghost ghost commented Jul 12, 2017

what is the problem that you are having?

I haven't tried it yet, but I would like to specifically call c functions in msvc object files by linking them as .syso with the go linker. Everything that I have read indicates that this is not possible but I will create a procedure to reproduce the specific error that occurs.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 12, 2017

specifically call c functions in msvc object files by linking them as .syso with the go linker

Have you tried building these into a DLL, and use them from inside of DLL?

I will create a procedure to reproduce the specific error that occurs.

Please, do. Thank you.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 12, 2017

Have you tried building these into a DLL, and use them from inside of DLL?

That was actually my original plan. I am using swig so it is not so convenient to compile the generated c code separately and then rewrite the functions as DLL exports. It's not difficult, but it is annoying when the workflow with gcc is just go generate; go build.

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

Alright, I have a procedure. Start with these files:

hello.go:

package main

/*
	extern void hello();
*/
import "C"

func main() {
	C.hello()
}

hello.c:

#include <stdio.h>

extern void hello()
{
    printf("Hello World from C");
}

then run these commands (with msvc on path):

cl /c hello.c
mv hello.obj hello.syso
mv hello.c hello.c.bak
go build

Result:
Warning: corrupt .drectve at end of def file

When running the produced file:

Exception 0xc0000005 0x8 0x13 0x13
PC=0x13
signal arrived during external code execution

main._Cfunc_hello()
        _//_obj/_cgo_gotypes.go:41 +
main.main()
        C://hello.go:9 +0x27

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:2197 +0x1
rax     0x4a5960
rbx     0xc042045f78
rcx     0x4a9a20
rdi     0xc042045f78
rsi     0x4adc60
rbp     0xc042045f38
rsp     0x6dfd68
r8      0xc042016340
r9      0x0
r10     0xc04204faa0
r11     0x4783c2
r12     0x0
r13     0x6
r14     0x0
r15     0xf1
rip     0x13
rflags  0x10216
cs      0x33
fs      0x53
gs      0x2b
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 13, 2017

I do not have cl command installed on my pc. How do I install msvc?

Thank you.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

You need the "build tools 2017" here. Microsoft actually now allows anyone to use visual studio for free as long as it's not for commercial development. If it's too much trouble, I can just give you the object file if you want.

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

If only c++ was not a thing, I wouldn't have to worry about this. But it is, so the pain continues...

@bradfitz bradfitz added this to the Go1.10 milestone Jul 13, 2017
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 13, 2017

You need the "build tools 2017" here.

Got it. Thank you.

If it's too much trouble, I can just give you the object file if you want.

Yes, please, post hello.obj somewhere.

Thinking about this some more. You are actually using gcc to link object file compiled with msvc compiler. Can you try and do your exercise, but replacing "go build" step with gcc linking hello.obj to a C program? Perhaps it has been done before. I suspect, if we know how to do that, we might be able to do similar with Go.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

AFAIK lld https://github.com/llvm-mirror/lld supports msvc object files.

Object file is here: https://github.com/xoviat/msvcgo/blob/master/hello.syso

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 13, 2017

lld https://github.com/llvm-mirror/lld supports msvc object files

Go uses gcc linker (not lld) on Windows.

Object file is here: https://github.com/xoviat/msvcgo/blob/master/hello.syso

I will try it when I get home in August. thank you.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

Go uses gcc linker (not lld) on Windows.

I know. But lld is the best open source documentation of msvc object format.

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

I actually get same error from ld so the error is definitely coming from ld.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 13, 2017

I actually get same error from ld so the error is definitely coming from ld.

Yes. We need to work out how to build C program by compiling part with msvc, and linking with gcc.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

Forgive my ignorance, but what exactly gcc linking? Is it linking output of the go linker?

@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

Running objconv on the object files to convert to elf:

objconv -felf hello.obj hello.syso

We now have these errors:

hello.syso: In function `__local_stdio_printf_options':
(.text$mn+0x3): undefined reference to `?_OptionsStorage@?1??__local_stdio_printf_options@@9@9'
hello.syso: In function `_vfprintf_l':
(.text$mn+0x3a): undefined reference to `__stdio_common_vfprintf'
hello.syso: In function `printf':
(.text$mn+0x28): undefined reference to `__acrt_iob_func'
collect2.exe: error: ld returned 1 exit status
@ghost
Copy link
Author

@ghost ghost commented Jul 13, 2017

Possibly stdio is off limits?

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 26, 2017

Forgive my ignorance, but what exactly gcc linking? Is it linking output of the go linker?

You use 2 programs to build your Go program:

  • compiler converts your .go files (1 package at the time) into an object file stored under %GOPATH%/pkg directory;
  • linker that builds final .exe file from object files from under %GOPATH%/pkg.

Sometimes (when one of your packages uses Cgo), the Go linker calls external linker to find all bits that are implemented in C. Imagine you call printf from your C code. The C printf compiled code needs to be part of Go executable, but Go linker does not know where to get it. So Go linker calls external linker to include that code.

Current Go uses gcc compiler/linker to compile and link C code (we use mingw gcc). If you are going to compile your C code with different compiler (by Microsoft), you would have to use correspondent linker (by Microsoft) to find all external C code that the compiler created.

So, I guess, I was wrong about suggesting to use gcc linker. For general scenario, you would have to use both Microsoft compiler and linker. We would have to figure out what Microsoft linker requires as input and match that.

You might get away without MC linker, if your C code does not have any external code. Please try some really simple C program (like the one that adds 2 integers or something). That might just work as you have described above.

Alex

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 26, 2017

Possibly stdio is off limits?

I suspect you need to call Microsoft linker to find that code.

Alex

@mattn
Copy link
Member

@mattn mattn commented Jul 26, 2017

I don't make sure but

objconv -felf hello.obj hello.syso

Maybe, you should try to do coff or omf instead of elf?

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 26, 2017

Maybe, you should try to do coff or omf instead of elf?

@xoviat yes, you should not convert .obj files to elf, Windows version of gcc generates pe/coff files just like any other Windows compiler.

Alex

@ghost
Copy link
Author

@ghost ghost commented Jul 26, 2017

the Go linker calls external linker to find all bits that are implemented in C.

What is the specific command used? If I know the mingw command, then perhaps I can proceed down a file comparison path to try to get msvc to match what mingw is putting out.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 26, 2017

You can see the exact comment by running go build -ldflags=-v.

@ghost
Copy link
Author

@ghost ghost commented Aug 5, 2017

Okay, so from what I can tell:

ld (link -> go.obj) + (gcc -> obj files) ==> a.out.exe

Go appears to create a temporary directory with these files. Is there any way to preserve the temporary directory so that I can inspect its contents?

@mattn
Copy link
Member

@mattn mattn commented Aug 5, 2017

Try go build -work

WORK=C:\Users\mattn\AppData\Local\Temp\go-build566171254

Object files are remaining in this directory.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Apr 7, 2019

Is it possible for you to update to the latest SDK and Build Tools? I'm on SDK 10.0.17763.0. and version 15.9.28307.222 of the build tools/devenv

@cchamplin finally I managed to update my MSVC. I have now

c:\Users\Alex\dev\go\src>%GOVSVARSPATH%
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

c:\Users\Alex\dev\go\src>cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27030.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

c:\Users\Alex\dev\go\src>

I tried again this version

commit 6741b7009d1894b5bf535d82ad46f4a379651670 (HEAD)
Author: Caleb Champlin <caleb.champlin@gmail.com>
Date:   Sat Sep 8 00:26:32 2018 -0600

    cmd/compile: add support for MSVC toolchain to go build

    Allows building with MSVC as an external compiler/linker.

    Setting CC=cl.exe inside an MSVC environment will automatically
    build cgo executables using MSVC as the external compiler and
    linker.

    For the builders setting the environment variable GOVSVARSPATH
    to the location of a msvsvars.bat file and setting the
    environment variable GOTESTMSVC=1 will automatically cause
    all.bat to run tests and compiler with both gcc and MSVC.

    Updates #20982

    Change-Id: I44be1f43aa0d53a688c595bc8336e0364b809ced

and all.bat completes successfully. But I get these warnings.

##### ../misc/cgo/stdio

##### ../misc/cgo/life

##### ../misc/cgo/test
# _/c_/Users/Alex/dev/go/misc/cgo/test
issue28896.cgo2.c
.\issue28896.go(30): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'g1'
.\issue28896.go(36): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'f2'
# _/c_/Users/Alex/dev/go/misc/cgo/test
cthread_windows.c
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(37) : note: index 'i' range checked by comparison on this line
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : note: feeds call on this line
PASS
scatter = 00007FF670706000
hello from C
sqrt is: 0
ok      _/c_/Users/Alex/dev/go/misc/cgo/test    3.534s
# _/c_/Users/Alex/dev/go/misc/cgo/test
issue28896.cgo2.c
.\issue28896.go(30): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'g1'
.\issue28896.go(36): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'f2'
# _/c_/Users/Alex/dev/go/misc/cgo/test
cthread_windows.c
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(37) : note: index 'i' range checked by comparison on this line
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : note: feeds call on this line
PASS
scatter = 00007FF7DD026000
hello from C
sqrt is: 0
ok      _/c_/Users/Alex/dev/go/misc/cgo/test    3.229s
# _/c_/Users/Alex/dev/go/misc/cgo/test
issue28896.cgo2.c
.\issue28896.go(30): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'g1'
.\issue28896.go(36): warning C4820: '<anonymous-tag>': '4' bytes padding added after data member 'f2'
# _/c_/Users/Alex/dev/go/misc/cgo/test
cthread_windows.c
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(37) : note: index 'i' range checked by comparison on this line
c:\users\alex\dev\go\misc\cgo\test\cthread_windows.c(39) : note: feeds call on this line
PASS
scatter = 00007FF655CF6000
hello from C
sqrt is: 0
ok      _/c_/Users/Alex/dev/go/misc/cgo/test    3.172s

##### ../test/bench/go1
testing: warning: no tests to run
PASS
ok      _/c_/Users/Alex/dev/go/test/bench/go1   3.178s

##### ../test

##### API check
+pkg go/build, type Context struct, CompilerType string
+pkg go/build, type Package struct, CgoMSCFLAGS []string
+pkg go/build, type Package struct, CgoMSCPPFLAGS []string
+pkg go/build, type Package struct, CgoMSCXXFLAGS []string
+pkg go/build, type Package struct, CgoMSLDFLAGS []string
+pkg os, method (*File) SyscallConn() (syscall.RawConn, error)

ALL TESTS PASSED

---
Installed Go for windows/amd64 in c:\Users\Alex\dev\go
Installed commands in c:\Users\Alex\dev\go\bin
*** You need to add c:\Users\Alex\dev\go\bin to your PATH.

c:\Users\Alex\dev\go\src>

And also I noticed that you execute all tests twice.

...
##### API check
+pkg go/build, type Context struct, CompilerType string
+pkg go/build, type Package struct, CgoMSCFLAGS []string
+pkg go/build, type Package struct, CgoMSCPPFLAGS []string
+pkg go/build, type Package struct, CgoMSCXXFLAGS []string
+pkg go/build, type Package struct, CgoMSLDFLAGS []string
+pkg os, method (*File) SyscallConn() (syscall.RawConn, error)

ALL TESTS PASSED

**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'


##### Testing packages.
ok      archive/tar     0.256s
ok      archive/zip     2.070s
ok      bufio   (cached)
ok      bytes   1.624s
ok      compress/bzip2  (cached)
ok      compress/flate  1.067s
...

Why?

Alex

@cchamplin
Copy link

@cchamplin cchamplin commented Apr 8, 2019

@alexbrainman The warnings can likely be suppressed for these tests, especially in this case the spectre warnings.

For the tests executing twice, I believe it was done that way for the builder, run through the tests once with gcc compilation then run through them again with MSVC (when enabled) to ensure compilation with both toolchains works. I'm not sure what's desired here, so I can change it so that when all.bat is called with MSVC enabled we only run the tests with MSVC, let me know. In that case I assume additional builders would need to be added for just MSVC, as opposed to some builders also building with MSVC?

@deadprogram
Copy link

@deadprogram deadprogram commented Apr 8, 2019

Agreed that on Windows it will be needed to run one builder using MinGW-W64 toolchain and another with the MSVC toolchain in order to test both scenarios.

@mxplusb
Copy link

@mxplusb mxplusb commented Apr 8, 2019

I believe it was done that way for the builder, run through the tests once with gcc compilation then run through them again with MSVC (when enabled) to ensure compilation with both toolchains works.

I agree this should be the preferred behaviour so there's a guarantee it doesn't break existing workloads if both toolchains are present.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Apr 19, 2019

The warnings can likely be suppressed for these tests, especially in this case the spectre warnings.

Go command does not display warnings, so warnings should not be displayed.

Also I noticed that go env command does not display CC_FOR_CGO variable. If CC_FOR_CGO affects how go build works, then CC_FOR_CGO should be displayed along with others. Do we really need CC_FOR_CGO variable? What else, but CC_FOR_CGO=gcc can it be?

For the tests executing twice, I believe it was done that way for the builder, run through the tests once with gcc compilation then run through them again with MSVC (when enabled) to ensure compilation with both toolchains works.

A lot of tests don't use gcc. It is a waste of people's / computer's time when package tests run twice for no reason. As you can see from #20982 (comment) archive/zip package tests were run twice - I don't see need for second run.

I am not sure how to arrange that, but maybe you should replace your call of go tool dist test in run.bat to do just a selection of tests that are needed to test MSVC build. See, for example, go tool dist test -h on how you can select just some specific tests. @bradfitz maybe we could create some flag or environment variable that will tell dist command to run MSVC specific tests.

But in general your https://go-review.googlesource.com/c/go/+/133946/5 is building executable for me. For example, I can build executable with gcc:

c:\Users\Alex\dev\src\issue\go\20982\hello>type *

hello.c


#include <stdio.h>

extern void hello()
{
    printf("Hello World from C");
}

hello.go


package main

/*
        extern void hello();
*/
import "C"
import "fmt"

func main() {
    fmt.Println("Hello from Go!")
        C.hello()
}

c:\Users\Alex\dev\src\issue\go\20982\hello>go build

c:\Users\Alex\dev\src\issue\go\20982\hello>hello
Hello from Go!
Hello World from C
c:\Users\Alex\dev\src\issue\go\20982\hello>

and then I can build executable with MSVC

c:\Users\Alex\dev\src\issue\go\20982\hello>%GOVSVARSPATH%
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

c:\Users\Alex\dev\src\issue\go\20982\hello>set CC=cl

c:\Users\Alex\dev\src\issue\go\20982\hello>go build
# issue/go/20982/hello
hello.c
C:\Program Files (x86)\Windows Kits\10\include\10.0.17763.0\ucrt\stdio.h(948): warning C4710: 'int printf(const char *const ,...)': function not inlined
C:\Program Files (x86)\Windows Kits\10\include\10.0.17763.0\ucrt\stdio.h(948): note: see declaration of 'printf'

c:\Users\Alex\dev\src\issue\go\20982\hello>hello
Hello from Go!
Hello World from C
c:\Users\Alex\dev\src\issue\go\20982\hello>

So building with MSVC will require users to run MS batch file to set all environment variables (see GOVSVARSPATH in my output) - and that, I assume, standard and has nothing to do with Go. And set CC=cl. Simple enough.

The only issue I see with this approach is that MSVC users will still require to have gcc installed. gcc is used by cmd/cgo to discover all C interfaces - this part is not implemented by using MSVC. Sounds unusual for people who do not know how cmd/cgo works, but it is what it is.

@ianlancetaylor and @bradfitz does this all sounds reasonable to push this forward? Should we try and make this available in go1.13 ?

Builders that have MSVC already installed can test all new code. Other builders will just skip the tests.

Alex

@mxplusb
Copy link

@mxplusb mxplusb commented Apr 19, 2019

gcc is used by cmd/cgo to discover all C interfaces - this part is not implemented by using MSVC.

How can that be overcome? Ideally it'd be great if there's no requirement for gcc, just MSVC.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Apr 19, 2019

How can that be overcome?

Parts of cmd/cgo that use gcc needs to be rewritten to use MSVC. If it is possible.

Alex

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@kesmit13
Copy link

@kesmit13 kesmit13 commented Sep 10, 2019

I was wondering if this feature is closer to making it into production? I see that it was tagged for 1.13, and that just came out. I'm trying to build Python extensions using a Go c-archive build which is linked to the extension code, but Python requires MSVC.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 10, 2019

I don't know what the status of this issue is. It's been kicked forward from release to release, so I'm going to move it to unplanned. If it gets into 1.14, great.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.14, Unplanned Sep 10, 2019
@kesmit13
Copy link

@kesmit13 kesmit13 commented Sep 11, 2019

Even with the double-compiler requirement, I'd love to see the current implementation make it into the next release. It at least makes it possible to compile executables/libraries using MSVC, which is better than nothing.

@mxplusb
Copy link

@mxplusb mxplusb commented Sep 11, 2019

Even with the double-compiler requirement, I'd love to see the current implementation make it into the next release.

I think this would be a good "beta" implementation. It would allow bugs to be flushed out and the implementation to start to mature. What else is left to be done before it could go through a serious review?

@cchamplin
Copy link

@cchamplin cchamplin commented Sep 13, 2019

So from my perspective and what I can recall from the last time I looked at this (~6 months ago) there were a dozen or so cleanup items that needed to be completed for the code review process. Additional work might also be needed to deduplicate some tests that were basically copied with very minor modifications to work with a different toolchain. There is also a need to review and turn off certain msvc warnings.

Major outstanding issues include:

We can't do type analysis without gcc, there is no simple or immediate solution to this.

Finalizing the setup and how everything is going to work for the builders.

I believe there have been some fairly significant changes to the compiler and linker in the last 6 months that may severely impact the current work done for this, but I haven't been been able to confirm that. Due to the complexity and high learning curve around navigating Go's compiler/linker/toolchain it may take considerable time for me to get myself up to speed enough to figure the impact if any the changes have on this body of work.

There is no support for the complex numeric types. MSCVs C lib is simply incompatible. I believe there is likely a way to shim this with C++ but I'm uncertain, perhaps someone from MS familiar with the MSVC tool chains can chime in.

Unfortunately life and other projects have gotten pretty heavily in the way of me putting more time towards this but I will try and spend some time on this in the upcoming weeks.

@mxplusb
Copy link

@mxplusb mxplusb commented Sep 13, 2019

@manbearian this issue is about adding MSVC support to golang. Who on the MSVC team would be good to consult for MSVC questions?

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Sep 15, 2019

@kesmit13 and @mxplusb

This

https://go-review.googlesource.com/c/go/+/133946/5

is @cchamplin latest code that works.

My current concern with the code, that it is unclear how to use it. What command do you run? How is my environment needs to be setup?

I think the next step is for someone to try and use that code, and see if they can actually use it. If it works, then the steps need to be documented, so others can use the code too. Maybe it is too complicated, or does not work. Than we need to adjust the code accordingly.

Once we are happy with current state of the code, we should rebase it onto master. There change adjusted too many different files, so it won't be easy to rebase. Maybe we could adjust the change to make less changes, so it is easier to rebase.

Alex

@cchamplin
Copy link

@cchamplin cchamplin commented Nov 18, 2019

@mxplusb @kesmit13 The current implementation has been rebased against master and can be found here: https://go-review.googlesource.com/c/go/+/133946/6

@alexbrainman

My current concern with the code, that it is unclear how to use it. What command do you run? How is my environment needs to be setup?

I agree that documentation needs to be added on how to use this. I'm not entirely sure where to put it though, let me know if you have thoughts. Idk if it should go under the cmd/go documentation for building/running via MSVC, or if it should live under the CGO documentation, or maybe somewhere else entirely?

A lot of tests don't use gcc. It is a waste of people's / computer's time when package tests run twice for no reason. As you can see from #20982 (comment) archive/zip package tests were run twice - I don't see need for second run.

I am not sure how to arrange that, but maybe you should replace your call of go tool dist test in run.bat to do just a selection of tests that are needed to test MSVC build. See, for example, go tool dist test -h on how you can select just some specific tests. @bradfitz maybe we could create some flag or environment variable that will tell dist command to run MSVC specific tests.

The change already includes a environment variable added to dist to let it know we are doing MSVC things "GO_TEST_TESTINGMSVC" I believe. What I am not sure about is how we will filter the tests to be ran down to things that only MSVC could impact if we aren't running all tests twice. Potentially just limiting it to CGO related tests, but I believe there are CGO tests all over the place and I don't know if dist has a mechanism to filter those out?

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Nov 26, 2019

I'm not entirely sure where to put it though, let me know if you have thoughts.

I think before updating any documentation, you should just describe how to build Cgo code with MSVC here. For @mxplusb and @kesmit13 so they can try your changes.

For example, to use gcc to build Cgo code, I

  1. adjust my %PATH% to include gcc.exe
  2. build Go first by running make.bat from %GOROOT%\src directory
  3. use standard go build and go install commands

How do I do the same with MSVC?

What I am not sure about is how we will filter the tests to be ran down to things that only MSVC could impact if we aren't running all tests twice. Potentially just limiting it to CGO related tests, but I believe there are CGO tests all over the place and I don't know if dist has a mechanism to filter those out?

I would start with %GOROOT%\misc\cgo directory. Whatever source you can build there you should build. Anything above that is up to you.

Alex

@mxplusb
Copy link

@mxplusb mxplusb commented Dec 11, 2019

@cchamplin sorry for the delays on this, I will do my best to test it this week and let you know how it goes.

@manbearian
Copy link

@manbearian manbearian commented Dec 13, 2019

Hi, folks sorry to be late to the party... i can try and answer questions about MSVC functionality or direct you to folks who can. i read through the conversation, but unfortunately, i'm not familiar enough with go to understand the GCC functionality required that may or may not be missing from MSVC.

@maoxs2 maoxs2 mentioned this issue Apr 17, 2020
@dhinesherode91
Copy link

@dhinesherode91 dhinesherode91 commented Jun 1, 2020

Hi Folks,
Is this feature got released in Go 1.14 ?
If released please share any available documents to make use of it.
If not yet released, is there any idea on when it will be released ?
Eagerly waiting for the MSVC support.
Thanks in advance :)

@manbearian
Copy link

@manbearian manbearian commented Jun 1, 2020

Hi, no one ever responded to my request for more information, so i'm not sure what is needed. Can someone fill me in please?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 1, 2020

Unfortunately, I don't know if anybody is actively working on this issue.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jun 2, 2020

Hi, no one ever responded to my request for more information, ...

Hello @manbearian

Which request is that? What information do you need?

Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.