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

Error in test\download_test_data.vcxproj custom build step when compiling with Visual Studio 2019 16.7.7 msbuild on Windows 10 #2593

Closed
1 of 2 tasks
Andreas-Schniertshauer opened this issue Jan 14, 2021 · 21 comments · May be fixed by #3765
Labels
platform: visual studio related to MSVC state: help needed the issue needs help to proceed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated state: waiting for PR

Comments

@Andreas-Schniertshauer
Copy link

I get the following error in the CustomBuild step of test\download_test_data.vcxproj when compiling with Visual Studio 2019 16.7.7 Professional msbuild on Windows 10 2004 Professional:

CustomBuild:
Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
Der Befehl "test" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
Building Custom Rule ./test/CMakeLists.txt

The Command "test" is not found, because it does not exist on Windows.
"test -d json_test_data" is found in the CustomBuild step of the test\download_test_data.vcxproj project.

The problem is located in download_test_data.cmake "COMMAND test -d json_test_data || ...",
here "COMMAND if not exist json_test_data ..." should be used on Windows.

Please describe the steps to reproduce the issue.

  1. Download json-3.9.1.zip and unzip
  2. Open a Developer command Prompt v16.7.7
  3. cd json-3.9.1
  4. Create directory build and cd build
  5. cmake ..\ -D JSON_Install:BOOL=OFF -D JSON_ImplicitConversions:BOOL=OFF -D JSON_BuildTests:BOOL=ON
  6. msbuild nlohmann_json.sln
  7. msbuild nlohmann_json.sln

What is the expected behavior?

No error.

And what is the actual behavior instead?

Step 6:
CustomBuild:
Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
Der Befehl "test" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
Building Custom Rule ./test/CMakeLists.txt

Step 7:
CustomBuild:
Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
Der Befehl "test" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
fatal: destination path 'json_test_data' already exists and is not an empty directory.
c:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(234,5): error MSB6006: "cmd.exe" wurde mit dem Code 128 beendet. [build\test\download_test_data.vcxproj]

Which compiler and operating system are you using?

  • Compiler: Visual Studio 2019 16.7.7 Professional
  • Operating system: Windows 10 2004 Pro German

Which version of the library did you use?

  • [x ] latest release version 3.9.1

If you experience a compilation error: can you compile and run the unit tests?

  • yes
  • no - please copy/paste the error message below
ctest --output-on-failure

did not work, there is mising a -C in the command

ctest -C Release --output-on-failure

would do the job, but an error is reported about json_test_data.
After deleting the json_test_data folder and executing

ctest -C Release --output-on-failure

it works.
Is also works without deleting the folder when "if exist" is used instead of "test" as reported on top of this post.

Total Test time (real) = 381.65 sec

The following tests FAILED:
51 - cmake_import_configure (Failed)
52 - cmake_import_build (Not Run)
53 - cmake_import_minver_configure (Failed)
54 - cmake_import_minver_build (Not Run)
57 - cmake_fetch_content_configure (Failed)
58 - cmake_fetch_content_build (Not Run)
Errors while running CTest

@nlohmann
Copy link
Owner

This is strange, because this job works without problems: https://github.com/nlohmann/json/runs/1679595437?check_suite_focus=true (Windows Server 2019)

@Andreas-Schniertshauer
Copy link
Author

I tested the commands that are executed in the job locally and you are correct, there is no error, because in the build step

cmake --build build --parallel 10

the folder json_test_data is not created, because the project json_unit (I can't find the .exe) is not build that references download_test_data. download_test_data is referenced only by json_unit and because json_unit is not build download_test_data is also not build. If you look into the download_test_data.vcxproj with a text editor you can see that there are various test -d json_test_data || calls in the CustomBuild step sections. If you build locally by opening the nlohmann_json.sln solution on your local computer and do a Build\Build Solution you get the error "Der Befehl "test" ist entweder falsch geschrieben oder konnte nicht gefunden werden."
cmake calls the ALL_BUILD target but the ALL_BUILD project does not reference the json_unit, so json_unit is not build, and you don't get the error when executing the job.
I get the error because I call msbuild nlohmann_json.sln ... and that is equal to opening the nlohmann_json.sln solution and do a Build\Build Solution in the GUI that builds every project in the solution. If you would build with msbuild in your job you would also get the error.

@Andreas-Schniertshauer
Copy link
Author

The problem is line 13 of cmake\download_test_data.cmake:

COMMAND test -d json_test_data || ${GIT_EXECUTABLE} clone -c advice.detachedHead=false --branch v${JSON_TEST_DATA_VERSION} ${JSON_TEST_DATA_URL}.git --quiet --depth 1

Here for Visual Studio Command Prompt on Windows it must be:

COMMAND if not exist json_test_data ${GIT_EXECUTABLE} clone -c advice.detachedHead=false --branch v${JSON_TEST_DATA_VERSION} ${JSON_TEST_DATA_URL}.git --quiet --depth 1

because there is no test in the Windows Command Shell and also no test in the Visual Studio Command Prompt shell.

@nlohmann
Copy link
Owner

How would CMake be able to detect Windows Command Shell?

@Andreas-Schniertshauer
Copy link
Author

How would CMake be able to detect Windows Command Shell?

It could be done same as with the uname and ver commands a few lines down:

find_program(TEST_COMMAND test)
if (TEST_COMMAND)
    add_custom_target(download_test_data
        COMMAND test -d json_test_data || ${GIT_EXECUTABLE} clone -c advice.detachedHead=false --branch v${JSON_TEST_DATA_VERSION} ${JSON_TEST_DATA_URL}.git --quiet --depth 1
        COMMENT "Downloading test data from ${JSON_TEST_DATA_URL} (v${JSON_TEST_DATA_VERSION})"
        WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
    )
else()
    add_custom_target(download_test_data
        COMMAND if exist json_test_data ${GIT_EXECUTABLE} clone -c advice.detachedHead=false --branch v${JSON_TEST_DATA_VERSION} ${JSON_TEST_DATA_URL}.git --quiet --depth 1
        COMMENT "Downloading test data from ${JSON_TEST_DATA_URL} (v${JSON_TEST_DATA_VERSION})"
        WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
    )
endif()

nlohmann added a commit that referenced this issue Jan 26, 2021
@nlohmann
Copy link
Owner

I think I found a better way. @Andreas-Schniertshauer can you please try the fix of b2244a7 in branch https://github.com/nlohmann/json/tree/issue2593?

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: help needed the issue needs help to proceed labels Jan 26, 2021
@Andreas-Schniertshauer
Copy link
Author

It only works with the first msbuild nlohmann_json.sln build, a second build results in an error:

CustomBuild:
...
Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
fatal: destination path 'json_test_data' already exists and is not an empty directory.

Also when working in the Visual Studio GUI e.g. debugging test-bor results in the same error.
The problem is this call

git clone -c advice.detachedHead=false --branch v3.0.0 https://github.com/nlohmann/json_test_data.git --quiet --depth 1

If you call this twice you also get the error because according to the git clone documentation: "... Cloning into an existing directory is only allowed if the directory is empty."

@nlohmann
Copy link
Owner

This is strange, and I cannot reproduce this. I understand that once git clone is called the second time, we have a problem. But this should be avoided as the command should only be executed if ${CMAKE_BINARY_DIR}/json_test_data is not present.

Can you please share the exact steps that lead to the problem (in particular, the CMake commands and in which paths you are executing what).

@Andreas-Schniertshauer
Copy link
Author

I switched to the development branch #2593 and changed into the json directory, opened a "Developer Command Prompt" and executed:

md build && cd build
cmake ..\ -D JSON_Install:BOOL=OFF -D JSON_ImplicitConversions:BOOL=OFF -D JSON_BuildTests:BOOL=ON
msbuild nlohmann_json.sln
msbuild nlohmann_json.sln

I looked into the json\build\test\download_test_data.vcxproj project file and there in the section CustomBuild in Command is the code that causes the problem:

setlocal
cd json\build
if %errorlevel% neq 0 goto :cmEnd
T:
if %errorlevel% neq 0 goto :cmEnd
"C:\Program Files\Git\bin\git.exe" clone -c advice.detachedHead=false --branch v3.0.0 https://github.com/nlohmann/json_test_data.git --quiet --depth 1
if %errorlevel% neq 0 goto :cmEnd
:cmEnd
endlocal & call :cmErrorLevel %errorlevel% & goto :cmDone
:cmErrorLevel
exit /b %1
:cmDone
if %errorlevel% neq 0 goto :VCEnd

git clone is executed every time the project is build, no check for the json_test_data folder any more.

@Andreas-Schniertshauer
Copy link
Author

Maybe you want to use cmake ExternalProject_Add that is executed when cmake is called, see https://cmake.org/cmake/help/latest/module/ExternalProject.html and not on every solution /project build.

@nlohmann
Copy link
Owner

Hm. I have to check the code generated for Makefiles or Ninja - but it seems the generated code is wrong.

@Andreas-Schniertshauer
Copy link
Author

Andreas-Schniertshauer commented Jan 27, 2021

build.ninja shows:

#############################################
# Custom command for json_test_data

build json_test_data: CUSTOM_COMMAND
  COMMAND = cmd.exe /C "cd /D json\build && "C:\Program Files\Git\bin\git.exe" clone -c advice.detachedHead=false --branch v3.0.0 https://github.com/nlohmann/json_test_data.git --quiet --depth 1"
  DESC = Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
  restat = 1

@nlohmann
Copy link
Owner

Yes, that's the download step. There should also be a target download_test_data. By the way: does the ninja build work for you?

@Andreas-Schniertshauer
Copy link
Author

cmake -GNinja ..\ -D JSON_Install:BOOL=OFF -D JSON_ImplicitConversions:BOOL=OFF -D JSON_BuildTests:BOOL=ON
ninja
ninja json_test_data
[1/1] Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
ninja json_test_data
ninja: no work to do.
ninja test

did work partially:

93% tests passed, 4 tests failed out of 60
Label Time Summary:
all                 = 1626.68 sec*proc (49 tests)
not_reproducible    =  12.37 sec*proc (10 tests)

Total Test time (real) = 1639.47 sec

The following tests FAILED:
         51 - cmake_import_configure (Failed)
         52 - cmake_import_build (Not Run)
         53 - cmake_import_minver_configure (Failed)
         54 - cmake_import_minver_build (Not Run)
Errors while running CTest
FAILED: CMakeFiles/test.util

@Andreas-Schniertshauer
Copy link
Author

51/60 Test #51: cmake_import_configure .......................***Failed    1.50 sec
      Start 52: cmake_import_build
Failed test dependencies: cmake_import_configure
52/60 Test #52: cmake_import_build ...........................***Not Run   0.00 sec
      Start 53: cmake_import_minver_configure
53/60 Test #53: cmake_import_minver_configure ................***Failed    1.09 sec
      Start 54: cmake_import_minver_build
Failed test dependencies: cmake_import_minver_configure
54/60 Test #54: cmake_import_minver_build ....................***Not Run   0.00 sec

@nlohmann
Copy link
Owner

Did you test this with branch issue2593? In that branch, the dependency for the tests is download_test_data, so you should actually test ninja download_test_data.

Anyway, can you please share the output of the failed test? (Run ctest --output-on-failure)

@Andreas-Schniertshauer
Copy link
Author

Yes, issue2593, now I used:

cmake -GNinja ..\ -D JSON_Install:BOOL=OFF -D JSON_ImplicitConversions:BOOL=OFF -D JSON_BuildTests:BOOL=ON
ninja
ninja download_test_data
[1/1] Downloading test data from https://github.com/nlohmann/json_test_data (v3.0.0)
ninja download_test_data
ninja test

Same result as with ninja json_test_data I think you can use both because of phony.

Here is the result of ctest -C Release --output-on-failure I stripped the top because they passed:

...
51/60 Test #51: cmake_import_configure .......................***Failed    0.06 sec
CMake Error at json/build/nlohmann_jsonConfig.cmake:6 (include):
  include could not find load file:

    json/build/nlohmann_jsonTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:5 (find_package)


-- Configuring incomplete, errors occurred!
See also "json/build/test/cmake_import/CMakeFiles/CMakeOutput.log".

      Start 52: cmake_import_build
Failed test dependencies: cmake_import_configure
52/60 Test #52: cmake_import_build ...........................***Not Run   0.00 sec
      Start 53: cmake_import_minver_configure
53/60 Test #53: cmake_import_minver_configure ................***Failed    0.05 sec
CMake Error at json/build/nlohmann_jsonConfig.cmake:6 (include):
  include could not find load file:

    json/build/nlohmann_jsonTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:5 (find_package)


-- Configuring incomplete, errors occurred!
See also "json/build/test/cmake_import_minver/CMakeFiles/CMakeOutput.log".

      Start 54: cmake_import_minver_build
Failed test dependencies: cmake_import_minver_configure
54/60 Test #54: cmake_import_minver_build ....................***Not Run   0.00 sec
      Start 55: cmake_add_subdirectory_configure
55/60 Test #55: cmake_add_subdirectory_configure .............   Passed    0.09 sec
      Start 56: cmake_add_subdirectory_build
56/60 Test #56: cmake_add_subdirectory_build .................   Passed    0.03 sec
      Start 57: cmake_fetch_content_configure
57/60 Test #57: cmake_fetch_content_configure ................   Passed    1.39 sec
      Start 58: cmake_fetch_content_build
58/60 Test #58: cmake_fetch_content_build ....................   Passed    0.03 sec
      Start 59: cmake_target_include_directories_configure
59/60 Test #59: cmake_target_include_directories_configure ...   Passed    0.06 sec
      Start 60: cmake_target_include_directories_build
60/60 Test #60: cmake_target_include_directories_build .......   Passed    0.03 sec

93% tests passed, 4 tests failed out of 60

Label Time Summary:
all                 = 1697.58 sec*proc (49 tests)
not_reproducible    =   1.73 sec*proc (10 tests)

Total Test time (real) = 1699.82 sec

The following tests FAILED:
         51 - cmake_import_configure (Failed)
         52 - cmake_import_build (Not Run)
         53 - cmake_import_minver_configure (Failed)
         54 - cmake_import_minver_build (Not Run)
Errors while running CTest

@nlohmann
Copy link
Owner

I currently see no way to fix this issue. The Windows builds run fine in the CI, and I have no way to reproduce the issue locally as I am not using MSVC. Happy to see PRs though.

@nlohmann nlohmann added state: help needed the issue needs help to proceed state: waiting for PR and removed kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Jul 16, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jan 9, 2022
@falbrechtskirchinger
Copy link
Contributor

One could always rewrite this as a separate CMake script and run that via ${CMAKE_COMMAND} -P <script.cmake>.

PR welcome, I'm sure.

@nlohmann
Copy link
Owner

Yes, PR welcome. But it was time to clean up a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: visual studio related to MSVC state: help needed the issue needs help to proceed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated state: waiting for PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants