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/dist: Windows 2000 fails to find the boostrap tool to execute on tip #7242

Closed
andlabs opened this issue Jan 31, 2014 · 17 comments
Closed

cmd/dist: Windows 2000 fails to find the boostrap tool to execute on tip #7242

andlabs opened this issue Jan 31, 2014 · 17 comments
Milestone

Comments

@andlabs
Copy link
Contributor

@andlabs andlabs commented Jan 31, 2014

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. hg clone -u tip https://code.google.com/p/go
2. cd go\src
3. all.bat

What is the expected output?
A complete build.

What do you see instead?
# Building C bootstrap tool.
cmd/dist

# Building compilers and Go bootstrap tool.
'"\go_bootstrap"' is not recognized as an internal or external command,
operable program or batch file.

== was unexpected at this time.

Which compiler are you using (5g, 6g, 8g, gccgo)?
mingw-w64 i686-4.8.2-release-win32-sjlj-rt_v3-rev2.7z

Which operating system are you using?
Windows 2000 Professional Service Pack 4 (running in VirtualBox 4.3)

Which version are you using?  (run 'go version')
latest tip

Please provide any additional information below.
Windows 2000 is the earliest version listed as supported on
http://golang.org/doc/install, so I thought to use it for backwards compatibility
testing of my Windows API code.
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 2, 2014

Comment 1:

Perhaps GOTOOLDIR environment variable is not set properly in make.bat. Please, comment
out "@echo off" at the top of make.bat, like so:
c:\go\root\src>hg diff
diff -r e5b12367190b src/make.bat
--- a/src/make.bat      Fri Jan 31 17:22:10 2014 -0800
+++ b/src/make.bat      Sun Feb 02 12:05:21 2014 +1100
@@ -26,7 +26,7 @@
 :: to include all cgo related files, .c and .go file with "cgo"
 :: build directive, in the build. Set it to 0 to ignore them.
-@echo off
+rem @echo off
 :: Keep environment variables within this script
 :: unless invoked with --no-local.
c:\go\root\src>
then run make.bat and post output here.
Thank you.
Alex

Labels changed: added repo-main, os-windows.

@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 2, 2014

Comment 2:

C:\Documents and Settings\Pietro Gagliardi\go\src>rem @echo off 
C:\Documents and Settings\Pietro Gagliardi\go\src>if x == x--no-local goto nolocal 
C:\Documents and Settings\Pietro Gagliardi\go\src>if x == x--no-local goto nolocal 
C:\Documents and Settings\Pietro Gagliardi\go\src>setlocal
C:\Documents and Settings\Pietro Gagliardi\go\src>set GOBUILDFAIL=0 
C:\Documents and Settings\Pietro Gagliardi\go\src>if exist make.bat goto ok 
C:\Documents and Settings\Pietro Gagliardi\go\src>del /F ".\pkg\runtime\runtime_defs.go"
 2>NUL 
