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

Standalone toolchain batch scripts broken on Windows #616

Closed
rcdailey opened this issue Jan 11, 2018 · 15 comments
Closed

Standalone toolchain batch scripts broken on Windows #616

rcdailey opened this issue Jan 11, 2018 · 15 comments
Assignees
Milestone

Comments

@rcdailey
Copy link

rcdailey commented Jan 11, 2018

So I'm trying to build boost using a standalone toolchain. I created the standalone toolchain from NDK r16b and then invoked my boost build like so:

set PATH="C:\path\to\standalone";%PATH%
b2 -j1 --reconfigure target-os=android toolset=clang threading=multi threadapi=pthread link=static runtime-link=shared address-model=32

At some point, boost starts compiling and invokes clang like so:

"clang++" -c -x c++ -O3 -Wno-inline -Wall  -DBOOST_ALL_NO_LIB=1 -DBOOST_DATE_TIME_STATIC_LINK -DDATE_TIME_INLINE -DNDEBUG -I"." -o "bin.v2\libs\date_time\build\clang-gnu-linux\release\address-model-32\link-static\target-os-android\threadapi-pthread\threading-multi\gregorian\greg_weekday.o" "libs\date_time\src\gregorian\greg_weekday.cpp"

This causes a failure because %0 in C:\path\to\standalone\bin\clang++.cmd resolves to "clang++" for some reason. To make sure, I added a log statement in clang++.cmd:

@echo off
if "%1" == "-cc1" goto :L
echo PATH TO ZERO: %0 :::: %1
%~dp0\clang50++.exe -target armv7a-none-linux-androideabi --sysroot %~dp0\..\sysroot -D__ANDROID_API__=15 %*
if ERRORLEVEL 1 exit /b 1
goto :done
:L
rem target/triple already spelled out.
%~dp0\clang50++.exe %*
if ERRORLEVEL 1 exit /b 1
:done

And I got this as output:

PATH TO ZERO: "clang++" :::: -c

Because the path to %0 is incorrect, %~dp0 ends up resolving the path to clang50++.exe as E:\code\_external\boost_1_66_0\\clang50++.exe. Note that E:\code\_external\boost_1_66_0\ happens to be my working directory, same thing you'd get from %CD%. Not sure what Windows is doing here.

@DanAlbert DanAlbert self-assigned this Jan 11, 2018
@DanAlbert DanAlbert added this to the r17 milestone Jan 11, 2018
@DanAlbert
Copy link
Member

Sounds like something worth fixing for r17.

@rprichard
Copy link
Collaborator

I suggest replacing %~dp0\ with %~dp0, because %~dp0 already ends in a backslash.

AFAICT, when I run a batch file using the PATH, %0 has only the base name (e.g. clang), but %~dp0 still has the absolute path to the directory containing the batch file.

I did see something odd in the bug report, though -- the PATH setting uses quotes:

set PATH="C:\path\to\standalone";%PATH%

(Maybe the reporter isn't actually using quotes, just like they're not literally using a C:\path\to\standalone directory.)

In cmd syntax, these quotes actually end up in the PATH variable, and they could break some programs. For example, they break C:\Windows\System32\where.exe:

C:\src>set PATH=C:\Windows\System32;C:\Windows

C:\src>where notepad
C:\Windows\System32\notepad.exe
C:\Windows\notepad.exe

C:\src>where where
C:\Windows\System32\where.exe

C:\src>set PATH="C:\Windows\System32";C:\Windows

C:\src>where notepad
C:\Windows\notepad.exe

C:\src>where where
INFO: Could not find files for the given pattern(s).

@rprichard
Copy link
Collaborator

I think the problem here is that %~dp0 is wrong when the batch file is invoked with quotes ("clang"), but it's fine when invoked without quotes (clang).

@rcdailey
Copy link
Author

So I corrected the quotes in my path:

$ echo %PATH%
E:\code\_standalone_toolchains\api15_arm_llvmstl\bin;C:\Program Files\ConEmu\ConEmu\Scripts;C:\Program Files\ConEmu;C:\Program Files\ConEmu\ConEmu;E:\Python36-32\Scripts\;E:\Python36-32\;C:\ProgramData\Oracle\Java\javapath;C:\Windows\System32;C:\windows;C:\windows\System32\Wbem;C:\windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Beyond Compare 4;E:\libxml2\bin;E:\ant\bin;E:\android\apktool;E:\android\ndk;E:\android\sdk-for-studio\tools;E:\android\sdk-for-studio\tools\bin;E:\android\sdk-for-studio\platform-tools;E:\code\ziosk-scripts;E:\tools\ninja;E:\Ruby22\bin;E:\Program Files\CMake\bin;C:\Program Files (x86)\Notepad++;C:\Program Files\doxygen\bin;C:\Program Files\TortoiseSVN\bin;E:\Git\cmd;E:\Git\mingw64\bin;E:\Git\usr\bin;C:\Users\robert\AppData\Local\Microsoft\WindowsApps;C:\Users\robert\AppData\Local\atom\bin;E:\Program Files\Microsoft VS Code\bin

