From 4dd769b9082d0ebc7edede6370e9a2f35102299c Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 09:58:58 -0700 Subject: [PATCH 01/34] add format target to cmake --- CMakeLists.txt | 6 +++++ format/CMakeLists.txt | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 format/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index a286795e67b..6604f15fbd4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,3 +86,9 @@ if(BUILD_TESTING) enable_testing() add_subdirectory(tests) endif() + +add_subdirectory(format) +if(CLANG_FORMAT_TARGETS) + add_custom_target(format) + add_dependencies(format ${CLANG_FORMAT_TARGETS}) +endif() diff --git a/format/CMakeLists.txt b/format/CMakeLists.txt new file mode 100644 index 00000000000..6863fff014c --- /dev/null +++ b/format/CMakeLists.txt @@ -0,0 +1,61 @@ +set(did_search ON) +if(NOT DEFINED CLANG_FORMAT) + message(STATUS "Searching for VS clang-format") + set(did_search OFF) +endif() + +find_program(CLANG_FORMAT + NAMES clang-format + PATHS "C:/Program Files/Microsoft Visual Studio/2022/Preview/VC/Tools/Llvm/bin" + DOC "The clang-format program to use" + NO_DEFAULT_PATH + NO_CMAKE_PATH + NO_CMAKE_ENVIRONMENT_PATH + NO_SYSTEM_ENVIRONMENT_PATH + NO_CMAKE_SYSTEM_PATH +) +if(CLANG_FORMAT) + if(did_search) + message(STATUS "Searching for VS clang-format - found") + endif() + + file(GLOB_RECURSE maybe-clang-format-files + "${PROJECT_SOURCE_DIR}/stl/inc/*" + "${PROJECT_SOURCE_DIR}/stl/src/*" + "${PROJECT_SOURCE_DIR}/tests/*" + "${PROJECT_SOURCE_DIR}/tools/*" + ) + set(clang-format-files "") + foreach(maybe-file IN LISTS maybe-clang-format-files) + cmake_path(GET maybe-file EXTENSION LAST_ONLY extension) + if(extension MATCHES [[^(|\.cpp|\.h|\.hpp)$]]) + list(APPEND clang-format-files "${maybe-file}") + endif() + endforeach() + + if(NOT clang-format-files) + message(WARNING "Could not find any files to clang-format!") + endif() + + set(clang-format-targets "") + foreach(file IN LISTS clang-format-files) + cmake_path(RELATIVE_PATH file + BASE_DIRECTORY "${PROJECT_SOURCE_DIR}" + OUTPUT_VARIABLE relative-file + ) + string(REPLACE "/" "_" relative-file "${relative-file}") + set(target_name "clang-format.${relative-file}") + add_custom_target("${target_name}" + COMMAND "${CLANG_FORMAT}" -style=file -i "${file}" + WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}" + ) + list(APPEND clang-format-targets "${target_name}") + endforeach() + + set(CLANG_FORMAT_TARGETS "${clang-format-targets}" PARENT_SCOPE) +else() + if(did_search) + message(WARNING "Searching for VS clang-format - not found.") + endif() + set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) +endif() From 507e12b19012b68989a0bc05d77bf2dcaa2bd01e Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 10:04:28 -0700 Subject: [PATCH 02/34] remove parallelize --- azure-devops/enforce-clang-format.cmd | 14 -- azure-pipelines.yml | 8 +- tools/CMakeLists.txt | 1 - tools/parallelize/CMakeLists.txt | 5 - tools/parallelize/parallelize.cpp | 243 -------------------------- 5 files changed, 2 insertions(+), 269 deletions(-) delete mode 100644 azure-devops/enforce-clang-format.cmd delete mode 100644 tools/parallelize/CMakeLists.txt delete mode 100644 tools/parallelize/parallelize.cpp diff --git a/azure-devops/enforce-clang-format.cmd b/azure-devops/enforce-clang-format.cmd deleted file mode 100644 index f2d666a6632..00000000000 --- a/azure-devops/enforce-clang-format.cmd +++ /dev/null @@ -1,14 +0,0 @@ -:: Copyright (c) Microsoft Corporation. -:: SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ --host_arch=amd64 -arch=amd64 -no_logo -"%1" "clang-format.exe -style=file -i" ^ -stl/inc ^ -stl/src ^ -tests ^ -tools -@echo If your build fails here, you need to format the following files with: -@clang-format.exe --version -@git status --porcelain stl tests tools 1>&2 -@echo clang-format will produce the following diff: -@git diff stl tests tools 1>&2 diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 979f8ed76c6..3c4773ec1ac 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -39,15 +39,11 @@ stages: cmake --build $(buildOutputLocation) displayName: 'Build Support Tools' env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - - task: BatchScript@1 + - script: | + cmake --build $(buildOutputLocation) --target format displayName: 'Enforce clang-format' timeoutInMinutes: 60 condition: succeededOrFailed() - inputs: - filename: 'azure-devops/enforce-clang-format.cmd' - failOnStandardError: true - arguments: '$(buildOutputLocation)/parallelize/parallelize.exe' - env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - task: BatchScript@1 displayName: 'Validate Files' timeoutInMinutes: 2 diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 56d80e3d367..1b271afbb2c 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -14,5 +14,4 @@ add_compile_options(/W4 /WX $<$>:/Zi> /permissive-) include_directories(inc) add_subdirectory(jobify) -add_subdirectory(parallelize) add_subdirectory(validate) diff --git a/tools/parallelize/CMakeLists.txt b/tools/parallelize/CMakeLists.txt deleted file mode 100644 index 348124b4564..00000000000 --- a/tools/parallelize/CMakeLists.txt +++ /dev/null @@ -1,5 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -add_executable(parallelize parallelize.cpp ../inc/stljobs.h) -target_link_libraries(parallelize Userenv.lib) diff --git a/tools/parallelize/parallelize.cpp b/tools/parallelize/parallelize.cpp deleted file mode 100644 index 8fa82216ce1..00000000000 --- a/tools/parallelize/parallelize.cpp +++ /dev/null @@ -1,243 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -#include "stljobs.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -class tp_wait { -public: - explicit tp_wait(PTP_WAIT_CALLBACK pfnwa, PVOID pv, PTP_CALLBACK_ENVIRON pcbe) - : wait(CreateThreadpoolWait(pfnwa, pv, pcbe)) { - if (!wait) { - api_failure("CreateThreadpoolWait"); - } - } - - tp_wait(const tp_wait&) = delete; - tp_wait& operator=(const tp_wait&) = delete; - - void wait_for(const HANDLE waitOn) noexcept { - SetThreadpoolWait(wait, waitOn, nullptr); - } - - ~tp_wait() { - WaitForThreadpoolWaitCallbacks(wait, TRUE); - CloseThreadpoolWait(wait); - } - -private: - PTP_WAIT wait{}; -}; - -class parallelizer { -public: - parallelizer() : subprocesses(std::make_unique(availableConcurrency)) { - for (size_t idx = 0; idx < availableConcurrency; ++idx) { - subprocesses[idx].parent = this; - } - } - - void add_command(std::wstring&& toRun) { - std::lock_guard lck(mtx); - const auto oldCommandsSize = commands.size(); - commands.emplace_back(std::move(toRun), std::nullopt); - const auto localConcurrency = runningConcurrency.load(std::memory_order_relaxed); - if (availableConcurrency == localConcurrency) { - return; - } - - runningConcurrency.store(localConcurrency + 1, std::memory_order_relaxed); - size_t scheduledOn = 0; - while (subprocesses[scheduledOn].commandRunning != SIZE_MAX) { - ++scheduledOn; - } - - nextCommandToRun = oldCommandsSize + 1; - subprocesses[scheduledOn].schedule(oldCommandsSize); - update_display(); - } - - void wait_all() noexcept { - std::unique_lock lck{mtx}; - cv.wait(lck, [this] { return runningConcurrency.load(std::memory_order_relaxed) == 0; }); - } - - [[nodiscard]] const std::vector>>& - results() const noexcept { - assert(runningConcurrency.load(std::memory_order_relaxed) == 0); - return commands; - } - -private: - struct entry; - std::mutex mtx{}; - std::condition_variable cv{}; - std::vector>> commands{}; - size_t nextCommandToRun{0}; - size_t availableConcurrency{std::thread::hardware_concurrency()}; - std::atomic runningConcurrency{0}; - std::unique_ptr subprocesses; - - struct entry { - parallelizer* parent; - size_t commandRunning{SIZE_MAX}; // guarded by parent->mtx - subprocess_executive executive; - tp_wait tpWait{callback, this, nullptr}; - - static void __stdcall callback( - PTP_CALLBACK_INSTANCE, void* thisRaw, PTP_WAIT, [[maybe_unused]] TP_WAIT_RESULT waitDisposition) noexcept { - assert(waitDisposition == WAIT_OBJECT_0); - const auto this_ = static_cast(thisRaw); - const auto parent = this_->parent; - auto results = this_->executive.complete(); - std::lock_guard lck{parent->mtx}; - parent->commands[this_->commandRunning].second.emplace(std::move(results)); - if (parent->nextCommandToRun == parent->commands.size()) { - this_->commandRunning = SIZE_MAX; - if (parent->runningConcurrency.fetch_sub(1, std::memory_order_relaxed) == 1) { - parent->cv.notify_all(); - } - } else { - this_->schedule(parent->nextCommandToRun++); - } - - parent->update_display(); - } - - void schedule(const size_t command) { - commandRunning = command; - executive.begin_execution(nullptr, parent->commands[command].first.data(), 0, nullptr); - tpWait.wait_for(executive.get_wait_handle()); - } - }; - - void update_display() noexcept { - const auto commandsSize = commands.size(); - const auto next = nextCommandToRun; - const auto running = runningConcurrency.load(std::memory_order_relaxed); - assert(running <= next); - printf("%zu scheduled; %zu completed; %zu running\n", commandsSize, next - running, running); - } -}; - -void schedule_command(parallelizer& p, const std::wstring_view commandPrefix, const std::wstring& native) { - std::wstring toExecute; - toExecute.reserve(commandPrefix.size() + 1 + native.size()); - toExecute.assign(commandPrefix); - toExecute.push_back(L' '); - toExecute.append(native); - p.add_command(std::move(toExecute)); -} - -extern "C" int wmain(int argc, wchar_t* argv[]) { - try { - using namespace std::string_view_literals; - - static constexpr std::array accepted_extensions{ - L""sv, // extensionless headers like - L".cpp"sv, - L".h"sv, - L".hpp"sv, - }; - - static_assert(std::is_sorted(accepted_extensions.begin(), accepted_extensions.end())); - - if (argc < 3) { - puts("Usage: parallelize.exe commandPrefix pathRoot0 [... pathRootN]\n" - "The command:\n" - "commandPrefix file\n" - "will be launched in parallel for regular files under any of pathRoots, recursively."); - printf("Accepted extensions: "); - for (const auto& sv : accepted_extensions) { - printf("\"%.*ls\", ", static_cast(sv.size()), sv.data()); - } - printf("\n"); - puts("Other extensions will be skipped."); - return 1; - } - - parallelizer p; - std::wstring_view commandPrefix(argv[1]); - for (int idx = 2; idx < argc; ++idx) { - std::filesystem::path thisSpec(argv[idx]); - if (std::filesystem::is_regular_file(thisSpec)) { - schedule_command(p, commandPrefix, thisSpec.native()); - continue; - } - - for (const auto& dirEntry : std::filesystem::recursive_directory_iterator(std::move(thisSpec))) { - if (!dirEntry.is_regular_file()) { - printf("Skipping non-regular-file %ls\n", dirEntry.path().c_str()); - continue; - } - - if (!binary_search( - accepted_extensions.begin(), accepted_extensions.end(), dirEntry.path().extension().native())) { - printf("Skipping non-accepted-extension %ls\n", dirEntry.path().c_str()); - continue; - } - - schedule_command(p, commandPrefix, dirEntry.path().native()); - } - } - - p.wait_all(); - bool exitSuccess = true; - size_t vacuousCount = 0; - for (auto& command : p.results()) { - auto& result = command.second.value(); - if (result.exitCode == 0) { - if (result.output.empty()) { - ++vacuousCount; - } else { - printf("%ls produced output:\n%s\n", command.first.c_str(), result.output.c_str()); - } - } else { - exitSuccess = false; - if (result.output.empty()) { - printf("%ls exited with 0x%08lX and no output.\n", command.first.c_str(), result.exitCode); - } else { - printf("%ls exited with 0x%08lX and output:\n%s\n", command.first.c_str(), result.exitCode, - result.output.c_str()); - } - } - } - - const auto totalCommands = p.results().size(); - printf("%zu commands ran, returned 0, and produced no output", vacuousCount); - if (vacuousCount == totalCommands) { - puts("."); - } else { - printf(" (out of %zu).", totalCommands); - } - - if (exitSuccess) { - return 0; - } else { - return 1; - } - } catch (std::filesystem::filesystem_error& err) { - fputs(err.what(), stderr); - abort(); - } catch (api_exception& api) { - api.give_up(); - } -} From dd1ea10fab8dd1c2287770beb750cc91cf245669 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 10:11:49 -0700 Subject: [PATCH 03/34] remember to call vsdevcmd; add ERROR_ON_CLANG_FORMAT_NOT_FOUND --- azure-pipelines.yml | 8 ++++++++ format/CMakeLists.txt | 10 ++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 3c4773ec1ac..20ce387ddda 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -40,10 +40,18 @@ stages: displayName: 'Build Support Tools' env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - script: | + if exist "$(buildOutputLocation)" ( + rmdir /S /Q "$(buildOutputLocation)" + ) + call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ + -host_arch=amd64 -arch=amd64 -no_logo + cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release -DERROR_ON_CLANG_FORMAT_NOT_FOUND=1 ^ + -S $(Build.SourcesDirectory)\tools -B $(buildOutputLocation) cmake --build $(buildOutputLocation) --target format displayName: 'Enforce clang-format' timeoutInMinutes: 60 condition: succeededOrFailed() + env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - task: BatchScript@1 displayName: 'Validate Files' timeoutInMinutes: 2 diff --git a/format/CMakeLists.txt b/format/CMakeLists.txt index 6863fff014c..4e8bde21be2 100644 --- a/format/CMakeLists.txt +++ b/format/CMakeLists.txt @@ -4,6 +4,12 @@ if(NOT DEFINED CLANG_FORMAT) set(did_search OFF) endif() +if(ERROR_ON_CLANG_FORMAT_NOT_FOUND) + set(message_level FATAL_ERROR) +else() + set(message_level WARNING) +endif() + find_program(CLANG_FORMAT NAMES clang-format PATHS "C:/Program Files/Microsoft Visual Studio/2022/Preview/VC/Tools/Llvm/bin" @@ -34,7 +40,7 @@ if(CLANG_FORMAT) endforeach() if(NOT clang-format-files) - message(WARNING "Could not find any files to clang-format!") + message("${message_level}" "Could not find any files to clang-format!") endif() set(clang-format-targets "") @@ -55,7 +61,7 @@ if(CLANG_FORMAT) set(CLANG_FORMAT_TARGETS "${clang-format-targets}" PARENT_SCOPE) else() if(did_search) - message(WARNING "Searching for VS clang-format - not found.") + message("${message_level}" "Searching for VS clang-format - not found.") endif() set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) endif() From bb4fa6452f79e1199dca4c145f1403197a1199b1 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 12:26:49 -0700 Subject: [PATCH 04/34] add a format.diff output; should correct errors --- azure-devops/Create-PRDiff.ps1 | 19 +++++++++++++ azure-devops/validate-files.cmd | 7 ++++- azure-pipelines.yml | 47 ++++++++++++++++----------------- 3 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 azure-devops/Create-PRDiff.ps1 diff --git a/azure-devops/Create-PRDiff.ps1 b/azure-devops/Create-PRDiff.ps1 new file mode 100644 index 00000000000..a0be6bf3f32 --- /dev/null +++ b/azure-devops/Create-PRDiff.ps1 @@ -0,0 +1,19 @@ +[CmdletBinding(PositionalBinding=$False)] +Param( + [Parameter(Mandatory=$True)] + [String]$DiffFile +) + +Start-Process -FilePath 'git' -ArgumentList 'diff' ` + -NoNewWindow -Wait ` + -RedirectStandardOutput $DiffFile +if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) +{ + $msg = @( + 'The formatting of the files in the repo were not what we expected.', + 'Please access the diff from format.diff in the build artifacts,' + 'and apply the patch with `git apply`' + ) + Write-Error ($msg -join "`n") + throw +} diff --git a/azure-devops/validate-files.cmd b/azure-devops/validate-files.cmd index e2f53c10f88..1b88c9f5f1d 100644 --- a/azure-devops/validate-files.cmd +++ b/azure-devops/validate-files.cmd @@ -1,4 +1,9 @@ :: Copyright (c) Microsoft Corporation. :: SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -"%1" +call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ + -host_arch=amd64 -arch=amd64 -no_logo +cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release ^ + -S "%1\tools" -B "%TEMP%\validate-build" +cmake --build "%TEMP%\validate-build" +"%TEMP%\validate-build\validate\validate.exe" @echo If your build fails here, you need to fix the listed issues. diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 20ce387ddda..588722ef725 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -17,7 +17,8 @@ stages: timeoutInMinutes: 90 displayName: 'Validation' variables: - buildOutputLocation: 'D:\tools' + - name: DiffFile + value: '$(Build.ArtifactStagingDirectory)\format.diff' steps: - script: | if exist "$(tmpDir)" ( @@ -29,38 +30,36 @@ stages: clean: true submodules: false - script: | - if exist "$(buildOutputLocation)" ( - rmdir /S /Q "$(buildOutputLocation)" - ) - call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ - -host_arch=amd64 -arch=amd64 -no_logo - cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release ^ - -S $(Build.SourcesDirectory)\tools -B $(buildOutputLocation) - cmake --build $(buildOutputLocation) - displayName: 'Build Support Tools' - env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - - script: | - if exist "$(buildOutputLocation)" ( - rmdir /S /Q "$(buildOutputLocation)" - ) call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ - -host_arch=amd64 -arch=amd64 -no_logo + -host_arch=amd64 -arch=amd64 -no_logo cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release -DERROR_ON_CLANG_FORMAT_NOT_FOUND=1 ^ - -S $(Build.SourcesDirectory)\tools -B $(buildOutputLocation) - cmake --build $(buildOutputLocation) --target format - displayName: 'Enforce clang-format' - timeoutInMinutes: 60 + -S $(Build.SourcesDirectory) -B $(tmpDir)\format-build + cmake --build $(tmpDir)\format-build --target format + displayName: 'clang-format' + timeoutInMinutes: 5 condition: succeededOrFailed() env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - task: BatchScript@1 - displayName: 'Validate Files' - timeoutInMinutes: 2 - condition: succeededOrFailed() inputs: filename: 'azure-devops/validate-files.cmd' failOnStandardError: true - arguments: '$(buildOutputLocation)/validate/validate.exe' + arguments: '$(Build.SourcesDirectory)' + displayName: 'Validate Files' + timeoutInMinutes: 5 + condition: succeededOrFailed() env: { TMP: $(tmpDir), TEMP: $(tmpDir) } + - task: Powershell@2 + displayName: 'Create Diff' + inputs: + filePath: azure-pipelines/Create-PRDiff.ps1 + arguments: '-DiffFile $(DiffFile)' + pwsh: true + - task: PublishBuildArtifacts@1 + condition: failed() + displayName: 'Publish Format and Messages File Diff' + inputs: + PathtoPublish: '$(DiffFile)' + ArtifactName: 'format.diff' - stage: Build_And_Test_x64 dependsOn: Code_Format From 6a2a992145119fa03de142108a7f92f90f8bba27 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 12:56:56 -0700 Subject: [PATCH 05/34] errrgh --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 588722ef725..1d6acc71505 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -28,7 +28,7 @@ stages: displayName: 'Setup TMP Directory' - checkout: self clean: true - submodules: false + submodules: true - script: | call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ -host_arch=amd64 -arch=amd64 -no_logo From f552cbad1fd9e845791810d4e35dcd0a01d3e169 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 13:24:16 -0700 Subject: [PATCH 06/34] aaaaaugh --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1d6acc71505..bd4d94b8d57 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -51,7 +51,7 @@ stages: - task: Powershell@2 displayName: 'Create Diff' inputs: - filePath: azure-pipelines/Create-PRDiff.ps1 + filePath: azure-devops/Create-PRDiff.ps1 arguments: '-DiffFile $(DiffFile)' pwsh: true - task: PublishBuildArtifacts@1 From abf9330ca989ffa54307e94730f412077c5fb7ea Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 17:06:20 -0700 Subject: [PATCH 07/34] fix build error, change how format is done --- CMakeLists.txt | 2 +- azure-devops/Create-PRDiff.ps1 | 19 ----------------- azure-devops/create-prdiff.ps1 | 25 +++++++++++++++++++++++ azure-devops/validate-files.cmd | 4 ++-- azure-pipelines.yml | 15 +++++++------- format.diff | 13 ++++++++++++ {format => tools/format}/CMakeLists.txt | 27 +++++++++++++++++-------- 7 files changed, 67 insertions(+), 38 deletions(-) delete mode 100644 azure-devops/Create-PRDiff.ps1 create mode 100644 azure-devops/create-prdiff.ps1 create mode 100644 format.diff rename {format => tools/format}/CMakeLists.txt (73%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6604f15fbd4..b4b000683fd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -87,7 +87,7 @@ if(BUILD_TESTING) add_subdirectory(tests) endif() -add_subdirectory(format) +add_subdirectory(tools/format) if(CLANG_FORMAT_TARGETS) add_custom_target(format) add_dependencies(format ${CLANG_FORMAT_TARGETS}) diff --git a/azure-devops/Create-PRDiff.ps1 b/azure-devops/Create-PRDiff.ps1 deleted file mode 100644 index a0be6bf3f32..00000000000 --- a/azure-devops/Create-PRDiff.ps1 +++ /dev/null @@ -1,19 +0,0 @@ -[CmdletBinding(PositionalBinding=$False)] -Param( - [Parameter(Mandatory=$True)] - [String]$DiffFile -) - -Start-Process -FilePath 'git' -ArgumentList 'diff' ` - -NoNewWindow -Wait ` - -RedirectStandardOutput $DiffFile -if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) -{ - $msg = @( - 'The formatting of the files in the repo were not what we expected.', - 'Please access the diff from format.diff in the build artifacts,' - 'and apply the patch with `git apply`' - ) - Write-Error ($msg -join "`n") - throw -} diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 new file mode 100644 index 00000000000..8d3aa87a643 --- /dev/null +++ b/azure-devops/create-prdiff.ps1 @@ -0,0 +1,25 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +[CmdletBinding(PositionalBinding = $False)] +Param( + [Parameter(Mandatory = $True)] + [String]$DiffFile +) + +Start-Process -FilePath 'git' -ArgumentList 'diff' ` + -NoNewWindow -Wait ` + -RedirectStandardOutput $DiffFile +if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { + $msg = @( + 'The formatting of the files in the repo was not what we expected.' + 'Please access the diff from format.diff in the build artifacts,' + 'and apply it with `git apply`.' + 'Alternatively, you can run the `format` CMake target:' + ' cmake --build --target format' + '' + ) + Write-Host ($msg -join '`n') + Write-Host (Get-Content -LiteralPath $DiffFile) + throw +} diff --git a/azure-devops/validate-files.cmd b/azure-devops/validate-files.cmd index 1b88c9f5f1d..05719d1f048 100644 --- a/azure-devops/validate-files.cmd +++ b/azure-devops/validate-files.cmd @@ -3,7 +3,7 @@ call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ -host_arch=amd64 -arch=amd64 -no_logo cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release ^ - -S "%1\tools" -B "%TEMP%\validate-build" + -S "%1\tools\validate" -B "%TEMP%\validate-build" cmake --build "%TEMP%\validate-build" -"%TEMP%\validate-build\validate\validate.exe" +"%TEMP%\validate-build\validate.exe" @echo If your build fails here, you need to fix the listed issues. diff --git a/azure-pipelines.yml b/azure-pipelines.yml index bd4d94b8d57..71b18d7e67e 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -18,7 +18,7 @@ stages: displayName: 'Validation' variables: - name: DiffFile - value: '$(Build.ArtifactStagingDirectory)\format.diff' + value: '$(Build.ArtifactStagingDirectory)/format.diff' steps: - script: | if exist "$(tmpDir)" ( @@ -28,13 +28,12 @@ stages: displayName: 'Setup TMP Directory' - checkout: self clean: true - submodules: true + submodules: false - script: | call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ -host_arch=amd64 -arch=amd64 -no_logo - cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release -DERROR_ON_CLANG_FORMAT_NOT_FOUND=1 ^ - -S $(Build.SourcesDirectory) -B $(tmpDir)\format-build - cmake --build $(tmpDir)\format-build --target format + cmake -G Ninja -S $(Build.SourcesDirectory)/tools/format -B $(tmpDir)/format-build + cmake --build $(tmpDir)/format-build displayName: 'clang-format' timeoutInMinutes: 5 condition: succeededOrFailed() @@ -51,14 +50,14 @@ stages: - task: Powershell@2 displayName: 'Create Diff' inputs: - filePath: azure-devops/Create-PRDiff.ps1 + filePath: azure-devops/create-prdiff.ps1 arguments: '-DiffFile $(DiffFile)' - pwsh: true + pwsh: false - task: PublishBuildArtifacts@1 condition: failed() displayName: 'Publish Format and Messages File Diff' inputs: - PathtoPublish: '$(DiffFile)' + PathToPublish: '$(DiffFile)' ArtifactName: 'format.diff' - stage: Build_And_Test_x64 diff --git a/format.diff b/format.diff new file mode 100644 index 00000000000..e89eea89c89 --- /dev/null +++ b/format.diff @@ -0,0 +1,13 @@ +diff --git a/azure-pipelines.yml b/azure-pipelines.yml +index bd4d94b8..5f5c0e36 100644 +--- a/azure-pipelines.yml ++++ b/azure-pipelines.yml +@@ -53,7 +53,7 @@ stages: + inputs: + filePath: azure-devops/Create-PRDiff.ps1 + arguments: '-DiffFile $(DiffFile)' +- pwsh: true ++ pwsh: false + - task: PublishBuildArtifacts@1 + condition: failed() + displayName: 'Publish Format and Messages File Diff' diff --git a/format/CMakeLists.txt b/tools/format/CMakeLists.txt similarity index 73% rename from format/CMakeLists.txt rename to tools/format/CMakeLists.txt index 4e8bde21be2..12d42a88cc2 100644 --- a/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -1,10 +1,13 @@ +cmake_minimum_required(VERSION 3.22) +project(stl-format NONE) + set(did_search ON) if(NOT DEFINED CLANG_FORMAT) message(STATUS "Searching for VS clang-format") set(did_search OFF) endif() -if(ERROR_ON_CLANG_FORMAT_NOT_FOUND) +if(PROJECT_IS_TOP_LEVEL) set(message_level FATAL_ERROR) else() set(message_level WARNING) @@ -26,10 +29,10 @@ if(CLANG_FORMAT) endif() file(GLOB_RECURSE maybe-clang-format-files - "${PROJECT_SOURCE_DIR}/stl/inc/*" - "${PROJECT_SOURCE_DIR}/stl/src/*" - "${PROJECT_SOURCE_DIR}/tests/*" - "${PROJECT_SOURCE_DIR}/tools/*" + "../../stl/inc/*" + "../../stl/src/*" + "../../tests/*" + "../../tools/*" ) set(clang-format-files "") foreach(maybe-file IN LISTS maybe-clang-format-files) @@ -46,7 +49,7 @@ if(CLANG_FORMAT) set(clang-format-targets "") foreach(file IN LISTS clang-format-files) cmake_path(RELATIVE_PATH file - BASE_DIRECTORY "${PROJECT_SOURCE_DIR}" + BASE_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/../.." OUTPUT_VARIABLE relative-file ) string(REPLACE "/" "_" relative-file "${relative-file}") @@ -58,10 +61,18 @@ if(CLANG_FORMAT) list(APPEND clang-format-targets "${target_name}") endforeach() - set(CLANG_FORMAT_TARGETS "${clang-format-targets}" PARENT_SCOPE) + if(PROJECT_IS_TOP_LEVEL) + add_custom_target(format ALL) + add_dependencies(format ${clang-format-targets}) + else() + set(CLANG_FORMAT_TARGETS "${clang-format-targets}" PARENT_SCOPE) + endif() else() if(did_search) message("${message_level}" "Searching for VS clang-format - not found.") endif() - set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) + + if(NOT PROJECT_IS_TOP_LEVEL) + set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) + endif() endif() From 791e5ba72b4cfc3def58c24a66af976cdc29619e Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 17:14:26 -0700 Subject: [PATCH 08/34] moar fixin' --- tools/CMakeLists.txt | 1 + tools/format/CMakeLists.txt | 5 ++++- tools/validate/CMakeLists.txt | 9 +++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 1b271afbb2c..5d4089779e6 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -13,5 +13,6 @@ add_compile_options(/W4 /WX $<$>:/Zi> /permissive-) include_directories(inc) +add_subdirectory(format) add_subdirectory(jobify) add_subdirectory(validate) diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index 12d42a88cc2..24836c668d8 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -1,5 +1,8 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + cmake_minimum_required(VERSION 3.22) -project(stl-format NONE) +project(msvc_standard_libraries_format NONE) set(did_search ON) if(NOT DEFINED CLANG_FORMAT) diff --git a/tools/validate/CMakeLists.txt b/tools/validate/CMakeLists.txt index 96fd38dcbaa..6bde4f49ebd 100644 --- a/tools/validate/CMakeLists.txt +++ b/tools/validate/CMakeLists.txt @@ -1,4 +1,13 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +project(msvc_standard_libraries_validate LANGUAGES CXX) + +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED True) +set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") + +add_compile_definitions(NOMINMAX UNICODE _UNICODE) +add_compile_options(/W4 /WX $<$>:/Zi> /permissive-) + add_executable(validate validate.cpp) From 4212816e16fd40ca195182e7f3261f7d989b380b Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 17:17:39 -0700 Subject: [PATCH 09/34] forgot cmr :'( --- tools/validate/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/validate/CMakeLists.txt b/tools/validate/CMakeLists.txt index 6bde4f49ebd..ec1da86ed22 100644 --- a/tools/validate/CMakeLists.txt +++ b/tools/validate/CMakeLists.txt @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +cmake_minimum_required(VERSION 3.22) project(msvc_standard_libraries_validate LANGUAGES CXX) set(CMAKE_CXX_STANDARD 20) From 8863e7a0e7ec2a2e450f1ddce1ebe63ea8ad59d5 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Tue, 26 Apr 2022 17:31:02 -0700 Subject: [PATCH 10/34] forgot to skip .diff files -.- they will have mixed CRLF/LF --- tools/validate/validate.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index 1491f1f3d1f..f67a4dcb3c9 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -201,6 +201,7 @@ int main() { L".dll"sv, L".exe"sv, L".obj"sv, + L".diff"sv, }; static constexpr array tabby_filenames{ From 1a27369500e433e289f616acaad8ca657c699774 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 13:05:56 -0700 Subject: [PATCH 11/34] okay fix builds hopefully --- CMakeLists.txt | 6 +++--- tools/format/CMakeLists.txt | 14 +++++--------- tools/validate/validate.cpp | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b4b000683fd..bf83f51ae6b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -87,8 +87,8 @@ if(BUILD_TESTING) add_subdirectory(tests) endif() -add_subdirectory(tools/format) -if(CLANG_FORMAT_TARGETS) +add_subdirectory(tools/format EXCLUDE_FROM_ALL) +if(TARGET clang-format-all) add_custom_target(format) - add_dependencies(format ${CLANG_FORMAT_TARGETS}) + add_dependencies(format clang-format-all) endif() diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index 24836c668d8..25f07c9ff6b 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -31,7 +31,7 @@ if(CLANG_FORMAT) message(STATUS "Searching for VS clang-format - found") endif() - file(GLOB_RECURSE maybe-clang-format-files + file(GLOB_RECURSE maybe-clang-format-files CONFIGURE_DEPENDS "../../stl/inc/*" "../../stl/src/*" "../../tests/*" @@ -64,18 +64,14 @@ if(CLANG_FORMAT) list(APPEND clang-format-targets "${target_name}") endforeach() - if(PROJECT_IS_TOP_LEVEL) - add_custom_target(format ALL) - add_dependencies(format ${clang-format-targets}) - else() - set(CLANG_FORMAT_TARGETS "${clang-format-targets}" PARENT_SCOPE) + if(clang-format-targets) + add_custom_target(clang-format-all ALL) + add_dependencies(clang-format-all ${clang-format-targets}) endif() else() if(did_search) message("${message_level}" "Searching for VS clang-format - not found.") endif() - if(NOT PROJECT_IS_TOP_LEVEL) - set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) - endif() + set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) endif() diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index f67a4dcb3c9..8f6a59aa1b6 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -198,10 +198,10 @@ int main() { }; static constexpr array skipped_extensions{ + L".diff"sv, L".dll"sv, L".exe"sv, L".obj"sv, - L".diff"sv, }; static constexpr array tabby_filenames{ From cf2f36797ce6d71881f9c6082b2c6ad8110f58d1 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 14:48:52 -0700 Subject: [PATCH 12/34] remove hiddenf iles --- tools/format/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index 25f07c9ff6b..6123dc946a2 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -40,7 +40,7 @@ if(CLANG_FORMAT) set(clang-format-files "") foreach(maybe-file IN LISTS maybe-clang-format-files) cmake_path(GET maybe-file EXTENSION LAST_ONLY extension) - if(extension MATCHES [[^(|\.cpp|\.h|\.hpp)$]]) + if(extension MATCHES [[^(|\.cpp|\.h|\.hpp)$]] AND NOT maybe-file MATCHES [[^\.]]) list(APPEND clang-format-files "${maybe-file}") endif() endforeach() From de2988d948a7cd8c4bf47c3927c5d5175789cd9b Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 15:43:41 -0700 Subject: [PATCH 13/34] remove format.diff file also make sure that we don't add diff files in the future (this is a thing Nicole has done more times than she'd like to admit) --- format.diff | 13 ------------- tools/validate/validate.cpp | 10 +++++++++- 2 files changed, 9 insertions(+), 14 deletions(-) delete mode 100644 format.diff diff --git a/format.diff b/format.diff deleted file mode 100644 index e89eea89c89..00000000000 --- a/format.diff +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/azure-pipelines.yml b/azure-pipelines.yml -index bd4d94b8..5f5c0e36 100644 ---- a/azure-pipelines.yml -+++ b/azure-pipelines.yml -@@ -53,7 +53,7 @@ stages: - inputs: - filePath: azure-devops/Create-PRDiff.ps1 - arguments: '-DiffFile $(DiffFile)' -- pwsh: true -+ pwsh: false - - task: PublishBuildArtifacts@1 - condition: failed() - displayName: 'Publish Format and Messages File Diff' diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index 8f6a59aa1b6..dd7d68e46dc 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -198,18 +198,24 @@ int main() { }; static constexpr array skipped_extensions{ - L".diff"sv, L".dll"sv, L".exe"sv, L".obj"sv, }; + // make sure someone doesn't accidentally include a diff in the tree + static constexpr array bad_extensions{ + L".diff"sv, + L".patch"sv, + }; + static constexpr array tabby_filenames{ L".gitmodules"sv, }; static_assert(is_sorted(skipped_directories.begin(), skipped_directories.end())); static_assert(is_sorted(skipped_extensions.begin(), skipped_extensions.end())); + static_assert(is_sorted(bad_extensions.begin(), bad_extensions.end())); static_assert(is_sorted(tabby_filenames.begin(), tabby_filenames.end())); vector buffer; // reused for performance @@ -245,6 +251,8 @@ int main() { if (binary_search(skipped_extensions.begin(), skipped_extensions.end(), extension)) { continue; + } else if (binary_search(bad_extensions.begin(), bad_extensions.end(), extensions)) { + fwprintf(stderr, L"Validation failed: the file \"%ls\" should not be checked in.\n", filepath.c_str()); } const TabPolicy tab_policy = binary_search(tabby_filenames.begin(), tabby_filenames.end(), filename) From fd5abee31dbc5e6ef99e9c1545da6e387cb972e6 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 15:48:39 -0700 Subject: [PATCH 14/34] single-quotes -> double-quotes --- azure-devops/create-prdiff.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 8d3aa87a643..40125535413 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -19,7 +19,7 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { ' cmake --build --target format' '' ) - Write-Host ($msg -join '`n') + Write-Host ($msg -join "`n") Write-Host (Get-Content -LiteralPath $DiffFile) throw } From 070076d4082fcef664af9242443959852684ef02 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 16:04:10 -0700 Subject: [PATCH 15/34] (* cries *) --- tools/validate/validate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index dd7d68e46dc..32afd0255f0 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -251,7 +251,7 @@ int main() { if (binary_search(skipped_extensions.begin(), skipped_extensions.end(), extension)) { continue; - } else if (binary_search(bad_extensions.begin(), bad_extensions.end(), extensions)) { + } else if (binary_search(bad_extensions.begin(), bad_extensions.end(), extension)) { fwprintf(stderr, L"Validation failed: the file \"%ls\" should not be checked in.\n", filepath.c_str()); } From 955beb6aafd77d5b6afa48dafe9690225b649c24 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 16:18:35 -0700 Subject: [PATCH 16/34] (* cries more *) --- azure-devops/create-prdiff.ps1 | 2 +- tools/format/CMakeLists.txt | 25 ++++++++++++------------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 40125535413..d266e04a1df 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -20,6 +20,6 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { '' ) Write-Host ($msg -join "`n") - Write-Host (Get-Content -LiteralPath $DiffFile) + Write-Host [string](Get-Content -LiteralPath $DiffFile) throw } diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index 6123dc946a2..96f60cde7e6 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -31,26 +31,27 @@ if(CLANG_FORMAT) message(STATUS "Searching for VS clang-format - found") endif() - file(GLOB_RECURSE maybe-clang-format-files CONFIGURE_DEPENDS + file(GLOB_RECURSE maybe_clang_format_files CONFIGURE_DEPENDS "../../stl/inc/*" "../../stl/src/*" "../../tests/*" "../../tools/*" ) - set(clang-format-files "") - foreach(maybe-file IN LISTS maybe-clang-format-files) - cmake_path(GET maybe-file EXTENSION LAST_ONLY extension) - if(extension MATCHES [[^(|\.cpp|\.h|\.hpp)$]] AND NOT maybe-file MATCHES [[^\.]]) - list(APPEND clang-format-files "${maybe-file}") + set(clang_format_files "") + foreach(maybe_file IN LISTS maybe_clang_format_files) + cmake_path(GET maybe_file FILENAME filename) + cmake_path(GET maybe_file EXTENSION LAST_ONLY extension) + if(extension MATCHES [[^(|\.cpp|\.h|\.hpp)$]] AND NOT filename MATCHES [[^\.]]) + list(APPEND clang_format_files "${maybe_file}") endif() endforeach() - if(NOT clang-format-files) + if(NOT clang_format_files) message("${message_level}" "Could not find any files to clang-format!") endif() - set(clang-format-targets "") - foreach(file IN LISTS clang-format-files) + add_custom_target(clang-format-all ALL) + foreach(file IN LISTS clang_format_files) cmake_path(RELATIVE_PATH file BASE_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/../.." OUTPUT_VARIABLE relative-file @@ -61,12 +62,10 @@ if(CLANG_FORMAT) COMMAND "${CLANG_FORMAT}" -style=file -i "${file}" WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}" ) - list(APPEND clang-format-targets "${target_name}") + add_dependencies(clang-format-all "${target_name}") endforeach() - if(clang-format-targets) - add_custom_target(clang-format-all ALL) - add_dependencies(clang-format-all ${clang-format-targets}) + if(clang_format_targets) endif() else() if(did_search) From ae2f1e699571475c3bbe5b6bbe5daa6f5c9136b6 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Wed, 27 Apr 2022 16:23:45 -0700 Subject: [PATCH 17/34] clean UP --- tools/format/CMakeLists.txt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index 96f60cde7e6..a28ee1ac126 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -64,13 +64,8 @@ if(CLANG_FORMAT) ) add_dependencies(clang-format-all "${target_name}") endforeach() - - if(clang_format_targets) - endif() else() if(did_search) message("${message_level}" "Searching for VS clang-format - not found.") endif() - - set(CLANG_FORMAT_TARGETS "" PARENT_SCOPE) endif() From ffbff5bc2e46533b092e4d347502d086839adf27 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 09:55:31 -0700 Subject: [PATCH 18/34] fix people's problems --- azure-devops/create-prdiff.ps1 | 6 +++--- azure-pipelines.yml | 2 +- tools/format/CMakeLists.txt | 6 +++--- tools/validate/validate.cpp | 4 +++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index d266e04a1df..6c2c3bf988d 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -18,8 +18,8 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' '' + Get-Content -LiteralPath $DiffFile -Raw ) - Write-Host ($msg -join "`n") - Write-Host [string](Get-Content -LiteralPath $DiffFile) - throw + Write-Error ($msg -join "`n") -ErrorAction Continue + Write-Host '##vso[task.setvariable variable=diffWritten]true' } diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 71b18d7e67e..702bb45f9c4 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -54,7 +54,7 @@ stages: arguments: '-DiffFile $(DiffFile)' pwsh: false - task: PublishBuildArtifacts@1 - condition: failed() + condition: eq(variables.diffWritten, 'true') displayName: 'Publish Format and Messages File Diff' inputs: PathToPublish: '$(DiffFile)' diff --git a/tools/format/CMakeLists.txt b/tools/format/CMakeLists.txt index a28ee1ac126..70d35bbd25d 100644 --- a/tools/format/CMakeLists.txt +++ b/tools/format/CMakeLists.txt @@ -4,10 +4,10 @@ cmake_minimum_required(VERSION 3.22) project(msvc_standard_libraries_format NONE) -set(did_search ON) +set(did_search OFF) if(NOT DEFINED CLANG_FORMAT) message(STATUS "Searching for VS clang-format") - set(did_search OFF) + set(did_search ON) endif() if(PROJECT_IS_TOP_LEVEL) @@ -31,7 +31,7 @@ if(CLANG_FORMAT) message(STATUS "Searching for VS clang-format - found") endif() - file(GLOB_RECURSE maybe_clang_format_files CONFIGURE_DEPENDS + file(GLOB_RECURSE maybe_clang_format_files "../../stl/inc/*" "../../stl/src/*" "../../tests/*" diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index 32afd0255f0..d2a88c6844d 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -251,8 +251,10 @@ int main() { if (binary_search(skipped_extensions.begin(), skipped_extensions.end(), extension)) { continue; - } else if (binary_search(bad_extensions.begin(), bad_extensions.end(), extension)) { + } + if (binary_search(bad_extensions.begin(), bad_extensions.end(), extension)) { fwprintf(stderr, L"Validation failed: the file \"%ls\" should not be checked in.\n", filepath.c_str()); + continue; } const TabPolicy tab_policy = binary_search(tabby_filenames.begin(), tabby_filenames.end(), filename) From abe3bd06d1692fa18bb08d2307b8fda7660c7303 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 09:56:44 -0700 Subject: [PATCH 19/34] [TEST] try clang-format --- tools/validate/validate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index d2a88c6844d..34b921149f3 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -2,11 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #define _CRT_SECURE_NO_WARNINGS -#include #include #include #include #include +#include #include #include #include From 0f57832a7bf00e7e3d9c84e754ba21669e3ae0ee Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:05:07 -0700 Subject: [PATCH 20/34] oops, need to fail the pipeline --- azure-pipelines.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 702bb45f9c4..4b493ad7b10 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -59,6 +59,9 @@ stages: inputs: PathToPublish: '$(DiffFile)' ArtifactName: 'format.diff' + - script: 'exit 1' + condition: eq(variables.diffWritten, 'true') + displayName: 'Fail the pipeline if branch needs formatting' - stage: Build_And_Test_x64 dependsOn: Code_Format From cce432e403e6ee8ec782950f439afa27d29d6875 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:14:30 -0700 Subject: [PATCH 21/34] NEAT --- azure-devops/create-prdiff.ps1 | 15 +++++++++++---- azure-pipelines.yml | 9 --------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 6c2c3bf988d..591a3c267ba 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -17,9 +17,16 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' - '' - Get-Content -LiteralPath $DiffFile -Raw ) - Write-Error ($msg -join "`n") -ErrorAction Continue - Write-Host '##vso[task.setvariable variable=diffWritten]true' + Write-Host "##vso[error]$($msg -join "`n##vso[error]")" + + Write-Host + + Write-Host '##[group]Expected formatting - diff' + Write-Host (Get-Content -LiteralPath $DiffFile -Raw) + Write-Host '##[endgroup]' + + Write-Host "##vso[artifact.upload artifactname=format.diff]$DiffFile" + + Write-Host '##vso[task.complete result=Failed]DONE' } diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4b493ad7b10..f77729684ac 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -53,15 +53,6 @@ stages: filePath: azure-devops/create-prdiff.ps1 arguments: '-DiffFile $(DiffFile)' pwsh: false - - task: PublishBuildArtifacts@1 - condition: eq(variables.diffWritten, 'true') - displayName: 'Publish Format and Messages File Diff' - inputs: - PathToPublish: '$(DiffFile)' - ArtifactName: 'format.diff' - - script: 'exit 1' - condition: eq(variables.diffWritten, 'true') - displayName: 'Fail the pipeline if branch needs formatting' - stage: Build_And_Test_x64 dependsOn: Code_Format From bb3222b2417e2d8e71c9e643820501b9cd285635 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:19:35 -0700 Subject: [PATCH 22/34] vso[error] -> [error] --- azure-devops/create-prdiff.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 591a3c267ba..330a0c23a0e 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -18,7 +18,7 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' ) - Write-Host "##vso[error]$($msg -join "`n##vso[error]")" + Write-Host "##[error]$($msg -join "`n##[error]")" Write-Host From a2d6aa479a6d666081ed8b1268350a680d503f95 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:31:02 -0700 Subject: [PATCH 23/34] how does that look? --- azure-devops/create-prdiff.ps1 | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 330a0c23a0e..52a98c5be2c 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -11,22 +11,23 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` -NoNewWindow -Wait ` -RedirectStandardOutput $DiffFile if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { - $msg = @( - 'The formatting of the files in the repo was not what we expected.' + $errorMessage = @( + '##[error]The formatting of the files in the repo was not what we expected.' 'Please access the diff from format.diff in the build artifacts,' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' ) - Write-Host "##[error]$($msg -join "`n##[error]")" + [Console]::Error.Write($errorMessage -join "`n") - Write-Host - - Write-Host '##[group]Expected formatting - diff' - Write-Host (Get-Content -LiteralPath $DiffFile -Raw) - Write-Host '##[endgroup]' - - Write-Host "##vso[artifact.upload artifactname=format.diff]$DiffFile" + $restOfMessage = @( + '' + '##[group]Expected formatting - diff' + Get-Content -LiteralPath $DiffFile -Raw + '##[endgroup]' + "##vso[artifact.upload artifactname=format.diff]$DiffFile" + '##vso[task.complete result=Failed]DONE' + ) - Write-Host '##vso[task.complete result=Failed]DONE' + [Console]::Out.Write($restOfMessage -join "`n") } From d32cc676d9cb3f00fa514da8b3a8142a14a6d547 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:36:15 -0700 Subject: [PATCH 24/34] does that result in failures --- azure-devops/create-prdiff.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 52a98c5be2c..5e88ef151aa 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -12,13 +12,13 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` -RedirectStandardOutput $DiffFile if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { $errorMessage = @( - '##[error]The formatting of the files in the repo was not what we expected.' + 'The formatting of the files in the repo was not what we expected.' 'Please access the diff from format.diff in the build artifacts,' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' ) - [Console]::Error.Write($errorMessage -join "`n") + Write-Error ($errorMessage -join "`n") -ErrorAction Continue $restOfMessage = @( '' @@ -29,5 +29,5 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { '##vso[task.complete result=Failed]DONE' ) - [Console]::Out.Write($restOfMessage -join "`n") + Write-Host ($restOfMessage -join "`n") } From 53e0527847831eea726488dd1e28bebac99d154b Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:42:01 -0700 Subject: [PATCH 25/34] do tests --- azure-devops/create-prdiff.ps1 | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 5e88ef151aa..a723654e6b7 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -12,22 +12,27 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` -RedirectStandardOutput $DiffFile if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { $errorMessage = @( - 'The formatting of the files in the repo was not what we expected.' + '##[error]The formatting of the files in the repo was not what we expected.' 'Please access the diff from format.diff in the build artifacts,' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' ) - Write-Error ($errorMessage -join "`n") -ErrorAction Continue + [Console]::Error.Write($errorMessage -join "`n") + [Console]::Error.Flush() + [Console]::Out.Write($errorMessage -join "`n") + [Console]::Out.Flush() $restOfMessage = @( '' '##[group]Expected formatting - diff' Get-Content -LiteralPath $DiffFile -Raw '##[endgroup]' + '##[error]Does putting errors here work?' "##vso[artifact.upload artifactname=format.diff]$DiffFile" '##vso[task.complete result=Failed]DONE' ) - Write-Host ($restOfMessage -join "`n") + [Console]::Out.Write($restOfMessage -join "`n") + [Console]::Out.Flush() } From 93d21869dac0a9884452fc0b81bb2eaca9538137 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:44:31 -0700 Subject: [PATCH 26/34] does logissue work? --- azure-devops/create-prdiff.ps1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index a723654e6b7..0f142c48cbc 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -12,7 +12,7 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` -RedirectStandardOutput $DiffFile if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { $errorMessage = @( - '##[error]The formatting of the files in the repo was not what we expected.' + '##vso[task.logissue type=error]The formatting of the files in the repo was not what we expected.' 'Please access the diff from format.diff in the build artifacts,' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' @@ -28,7 +28,6 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { '##[group]Expected formatting - diff' Get-Content -LiteralPath $DiffFile -Raw '##[endgroup]' - '##[error]Does putting errors here work?' "##vso[artifact.upload artifactname=format.diff]$DiffFile" '##vso[task.complete result=Failed]DONE' ) From e9ff5c41b02c571912e732d588fcef97d4e284ac Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:51:45 -0700 Subject: [PATCH 27/34] figured it out! --- azure-devops/create-prdiff.ps1 | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 0f142c48cbc..8c9ee3956da 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -11,19 +11,12 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` -NoNewWindow -Wait ` -RedirectStandardOutput $DiffFile if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { - $errorMessage = @( + $message = @( '##vso[task.logissue type=error]The formatting of the files in the repo was not what we expected.' 'Please access the diff from format.diff in the build artifacts,' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' - ) - [Console]::Error.Write($errorMessage -join "`n") - [Console]::Error.Flush() - [Console]::Out.Write($errorMessage -join "`n") - [Console]::Out.Flush() - - $restOfMessage = @( '' '##[group]Expected formatting - diff' Get-Content -LiteralPath $DiffFile -Raw @@ -31,7 +24,5 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { "##vso[artifact.upload artifactname=format.diff]$DiffFile" '##vso[task.complete result=Failed]DONE' ) - - [Console]::Out.Write($restOfMessage -join "`n") - [Console]::Out.Flush() + Write-Host ($message -join "`n") } From 0f539a876d39b4e3694b9a4778226baf2ee43614 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 10:59:09 -0700 Subject: [PATCH 28/34] clang-format --- tools/validate/validate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index 34b921149f3..d2a88c6844d 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -2,11 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #define _CRT_SECURE_NO_WARNINGS +#include #include #include #include #include -#include #include #include #include From 2688dce6fdb0dd3fa82f809ddffb55235f1144e8 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 11:00:34 -0700 Subject: [PATCH 29/34] [TEST] validate (tabs) --- CMakeLists.txt | 66 +++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bf83f51ae6b..240efe019be 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ option(BUILD_TESTING "Enable testing" ON) set(VCLIBS_SUFFIX "_oss" CACHE STRING "suffix for built DLL names to avoid conflicts with distributed DLLs") if(NOT DEFINED VCLIBS_TARGET_ARCHITECTURE) - set(VCLIBS_TARGET_ARCHITECTURE "${CMAKE_CXX_COMPILER_ARCHITECTURE_ID}") + set(VCLIBS_TARGET_ARCHITECTURE "${CMAKE_CXX_COMPILER_ARCHITECTURE_ID}") endif() set(CMAKE_CXX_FLAGS "") @@ -20,41 +20,41 @@ set(CMAKE_CXX_STANDARD_LIBRARIES_INIT "kernel32.lib") set(CMAKE_MSVC_RUNTIME_LIBRARY "") if("${VCLIBS_TARGET_ARCHITECTURE}" MATCHES "^[xX]86$") - set(VCLIBS_TARGET_ARCHITECTURE "x86") - set(VCLIBS_I386_OR_AMD64 "i386") - set(VCLIBS_X86_OR_X64 "x86") - # Note that we set _WIN32_WINNT to a high level to make declarations available, but still engage downlevel - # runtime dynamic linking by setting our own _STL_WIN32_WINNT back to Windows XP. - add_compile_definitions(_X86_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) - add_compile_options(/arch:IA32) + set(VCLIBS_TARGET_ARCHITECTURE "x86") + set(VCLIBS_I386_OR_AMD64 "i386") + set(VCLIBS_X86_OR_X64 "x86") + # Note that we set _WIN32_WINNT to a high level to make declarations available, but still engage downlevel + # runtime dynamic linking by setting our own _STL_WIN32_WINNT back to Windows XP. + add_compile_definitions(_X86_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + add_compile_options(/arch:IA32) elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[xX]64$") - set(VCLIBS_TARGET_ARCHITECTURE "x64") - set(VCLIBS_I386_OR_AMD64 "amd64") - set(VCLIBS_X86_OR_X64 "x64") - add_compile_definitions(_AMD64_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + set(VCLIBS_TARGET_ARCHITECTURE "x64") + set(VCLIBS_I386_OR_AMD64 "amd64") + set(VCLIBS_X86_OR_X64 "x64") + add_compile_definitions(_AMD64_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[aA][rR][mM][vV]7$") - set(VCLIBS_TARGET_ARCHITECTURE "arm") - set(VCLIBS_I386_OR_AMD64 "arm") - set(VCLIBS_X86_OR_X64 "arm") - add_compile_definitions(_ARM_ _VCRT_WIN32_WINNT=0x0602 _STL_WIN32_WINNT=0x0602 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) - string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") + set(VCLIBS_TARGET_ARCHITECTURE "arm") + set(VCLIBS_I386_OR_AMD64 "arm") + set(VCLIBS_X86_OR_X64 "arm") + add_compile_definitions(_ARM_ _VCRT_WIN32_WINNT=0x0602 _STL_WIN32_WINNT=0x0602 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[aA][rR][mM]64$") - set(VCLIBS_TARGET_ARCHITECTURE "arm64") - set(VCLIBS_I386_OR_AMD64 "arm64") - set(VCLIBS_X86_OR_X64 "arm64") - add_compile_definitions(_ARM64_ _VCRT_WIN32_WINNT=0x0A00 _STL_WIN32_WINNT=0x0A00 - _WIN32_WINNT=0x0A00 NTDDI_VERSION=NTDDI_WIN10) - string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") + set(VCLIBS_TARGET_ARCHITECTURE "arm64") + set(VCLIBS_I386_OR_AMD64 "arm64") + set(VCLIBS_X86_OR_X64 "arm64") + add_compile_definitions(_ARM64_ _VCRT_WIN32_WINNT=0x0A00 _STL_WIN32_WINNT=0x0A00 + _WIN32_WINNT=0x0A00 NTDDI_VERSION=NTDDI_WIN10) + string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") else() - message(FATAL_ERROR "Could not determine target architecture: VCLIBS_TARGET_ARCHITECTURE: ${VCLIBS_TARGET_ARCHITECTURE}") + message(FATAL_ERROR "Could not determine target architecture: VCLIBS_TARGET_ARCHITECTURE: ${VCLIBS_TARGET_ARCHITECTURE}") endif() add_compile_definitions( - _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH WIN32_LEAN_AND_MEAN STRICT _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS - _CRT_DECLARE_NONSTDC_NAMES=1 ) + _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH WIN32_LEAN_AND_MEAN STRICT _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS + _CRT_DECLARE_NONSTDC_NAMES=1 ) add_compile_options(/diagnostics:caret /W4 /WX /w14265 /w15038 /d1FastFail /guard:cf /Z7 /Gy /Zp8 /std:c++latest /permissive- /Zc:threadSafeInit- /Zl) @@ -83,12 +83,12 @@ add_subdirectory(boost-math) add_subdirectory(stl) if(BUILD_TESTING) - enable_testing() - add_subdirectory(tests) + enable_testing() + add_subdirectory(tests) endif() add_subdirectory(tools/format EXCLUDE_FROM_ALL) if(TARGET clang-format-all) - add_custom_target(format) - add_dependencies(format clang-format-all) + add_custom_target(format) + add_dependencies(format clang-format-all) endif() From 19d0f42fd9dfab77a47e43f6db8a5b76ccb9ca56 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 11:29:29 -0700 Subject: [PATCH 30/34] validate prettifying --- azure-devops/validate-files.cmd | 9 ---- azure-pipelines.yml | 13 ++++-- tools/validate/CMakeLists.txt | 3 +- tools/validate/validate.cpp | 82 +++++++++++++++++++++++---------- 4 files changed, 67 insertions(+), 40 deletions(-) delete mode 100644 azure-devops/validate-files.cmd diff --git a/azure-devops/validate-files.cmd b/azure-devops/validate-files.cmd deleted file mode 100644 index 05719d1f048..00000000000 --- a/azure-devops/validate-files.cmd +++ /dev/null @@ -1,9 +0,0 @@ -:: Copyright (c) Microsoft Corporation. -:: SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ - -host_arch=amd64 -arch=amd64 -no_logo -cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release ^ - -S "%1\tools\validate" -B "%TEMP%\validate-build" -cmake --build "%TEMP%\validate-build" -"%TEMP%\validate-build\validate.exe" -@echo If your build fails here, you need to fix the listed issues. diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f77729684ac..d7c30ec76f6 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -38,11 +38,13 @@ stages: timeoutInMinutes: 5 condition: succeededOrFailed() env: { TMP: $(tmpDir), TEMP: $(tmpDir) } - - task: BatchScript@1 - inputs: - filename: 'azure-devops/validate-files.cmd' - failOnStandardError: true - arguments: '$(Build.SourcesDirectory)' + - script: | + call "%ProgramFiles%\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" ^ + -host_arch=amd64 -arch=amd64 -no_logo + cmake -G Ninja -DCMAKE_CXX_COMPILER=cl -DCMAKE_BUILD_TYPE=Release ^ + -S $(Build.SourcesDirectory)/tools/validate -B $(tmpDir)/validate-build + cmake --build $(tmpDir)/validate-build + "$(tmpDir)\validate-build\validate.exe" displayName: 'Validate Files' timeoutInMinutes: 5 condition: succeededOrFailed() @@ -53,6 +55,7 @@ stages: filePath: azure-devops/create-prdiff.ps1 arguments: '-DiffFile $(DiffFile)' pwsh: false + condition: succeededOrFailed() - stage: Build_And_Test_x64 dependsOn: Code_Format diff --git a/tools/validate/CMakeLists.txt b/tools/validate/CMakeLists.txt index ec1da86ed22..81909e1f394 100644 --- a/tools/validate/CMakeLists.txt +++ b/tools/validate/CMakeLists.txt @@ -9,6 +9,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED True) set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") add_compile_definitions(NOMINMAX UNICODE _UNICODE) -add_compile_options(/W4 /WX $<$>:/Zi> /permissive-) +# we use SAL annotations, so pass /analyze +add_compile_options(/W4 /WX $<$>:/Zi> /permissive- /analyze) add_executable(validate validate.cpp) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index d2a88c6844d..b2ca81ce5c5 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -52,12 +53,26 @@ class BinaryFile { FILE* m_file{nullptr}; }; +int validation_failure(const filesystem::path& filepath, _Printf_format_string_ const wchar_t* format, ...) { + fwprintf(stderr, L"##vso[task.logissue type=error;sourcepath=%ls]Validation failed: ", filepath.c_str()); + + va_list args; + va_start(args, format); + int result = vfwprintf(stderr, format, args); + va_end(args); + + fwprintf(stderr, L"\n"); + return result; +} + enum class TabPolicy : bool { Forbidden, Allowed }; -void scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vector& buffer) { +bool scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vector& buffer) { constexpr char CR = '\r'; constexpr char LF = '\n'; + bool has_error = false; + bool has_cr = false; bool has_lf = false; bool has_crlf = false; @@ -97,11 +112,12 @@ void scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec } else if (ch != CR && ch != LF && !(ch >= 0x20 && ch <= 0x7E)) { // [0x20, 0x7E] are the printable characters, including the space character. // https://en.wikipedia.org/wiki/ASCII#Printable_characters + has_error = true; ++disallowed_characters; constexpr size_t MaxErrorsForDisallowedCharacters = 10; if (disallowed_characters <= MaxErrorsForDisallowedCharacters) { - fwprintf(stderr, L"Validation failed: %ls contains disallowed character 0x%02X.\n", - filepath.c_str(), static_cast(ch)); + validation_failure( + filepath, L"file contains disallowed character 0x%02X.", static_cast(ch)); } } @@ -126,41 +142,46 @@ void scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec } if (has_cr) { - fwprintf( - stderr, L"Validation failed: %ls contains CR line endings (possibly damaged CRLF).\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file contains CR line endings (possibly damaged CRLF)."); } else if (has_lf && has_crlf) { - fwprintf(stderr, L"Validation failed: %ls contains mixed line endings (both LF and CRLF).\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file contains mixed line endings (both LF and CRLF)."); } else if (has_lf) { - fwprintf(stderr, L"Validation failed: %ls contains LF line endings.", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file contains LF line endings."); if (prev != LF) { - fwprintf(stderr, L" Also, it doesn't end with a newline.\n"); + validation_failure(filepath, L"file doesn't end with a newline."); } else if (previous2 == LF) { - fwprintf(stderr, L" Also, it ends with multiple newlines.\n"); - } else { - fwprintf(stderr, L"\n"); + validation_failure(filepath, L" file ends with multiple newlines."); } } else if (has_crlf) { if (previous2 != CR || prev != LF) { - fwprintf(stderr, L"Validation failed: %ls doesn't end with a newline.\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file doesn't end with a newline."); } else if (previous3 == LF) { - fwprintf(stderr, L"Validation failed: %ls ends with multiple newlines.\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L" file ends with multiple newlines."); } } else { - fwprintf(stderr, L"Validation failed: %ls doesn't contain any newlines.\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file doesn't contain any newlines."); } if (has_utf8_bom) { - fwprintf(stderr, L"Validation failed: %ls contains UTF-8 BOM characters.\n", filepath.c_str()); + has_error = true; + validation_failure(filepath, L"file contains UTF-8 BOM characters."); } if (tab_policy == TabPolicy::Forbidden && tab_characters != 0) { - fwprintf(stderr, L"Validation failed: %ls contains %zu tab characters.\n", filepath.c_str(), tab_characters); + has_error = true; + validation_failure(filepath, L"file contains %zu tab characters.", tab_characters); } if (trailing_whitespace_lines != 0) { - fwprintf(stderr, L"Validation failed: %ls contains %zu lines with trailing whitespace.\n", filepath.c_str(), - trailing_whitespace_lines); + has_error = true; + validation_failure(filepath, L"file contains %zu lines with trailing whitespace.", trailing_whitespace_lines); } if (overlength_lines != 0) { @@ -179,10 +200,13 @@ void scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec static_assert(is_sorted(checked_extensions.begin(), checked_extensions.end())); if (binary_search(checked_extensions.begin(), checked_extensions.end(), filepath.extension().wstring())) { - fwprintf(stderr, L"Validation failed: %ls contains %zu lines with more than %zu columns.\n", - filepath.c_str(), overlength_lines, max_line_length); + has_error = true; + validation_failure( + filepath, L"file contains %zu lines with more than %zu columns.\n", overlength_lines, max_line_length); } } + + return has_error; } int main() { @@ -219,6 +243,7 @@ int main() { static_assert(is_sorted(tabby_filenames.begin(), tabby_filenames.end())); vector buffer; // reused for performance + bool any_errors = false; for (filesystem::recursive_directory_iterator rdi{"."}, last; rdi != last; ++rdi) { const filesystem::path& filepath = rdi->path(); @@ -239,12 +264,12 @@ int main() { constexpr size_t maximum_relative_path_length = 120; if (relative_path.size() > maximum_relative_path_length) { - fwprintf(stderr, L"Validation failed: the path \"%ls\" is too long (%zu characters; the limit is %zu).\n", - filepath.c_str(), relative_path.size(), maximum_relative_path_length); + validation_failure(filepath, L"filepath is too long (%zu characters; the limit is %zu).", + relative_path.size(), maximum_relative_path_length); } if (relative_path.find(L' ') != wstring::npos) { - fwprintf(stderr, L"Validation failed: the path \"%ls\" contains spaces.\n", filepath.c_str()); + validation_failure(filepath, L"filepath contains spaces."); } const wstring extension = filepath.extension().wstring(); @@ -253,7 +278,7 @@ int main() { continue; } if (binary_search(bad_extensions.begin(), bad_extensions.end(), extension)) { - fwprintf(stderr, L"Validation failed: the file \"%ls\" should not be checked in.\n", filepath.c_str()); + validation_failure(filepath, L"file should not be checked in."); continue; } @@ -261,6 +286,13 @@ int main() { ? TabPolicy::Allowed : TabPolicy::Forbidden; - scan_file(filepath, tab_policy, buffer); + any_errors |= scan_file(filepath, tab_policy, buffer); } + + if (any_errors) { + fwprintf( + stderr, L"##vso[task.logissue type=warning]If your build fails here, you need to fix the listed issues."); + } + + return any_errors ? 1 : 0; } From 9f899e405b08b1caa5f6ef7c49fbca76a167aab2 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 11:34:22 -0700 Subject: [PATCH 31/34] more prettifying --- tools/validate/validate.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index b2ca81ce5c5..7c265d03618 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -54,7 +54,8 @@ class BinaryFile { }; int validation_failure(const filesystem::path& filepath, _Printf_format_string_ const wchar_t* format, ...) { - fwprintf(stderr, L"##vso[task.logissue type=error;sourcepath=%ls]Validation failed: ", filepath.c_str()); + fwprintf(stderr, L"##vso[task.logissue type=error;sourcepath=%ls;linenumber=1;columnnumber=1]Validation failed: ", + filepath.c_str()); va_list args; va_start(args, format); @@ -291,8 +292,7 @@ int main() { if (any_errors) { fwprintf( - stderr, L"##vso[task.logissue type=warning]If your build fails here, you need to fix the listed issues."); + stderr, L"##vso[task.logissue type=warning]If your build fails here, you need to fix the listed issues.\n"); + fwprintf(stderr, L"##vso[task.complete result=Failed]DONE\n"); } - - return any_errors ? 1 : 0; } From b8566a9d4945dac2b3853370b1f1c36920199a16 Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Thu, 28 Apr 2022 11:38:21 -0700 Subject: [PATCH 32/34] final format --- CMakeLists.txt | 66 +++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 240efe019be..bf83f51ae6b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ option(BUILD_TESTING "Enable testing" ON) set(VCLIBS_SUFFIX "_oss" CACHE STRING "suffix for built DLL names to avoid conflicts with distributed DLLs") if(NOT DEFINED VCLIBS_TARGET_ARCHITECTURE) - set(VCLIBS_TARGET_ARCHITECTURE "${CMAKE_CXX_COMPILER_ARCHITECTURE_ID}") + set(VCLIBS_TARGET_ARCHITECTURE "${CMAKE_CXX_COMPILER_ARCHITECTURE_ID}") endif() set(CMAKE_CXX_FLAGS "") @@ -20,41 +20,41 @@ set(CMAKE_CXX_STANDARD_LIBRARIES_INIT "kernel32.lib") set(CMAKE_MSVC_RUNTIME_LIBRARY "") if("${VCLIBS_TARGET_ARCHITECTURE}" MATCHES "^[xX]86$") - set(VCLIBS_TARGET_ARCHITECTURE "x86") - set(VCLIBS_I386_OR_AMD64 "i386") - set(VCLIBS_X86_OR_X64 "x86") - # Note that we set _WIN32_WINNT to a high level to make declarations available, but still engage downlevel - # runtime dynamic linking by setting our own _STL_WIN32_WINNT back to Windows XP. - add_compile_definitions(_X86_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) - add_compile_options(/arch:IA32) + set(VCLIBS_TARGET_ARCHITECTURE "x86") + set(VCLIBS_I386_OR_AMD64 "i386") + set(VCLIBS_X86_OR_X64 "x86") + # Note that we set _WIN32_WINNT to a high level to make declarations available, but still engage downlevel + # runtime dynamic linking by setting our own _STL_WIN32_WINNT back to Windows XP. + add_compile_definitions(_X86_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + add_compile_options(/arch:IA32) elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[xX]64$") - set(VCLIBS_TARGET_ARCHITECTURE "x64") - set(VCLIBS_I386_OR_AMD64 "amd64") - set(VCLIBS_X86_OR_X64 "x64") - add_compile_definitions(_AMD64_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + set(VCLIBS_TARGET_ARCHITECTURE "x64") + set(VCLIBS_I386_OR_AMD64 "amd64") + set(VCLIBS_X86_OR_X64 "x64") + add_compile_definitions(_AMD64_ _VCRT_WIN32_WINNT=0x0501 _STL_WIN32_WINNT=0x0501 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[aA][rR][mM][vV]7$") - set(VCLIBS_TARGET_ARCHITECTURE "arm") - set(VCLIBS_I386_OR_AMD64 "arm") - set(VCLIBS_X86_OR_X64 "arm") - add_compile_definitions(_ARM_ _VCRT_WIN32_WINNT=0x0602 _STL_WIN32_WINNT=0x0602 - _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) - string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") + set(VCLIBS_TARGET_ARCHITECTURE "arm") + set(VCLIBS_I386_OR_AMD64 "arm") + set(VCLIBS_X86_OR_X64 "arm") + add_compile_definitions(_ARM_ _VCRT_WIN32_WINNT=0x0602 _STL_WIN32_WINNT=0x0602 + _WIN32_WINNT=0x0602 NTDDI_VERSION=NTDDI_WIN8) + string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") elseif(VCLIBS_TARGET_ARCHITECTURE MATCHES "^[aA][rR][mM]64$") - set(VCLIBS_TARGET_ARCHITECTURE "arm64") - set(VCLIBS_I386_OR_AMD64 "arm64") - set(VCLIBS_X86_OR_X64 "arm64") - add_compile_definitions(_ARM64_ _VCRT_WIN32_WINNT=0x0A00 _STL_WIN32_WINNT=0x0A00 - _WIN32_WINNT=0x0A00 NTDDI_VERSION=NTDDI_WIN10) - string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") + set(VCLIBS_TARGET_ARCHITECTURE "arm64") + set(VCLIBS_I386_OR_AMD64 "arm64") + set(VCLIBS_X86_OR_X64 "arm64") + add_compile_definitions(_ARM64_ _VCRT_WIN32_WINNT=0x0A00 _STL_WIN32_WINNT=0x0A00 + _WIN32_WINNT=0x0A00 NTDDI_VERSION=NTDDI_WIN10) + string(APPEND CMAKE_CXX_STANDARD_LIBRARIES " Synchronization.lib") else() - message(FATAL_ERROR "Could not determine target architecture: VCLIBS_TARGET_ARCHITECTURE: ${VCLIBS_TARGET_ARCHITECTURE}") + message(FATAL_ERROR "Could not determine target architecture: VCLIBS_TARGET_ARCHITECTURE: ${VCLIBS_TARGET_ARCHITECTURE}") endif() add_compile_definitions( - _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH WIN32_LEAN_AND_MEAN STRICT _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS - _CRT_DECLARE_NONSTDC_NAMES=1 ) + _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH WIN32_LEAN_AND_MEAN STRICT _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS + _CRT_DECLARE_NONSTDC_NAMES=1 ) add_compile_options(/diagnostics:caret /W4 /WX /w14265 /w15038 /d1FastFail /guard:cf /Z7 /Gy /Zp8 /std:c++latest /permissive- /Zc:threadSafeInit- /Zl) @@ -83,12 +83,12 @@ add_subdirectory(boost-math) add_subdirectory(stl) if(BUILD_TESTING) - enable_testing() - add_subdirectory(tests) + enable_testing() + add_subdirectory(tests) endif() add_subdirectory(tools/format EXCLUDE_FROM_ALL) if(TARGET clang-format-all) - add_custom_target(format) - add_dependencies(format clang-format-all) + add_custom_target(format) + add_dependencies(format clang-format-all) endif() From bf41006fb19c22be6117bbe87e4b6ddb833af5fc Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 29 Apr 2022 00:01:41 -0700 Subject: [PATCH 33/34] Code review feedback, fixing missed path failures. --- azure-devops/create-prdiff.ps1 | 2 +- tools/validate/validate.cpp | 65 +++++++++++++++------------------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index 8c9ee3956da..bad70cf2451 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -18,7 +18,7 @@ if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format' '' - '##[group]Expected formatting - diff' + '##[group]Expected formatting - click to expand diff' Get-Content -LiteralPath $DiffFile -Raw '##[endgroup]' "##vso[artifact.upload artifactname=format.diff]$DiffFile" diff --git a/tools/validate/validate.cpp b/tools/validate/validate.cpp index 7c265d03618..c590c7dd9ea 100644 --- a/tools/validate/validate.cpp +++ b/tools/validate/validate.cpp @@ -53,13 +53,16 @@ class BinaryFile { FILE* m_file{nullptr}; }; -int validation_failure(const filesystem::path& filepath, _Printf_format_string_ const wchar_t* format, ...) { +int validation_failure( + bool& any_errors, const filesystem::path& filepath, _Printf_format_string_ const wchar_t* format, ...) { + any_errors = true; + fwprintf(stderr, L"##vso[task.logissue type=error;sourcepath=%ls;linenumber=1;columnnumber=1]Validation failed: ", filepath.c_str()); va_list args; va_start(args, format); - int result = vfwprintf(stderr, format, args); + const int result = vfwprintf(stderr, format, args); va_end(args); fwprintf(stderr, L"\n"); @@ -68,12 +71,11 @@ int validation_failure(const filesystem::path& filepath, _Printf_format_string_ enum class TabPolicy : bool { Forbidden, Allowed }; -bool scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vector& buffer) { +void scan_file( + bool& any_errors, const filesystem::path& filepath, const TabPolicy tab_policy, vector& buffer) { constexpr char CR = '\r'; constexpr char LF = '\n'; - bool has_error = false; - bool has_cr = false; bool has_lf = false; bool has_crlf = false; @@ -113,12 +115,11 @@ bool scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec } else if (ch != CR && ch != LF && !(ch >= 0x20 && ch <= 0x7E)) { // [0x20, 0x7E] are the printable characters, including the space character. // https://en.wikipedia.org/wiki/ASCII#Printable_characters - has_error = true; ++disallowed_characters; constexpr size_t MaxErrorsForDisallowedCharacters = 10; if (disallowed_characters <= MaxErrorsForDisallowedCharacters) { - validation_failure( - filepath, L"file contains disallowed character 0x%02X.", static_cast(ch)); + validation_failure(any_errors, filepath, L"file contains disallowed character 0x%02X.", + static_cast(ch)); } } @@ -143,46 +144,38 @@ bool scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec } if (has_cr) { - has_error = true; - validation_failure(filepath, L"file contains CR line endings (possibly damaged CRLF)."); + validation_failure(any_errors, filepath, L"file contains CR line endings (possibly damaged CRLF)."); } else if (has_lf && has_crlf) { - has_error = true; - validation_failure(filepath, L"file contains mixed line endings (both LF and CRLF)."); + validation_failure(any_errors, filepath, L"file contains mixed line endings (both LF and CRLF)."); } else if (has_lf) { - has_error = true; - validation_failure(filepath, L"file contains LF line endings."); + validation_failure(any_errors, filepath, L"file contains LF line endings."); if (prev != LF) { - validation_failure(filepath, L"file doesn't end with a newline."); + validation_failure(any_errors, filepath, L"file doesn't end with a newline."); } else if (previous2 == LF) { - validation_failure(filepath, L" file ends with multiple newlines."); + validation_failure(any_errors, filepath, L"file ends with multiple newlines."); } } else if (has_crlf) { if (previous2 != CR || prev != LF) { - has_error = true; - validation_failure(filepath, L"file doesn't end with a newline."); + validation_failure(any_errors, filepath, L"file doesn't end with a newline."); } else if (previous3 == LF) { - has_error = true; - validation_failure(filepath, L" file ends with multiple newlines."); + validation_failure(any_errors, filepath, L"file ends with multiple newlines."); } } else { - has_error = true; - validation_failure(filepath, L"file doesn't contain any newlines."); + validation_failure(any_errors, filepath, L"file doesn't contain any newlines."); } if (has_utf8_bom) { - has_error = true; - validation_failure(filepath, L"file contains UTF-8 BOM characters."); + validation_failure(any_errors, filepath, L"file contains UTF-8 BOM characters."); } if (tab_policy == TabPolicy::Forbidden && tab_characters != 0) { - has_error = true; - validation_failure(filepath, L"file contains %zu tab characters.", tab_characters); + validation_failure(any_errors, filepath, L"file contains %zu tab characters.", tab_characters); } if (trailing_whitespace_lines != 0) { - has_error = true; - validation_failure(filepath, L"file contains %zu lines with trailing whitespace.", trailing_whitespace_lines); + validation_failure( + any_errors, filepath, L"file contains %zu lines with trailing whitespace.", trailing_whitespace_lines); } if (overlength_lines != 0) { @@ -201,13 +194,10 @@ bool scan_file(const filesystem::path& filepath, const TabPolicy tab_policy, vec static_assert(is_sorted(checked_extensions.begin(), checked_extensions.end())); if (binary_search(checked_extensions.begin(), checked_extensions.end(), filepath.extension().wstring())) { - has_error = true; - validation_failure( - filepath, L"file contains %zu lines with more than %zu columns.\n", overlength_lines, max_line_length); + validation_failure(any_errors, filepath, L"file contains %zu lines with more than %zu columns.\n", + overlength_lines, max_line_length); } } - - return has_error; } int main() { @@ -265,12 +255,12 @@ int main() { constexpr size_t maximum_relative_path_length = 120; if (relative_path.size() > maximum_relative_path_length) { - validation_failure(filepath, L"filepath is too long (%zu characters; the limit is %zu).", + validation_failure(any_errors, filepath, L"filepath is too long (%zu characters; the limit is %zu).", relative_path.size(), maximum_relative_path_length); } if (relative_path.find(L' ') != wstring::npos) { - validation_failure(filepath, L"filepath contains spaces."); + validation_failure(any_errors, filepath, L"filepath contains spaces."); } const wstring extension = filepath.extension().wstring(); @@ -278,8 +268,9 @@ int main() { if (binary_search(skipped_extensions.begin(), skipped_extensions.end(), extension)) { continue; } + if (binary_search(bad_extensions.begin(), bad_extensions.end(), extension)) { - validation_failure(filepath, L"file should not be checked in."); + validation_failure(any_errors, filepath, L"file should not be checked in."); continue; } @@ -287,7 +278,7 @@ int main() { ? TabPolicy::Allowed : TabPolicy::Forbidden; - any_errors |= scan_file(filepath, tab_policy, buffer); + scan_file(any_errors, filepath, tab_policy, buffer); } if (any_errors) { From d4fe38f59cb1adc8a1b93609c860b94b369f080c Mon Sep 17 00:00:00 2001 From: nicole mazzuca Date: Fri, 29 Apr 2022 13:32:41 -0700 Subject: [PATCH 34/34] add instructions for downloading --- azure-devops/create-prdiff.ps1 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/azure-devops/create-prdiff.ps1 b/azure-devops/create-prdiff.ps1 index bad70cf2451..4d2c004fa6d 100644 --- a/azure-devops/create-prdiff.ps1 +++ b/azure-devops/create-prdiff.ps1 @@ -13,7 +13,8 @@ Start-Process -FilePath 'git' -ArgumentList 'diff' ` if (0 -ne (Get-Item -LiteralPath $DiffFile).Length) { $message = @( '##vso[task.logissue type=error]The formatting of the files in the repo was not what we expected.' - 'Please access the diff from format.diff in the build artifacts,' + 'Please access the diff from format.diff in the build artifacts' + '(you can download it by clicking the three dots at the right)' 'and apply it with `git apply`.' 'Alternatively, you can run the `format` CMake target:' ' cmake --build --target format'