C:\Documents and Settings\Pietro Gagliardi\go\src>cd .. 
C:\Documents and Settings\Pietro Gagliardi\go>set GOROOT=C:\Documents and
Settings\Pietro Gagliardi\go 
C:\Documents and Settings\Pietro Gagliardi\go>cd src 
C:\Documents and Settings\Pietro Gagliardi\go\src>if "xC:\Documents and Settings\Pietro
Gagliardi\go" == "x" set GOROOT_FINAL=C:\Documents and Settings\Pietro Gagliardi\go 
C:\Documents and Settings\Pietro Gagliardi\go\src>set
DEFGOROOT=-DGOROOT_FINAL="\"C:\\Documents and Settings\\Pietro Gagliardi\\go\"" 
C:\Documents and Settings\Pietro Gagliardi\go\src>echo # Building C bootstrap tool. 
# Building C bootstrap tool.
C:\Documents and Settings\Pietro Gagliardi\go\src>echo cmd/dist 
cmd/dist
C:\Documents and Settings\Pietro Gagliardi\go\src>if not exist ..\bin\tool mkdir
..\bin\tool 
C:\Documents and Settings\Pietro Gagliardi\go\src>gcc -O2 -Wall -Werror -o
cmd/dist/dist.exe -Icmd/dist -DGOROOT_FINAL="\"C:\\Documents and Settings\\Pietro
Gagliardi\\go\"" cmd/dist/buf.c cmd/dist/build.c cmd/dist/buildgc.c cmd/dist/buildgo.c
cmd/dist/buildruntime.c cmd/dist/goc2c.c cmd/dist/main.c cmd/dist/windows.c
cmd/dist/arm.c 
C:\Documents and Settings\Pietro Gagliardi\go\src>if errorlevel 1 goto fail 
C:\Documents and Settings\Pietro Gagliardi\go\src>.\cmd\dist\dist env -wp  1>env.bat 
C:\Documents and Settings\Pietro Gagliardi\go\src>if errorlevel 1 goto fail 
C:\Documents and Settings\Pietro Gagliardi\go\src>call env.bat 
C:\Documents and Settings\Pietro Gagliardi\go\src>del env.bat 
C:\Documents and Settings\Pietro Gagliardi\go\src>echo.
C:\Documents and Settings\Pietro Gagliardi\go\src>if x == x--dist-tool goto copydist 
C:\Documents and Settings\Pietro Gagliardi\go\src>if x == x--dist-tool goto copydist 
C:\Documents and Settings\Pietro Gagliardi\go\src>echo # Building compilers and Go
bootstrap tool. 
# Building compilers and Go bootstrap tool.
C:\Documents and Settings\Pietro Gagliardi\go\src>set buildall=-a 
C:\Documents and Settings\Pietro Gagliardi\go\src>if x == x--no-clean set buildall= 
C:\Documents and Settings\Pietro Gagliardi\go\src>.\cmd\dist\dist bootstrap -a -v 
C:\Documents and Settings\Pietro Gagliardi\go\src>if errorlevel 1 goto fail 
C:\Documents and Settings\Pietro Gagliardi\go\src>move .\cmd\dist\dist.exe "\dist.exe" 
C:\Documents and Settings\Pietro Gagliardi\go\src>"\go_bootstrap" clean -i std 
'"\go_bootstrap"' is not recognized as an internal or external command,
operable program or batch file.
C:\Documents and Settings\Pietro Gagliardi\go\src>echo.
== was unexpected at this time.
C:\Documents and Settings\Pietro Gagliardi\go\src>if not  ==  goto localbuild
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 3, 2014

Comment 3:

If you run
"C:\Documents and Settings\Pietro Gagliardi\go\src\cmd\dist\dist" env -wp
command. Does it print anything?
If you move go directory from "C:\Documents and Settings\Pietro Gagliardi\" and into
c:\, does it help? Maybe it is something to do with spaces in "C:\Documents and
Settings\Pietro Gagliardi\".
Unfortunately I don't have w2k anymore to try all that. But you can debug it yourself -
it is cmd/dist command - small C program.
Alex
@davecheney
Copy link
Contributor

@davecheney davecheney commented Feb 3, 2014

Comment 4:

@Pietro please follow Alex's advice and try building from a short path prefix. Windows
builds are known to fail if the combined path prefix is longer than 256 characters and
this is easy to do due to the way the go tool constructs temporary paths.

Status changed to WaitingForReply.

@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 6, 2014

Comment 5:

Same issue.
Where exactly would %GOTOOLDIR% be set?
===
C:\go\src>rem @echo off 
C:\go\src>if x == x--no-local goto nolocal 
C:\go\src>if x == x--no-local goto nolocal 
C:\go\src>setlocal
C:\go\src>set GOBUILDFAIL=0 
C:\go\src>if exist make.bat goto ok 
C:\go\src>del /F ".\pkg\runtime\runtime_defs.go"  2>NUL 
C:\go\src>cd .. 
C:\go>set GOROOT=C:\go 
C:\go>cd src 
C:\go\src>if "x" == "x" set GOROOT_FINAL=C:\go 
C:\go\src>set DEFGOROOT=-DGOROOT_FINAL="\"C:\\go\"" 
C:\go\src>echo # Building C bootstrap tool. 
# Building C bootstrap tool.
C:\go\src>echo cmd/dist 
cmd/dist
C:\go\src>if not exist ..\bin\tool mkdir ..\bin\tool 
C:\go\src>gcc -O2 -Wall -Werror -o cmd/dist/dist.exe -Icmd/dist
-DGOROOT_FINAL="\"C:\\go\"" cmd/dist/buf.c cmd/dist/build.c cmd/dist/buildgc.c
cmd/dist/buildgo.c cmd/dist/buildruntime.c cmd/dist/goc2c.c cmd/dist/main.c
cmd/dist/windows.c cmd/dist/arm.c 
C:\go\src>if errorlevel 1 goto fail 
C:\go\src>.\cmd\dist\dist env -wp  1>env.bat 
C:\go\src>if errorlevel 1 goto fail 
C:\go\src>call env.bat 
C:\go\src>del env.bat 
C:\go\src>echo.
C:\go\src>if x == x--dist-tool goto copydist 
C:\go\src>if x == x--dist-tool goto copydist 
C:\go\src>echo # Building compilers and Go bootstrap tool. 
# Building compilers and Go bootstrap tool.
C:\go\src>set buildall=-a 
C:\go\src>if x == x--no-clean set buildall= 
C:\go\src>.\cmd\dist\dist bootstrap -a -v 
C:\go\src>if errorlevel 1 goto fail 
C:\go\src>move .\cmd\dist\dist.exe "\dist.exe" 
C:\go\src>"\go_bootstrap" clean -i std 
'"\go_bootstrap"' is not recognized as an internal or external command,
operable program or batch file.
C:\go\src>echo.
== was unexpected at this time.
C:\go\src>if not  ==  goto localbuild
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 7, 2014

Comment 6:

@pietro10,
> ... Where exactly would %GOTOOLDIR% be set?
> C:\go\src>.\cmd\dist\dist env -wp  1>env.bat 
this command (.\cmd\dist\dist env -wp) outputs bunch of lines in the form of
set CC=gcc
and similar. GOTOOLDIR is suppose to be amongst them. These suppose to get written to a
temp env.bat batch file.
> C:\go\src>call env.bat 
Here env.bat is executed, but I don't see it do anything. Is it empty? If yes, then why?
Does ".\cmd\dist\dist env -wp" actually outputs anything? If not, why? You have the
source, just put print statements where needed to answer all these questions. Until you
discover the reason.
Thank you.
Alex
@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 7, 2014

Comment 7:

That was the intention; I just needed to know where to start ;)
env.bat is empty; so is the output of .\cmd\dist\dist env -wp.
In fact, it appears dist.exe does nothing: invalid input like dist.exe qwerty does
nothing at all! It appears the xprintf() family of functions are not actually printing
anything. Investigating that furhter reveals that
    n = vsnprintf(NULL, 0, fmt, arg);
    p = xmalloc(n+1);
    vsnprintf(p, n+1, fmt, arg);
is what's not working: p is just set to a zero-length string in every case. I figured
the problem was that arg was being reused without being reset, so I tried that, and
presto - output!
The solution is that everywhere these three lines exist, va_end and va_start have to be
called again. I don't know if this will introduce any bugs on other systems, though...
How would I submit a patch to fix this (if you so wish)? Thanks again!
@minux
Copy link
Member

@minux minux commented Feb 7, 2014

Comment 8:

please follow steps outlined in golang.org/doc/contribute.html to submit a CL.
Thanks!
@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 7, 2014

Comment 9:

Before I submit a code review, just wondering: is there any reason xprint() and
errprint() call Windows API I/O functions instead of stdio functions? This is
nonstandard use of stdarg.h, after all...
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 7, 2014

Comment 10:

I don't know. rsc wrote the program. It is rarely changed. I suspect he used Go standard
packages as starting point, and we don't have access to any C libs. I suggest you fix
the bug, but keep the code similar. Thank you.
Alex
@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 8, 2014

Comment 11:

Hm... I think I may have fooled myself earlier with leftover code from the experiments:
adding the va_end/va_start pair now does not fix the problem. I tried using va_copy, but
that either didn't help or occasionally produced some corrupt bytes as output.
Then I figured out the /actual problem/:
    n = vsnprintf(NULL, 0, fmt, arg);
was returning -1. It appears this expected behavior of vsnprintf is undocumented (at
least according to http://msdn.microsoft.com/en-us/library/1kt27hek%28v=vs.80%29.aspx).
I'm guessing, then, that this trick (which looks to be rather common) does not work on
Windows 2000 (I doubt it's MinGW; I'm using the exact same build on Windows XP with no
issues). (I also checked: while glibc says it will return the number of remaining bytes
if a truncated write occurs, Microsoft says it will return -1.)
Honestly? I think this should just be fixed by switching to printf/fprintf instead of
going through the Windows API, but... I know from testing that printf/fprintf works fine.
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 8, 2014

Comment 12:

I am not sure what you proposal is, but, please, send a CL and someone will decide if it
is acceptable. Thank you.
Alex

Status changed to Accepted.

@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Feb 18, 2014

Comment 13:

Late but finally submitted: https://golang.org/cl/65280043
@rsc
Copy link
Contributor

@rsc rsc commented Mar 3, 2014

Comment 14:

Waiting for pietro10@mac.com to complete a CLA.

Labels changed: added release-go1.3.

Status changed to Started.

@rsc
Copy link
Contributor

@rsc rsc commented Mar 5, 2014

Comment 15:

This issue was closed by revision 3f1374f.

Status changed to Fixed.

@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Mar 5, 2014

Comment 16:

Late apology for the missed CLA; missed that part, noted for next time.
@andlabs
Copy link
Contributor Author

@andlabs andlabs commented Mar 6, 2014

Comment 17:

(update: CLA signed)
@andlabs andlabs added fixed labels Mar 6, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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