And I still get this:

  "clang++" -c -x c++ -O3 -Wno-inline -Wall  -DBOOST_ALL_NO_LIB=1 -DBOOST_DATE_TIME_STATIC_LINK -DDATE_TIME_INLINE -DNDEBUG -I"." -o "bin.v2\libs\date_time\build\clang-gnu-linux\release\address-model-32\architecture-arm\link-static\target-os-android\threadapi-pthread\threading-multi\gregorian\greg_weekday.o" "libs\date_time\src\gregorian\greg_weekday.cpp"

...failed clang-linux.compile.c++.without-pth bin.v2\libs\date_time\build\clang-gnu-linux\release\address-model-32\architecture-arm\link-static\target-os-android\threadapi-pthread\threading-multi\gregorian\greg_weekday.o...
clang-linux.compile.c++.without-pth bin.v2\libs\date_time\build\clang-gnu-linux\release\address-model-32\architecture-arm\link-static\target-os-android\threadapi-pthread\threading-multi\gregorian\date_generators.o
PATH TO ZERO: "clang++" :::: -c
'E:\code\_external\boost_1_66_0\\clang50++.exe' is not recognized as an internal or external command,
operable program or batch file.

@rprichard
Copy link
Collaborator

If you could get Boost to leave off the quotes around clang++, I think it would work. e.g. Change this:

  "clang++" -c -x c++ -O3 -Wno-inline -Wall

to:

  clang++ -c -x c++ -O3 -Wno-inline -Wall

It does seem like something we should fix.

@rcdailey
Copy link
Author

I'm honestly not sure where in the bjam system of boost those quotes are inserted. It's an incredibly complex build system. It might be something that bjam.exe is doing, worst case. I don't think it's something I could easily test, unless you have an idea for me.

@rprichard rprichard assigned rprichard and unassigned DanAlbert Jan 12, 2018
@rcdailey
Copy link
Author

I left a message on the boost dev mailing list and I got this response:

AMDG

On 01/11/2018 03:31 PM, Robert Dailey via Boost wrote:

I filed an issue over at the Android NDK github page here:
#616

When I invoke b2 to build boost using the standalone toolchain
generated by the NDK, bjam seems to be wrapping the command in quotes,
like "clang++". This throws off their logic in the clang++.cmd batch
script. I was asked to test running b2 without quotes (e.g. clang++),
but I am not sure how to do this.

Could someone help me understand if this is a code change to b2?

Yes it's a change. It was done in order to handle
the case where the path to clang++ contains spaces.

boostorg/build@c442d64

Or if
there's a jam script somewhere I can modify?

tools/build/src/tools/clang-linux.jam
Search for "$(CONFIG_COMMAND)"

I'm not sure if this is a
boost issue or an NDK issue at this point. I appreciate any help from
the boost developers.

In Christ,
Steven Watanabe

So per his recommendation, I removed the quotes and now it builds fine. Honestly I'm not sure what the solution is here. Would it be possible for the NDK team to work with the Boost developers to come to an agreement on a solution here?

@rprichard
Copy link
Collaborator

Honestly I'm not sure what the solution is here. Would it be possible for the NDK team to work with the Boost developers to come to an agreement on a solution here?

I think we can fix this problem in the standalone toolchain's batch file wrappers.

@alexcohn
Copy link

alexcohn commented Jan 14, 2018

I believe your mistake is modifying %PATH%. Boost has a cleaner way to specify where your compiler is located, see http://www.boost.org/build/tutorial.html#best_practices.

You can create a user-config.jam file in your %HOME% directory, where you can write

using clang : <root>C:/path/to/standalone/bin

and so on

@rcdailey
Copy link
Author

rcdailey commented Jan 14, 2018 via email

@rprichard
Copy link
Collaborator

This is a general problem with CMD. I found two other projects that hit the problem:

It's not clear to me that either issue was fixed.

AFACT, the %~dp0 syntax is equivalent to %~d0%~p0. %~d0 outputs a drive, and %~p0 outputs a path. UNC paths are handled somehow -- possibly with a drive of \\. There are other variants -- %~n0 outputs a "name" (with no extension), and %~x0 outputs an optional extension, starting with the period.

The d and p can be reversed: %~pd0 outputs the same thing as %~dp0.

This syntax can be used with FOR loop variables and also with arguments (%1, %2, ...), but it appears to have special behavior with %0. When %0 is a filename (e.g. clang.cmd), %~dp0 will identify the directory in the PATH where the batch file was found, but if %1 is clang.cmd, %~dp1 will be the current working directory.

CMD needs to break %0 down into drive, path, name, and extension. I'm guessing this parsing code somehow repeats the PATH search for the batch file, and it doesn't always handle paths with quotes.

For example, suppose showarg0.cmd at C:\src\bin is in the PATH and has this code:

@echo [%~d0] [%~p0] [%~n0] [%~x0]

These work:

C:\>showarg0                      ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>showarg0.cmd                  ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>C:\src\bin\showarg0           ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>C:\src\bin\showarg0.cmd       ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>showarg0".cmd"                ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>showarg0."cmd"                ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>showarg0.c"md"                ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>s"ho"warg0.cmd                ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>s"ho"warg0                    ==> [C:] [\src\bin\] [showarg0] [.cmd]
C:\>"C:\src\bin\showarg0.cmd"     ==> [C:] [\src\bin\] [showarg0] [.cmd]

With the exception of the last command, CMD tends to choke on an %0 that starts with a double-quote:

C:\>"showarg0"                    ==> [C:] [\] [showarg0] []
C:\>"showarg0.cmd"                ==> [C:] [\] [showarg0] [.cmd]
C:\>"showarg0".cmd                ==> [C:] [\] [showarg0"] [.cmd]
C:\>"sho"warg0.cmd                ==> [C:] [\] [sho"warg0] [.cmd]
C:\>"sho"warg0                    ==> [C:] [\] [sho"warg0] []
C:\>"c":\src\bin\showarg0.cmd     ==> [C:] [\c":\src\bin\] [showarg0] [.cmd]
C:\>"c:"\src\bin\showarg0.cmd     ==> [C:] [\"\src\bin\] [showarg0] [.cmd]
C:\>"c:\"src\bin\showarg0.cmd     ==> [c:] [\"src\bin\] [showarg0] [.cmd]
C:\>"C:\src\bin\showarg0"         ==> [C:] [\src\bin\] [showarg0] []

CMD has another special behavior with %0 -- if %0 refers to a label in the current batch file, then the %0 path variables apparently use some normalized path, so %~dp0 works.

workaround.cmd:

@echo off
call :fix
exit /b
:fix
echo [%0] [%~d0] [%~p0] [%~n0] [%~x0]
exit /b

It always prints, [:fix] [C:] [\src\bin\] [workaround] [.cmd], regardless of how workaround.cmd was invoked.

I'm not sure this behavior is documented anywhere, and it doesn't work with Wine CMD. Wine CMD already does the right thing without the workaround, but always fails (i.e. prints the current working directory) with the workaround. It's probably OK to break Wine CMD. (Maybe it's easy to keep it working, though, by checking the ver command, which prints Wine CMD Version 5.1.2600 (1.6.2) for me. I verified that ver is a CMD built-in, so it's not spawning another process.)

@DanAlbert
Copy link
Member

Working with Windows at the cost of breaking with Wine sounds like the right trade-off to me.

@rprichard
Copy link
Collaborator

Yeah, it'll be easy to fix if we want to later.

@rprichard
Copy link
Collaborator

Fixed here: https://android-review.googlesource.com/c/platform/ndk/+/589724.

The fix should be in r17.

@rprichard
Copy link
Collaborator

I filed a Wine bug: https://bugs.winehq.org/show_bug.cgi?id=44369.

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
It will allow the batch files to be executed using quotes and found using
the PATH:

    "clang"
    "clang.cmd"
    "clang".cmd
    "<triple>-clang"

Regarding the local/temp variable:
 * I'm hoping the _ prefix makes the variable unique.
 * setlocal ensures that the batch file won't affect the caller's
   environment.
 * `set "_BIN_DIR="` prevents the variable from being inherited in the
   clang subprocess. The quotes are necessary because
   `set _BIN_DIR= && ...` would set _BIN_DIR to a single space.

Bug: android/ndk#616
Test: manual
Test: python.exe run_tests.py --rebuild ...
Change-Id: Iff37183de269931b96a6a8842578dd50d366ae31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants