Skip to content

Commit

Permalink
Change post-build checks to be warnings rather than errors. (#245)
Browse files Browse the repository at this point in the history
* Change post-build checks to be warnings rather than errors. Note that they are still treated as errors for purposes of including a port in our curated catalog.

Improve the diagnostics for the pkgconfig check. Example:

```
billyoneal@Billys-MacBook-Pro vcpkg % ../vcpkg-tool/build/vcpkg install gtest --no-binarycaching
Computing installation plan...
The following packages will be built and installed:
    gtest[core]:arm64-osx -> 1.11.0
Detecting compiler hash for triplet arm64-osx...
Starting package 1/1: gtest:arm64-osx
Building package gtest[core]:arm64-osx...
-- Using community triplet arm64-osx. This triplet configuration is not guaranteed to succeed.
-- [COMMUNITY] Loading triplet configuration from: /Users/billyoneal/vcpkg/triplets/community/arm64-osx.cmake
-- Downloading https://github.com/google/googletest/archive/release-1.11.0.tar.gz -> google-googletest-release-1.11.0.tar.gz...
-- Cleaning sources at /Users/billyoneal/vcpkg/buildtrees/gtest/src/ase-1.11.0-30e87b9484.clean. Use --editable to skip cleaning for the packages you specify.
-- Extracting source /Users/billyoneal/vcpkg/downloads/google-googletest-release-1.11.0.tar.gz
-- Applying patch fix-main-lib-path.patch
-- Using source at /Users/billyoneal/vcpkg/buildtrees/gtest/src/ase-1.11.0-30e87b9484.clean
-- Configuring arm64-osx-dbg
-- Configuring arm64-osx-rel
CMake Warning at scripts/cmake/vcpkg_configure_cmake.cmake:433 (message):
  The following variables are not used in CMakeLists.txt:

      BUILD_GTEST
      CMAKE_DEBUG_POSTFIX

  Please recheck them and remove the unnecessary options from the
  `vcpkg_configure_cmake` call.

  If these options should still be passed for whatever reason, please use the
  `MAYBE_UNUSED_VARIABLES` argument.
Call Stack (most recent call first):
  ports/gtest/portfile.cmake:17 (vcpkg_configure_cmake)
  scripts/ports.cmake:141 (include)

-- Building arm64-osx-dbg
-- Building arm64-osx-rel
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest_main.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-all.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-death-test.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-filepath.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-internal-inl.h
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-matchers.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-port.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-printers.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-test-part.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/src/gtest-typed-test.cc
-- Installing: /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/share/gtest/copyright
-- Performing post-build validation
pkgconfig directories should be lib/pkgconfig or lib/debug/pkgconfig.
The following misplaced pkgconfig files were found:

    /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/lib/manual-link/pkgconfig/gmock_main.pc
    /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/lib/manual-link/pkgconfig/gtest_main.pc
    /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/debug/lib/manual-link/pkgconfig/gmock_main.pc
    /Users/billyoneal/vcpkg/packages/gtest_arm64-osx/debug/lib/manual-link/pkgconfig/gtest_main.pc

You can move the pkgconfig files with commands similar to:

    file(MAKE_DIRECTORY  "${CURRENT_PACKAGES_DIR}/lib/pkgconfig" "${CURRENT_PACKAGES_DIR}/lib/debug/pkgconfig")
    file(RENAME "${CURRENT_PACKAGES_DIR}/lib/manual-link/pkgconfig/gmock_main.pc" "${CURRENT_PACKAGES_DIR}/lib/pkgconfig")
    file(RENAME "${CURRENT_PACKAGES_DIR}/lib/manual-link/pkgconfig/gtest_main.pc" "${CURRENT_PACKAGES_DIR}/lib/pkgconfig")
    file(RENAME "${CURRENT_PACKAGES_DIR}/debug/lib/manual-link/pkgconfig/gmock_main.pc" "${CURRENT_PACKAGES_DIR}/debug/lib/pkgconfig")
    file(RENAME "${CURRENT_PACKAGES_DIR}/debug/lib/manual-link/pkgconfig/gtest_main.pc" "${CURRENT_PACKAGES_DIR}/debug/lib/pkgconfig")
    vcpkg_fixup_pkgconfig()

Found 1 error(s). Please correct the portfile:
    /Users/billyoneal/vcpkg/ports/gtest/portfile.cmake
-- Performing post-build validation done
Installing package gtest[core]:arm64-osx...
Elapsed time for package gtest:arm64-osx: 7.916 s

Total elapsed time: 10.12 s

The package gtest provides CMake targets:

    find_package(GTest CONFIG REQUIRED)
    target_link_libraries(main PRIVATE GTest::gmock GTest::gtest GTest::gmock_main GTest::gtest_main)

billyoneal@Billys-MacBook-Pro vcpkg %
```

* Add filenames and REMOVE_RECURSE note.

* Add non x- switch to enforce post build checks, and use "problem" rather than "error".

* Improve warning text again.

* Actually make the hidden switch work :)

* Attempt to fix broken git by updating Azure Pipelines macos agents
  • Loading branch information
BillyONeal authored Nov 3, 2021
1 parent 4c1a212 commit 4a4bc95
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 49 deletions.
9 changes: 9 additions & 0 deletions azure-pipelines/end-to-end-tests-dir/backcompat-helpers.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ foreach ($backcompatFeaturePort in $backcompatFeaturePorts) {
throw $CurrentTest
}

$failArgs = $succeedArgs + @('--enforce-port-checks')
$CurrentTest = "Should fail: ./vcpkg $($failArgs -join ' ')"
Run-Vcpkg @failArgs
if ($LastExitCode -ne 0) {
Write-Host "... failed (this is good!)."
} else {
throw $CurrentTest
}

# Install failed when prohibiting backcompat features, so it should succeed if we allow them
$CurrentTest = "Should succeeed: ./vcpkg $($succeedArgs -join ' ')"
Run-Vcpkg @succeedArgs
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines/pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- job: osx
displayName: 'OSX'
pool:
vmImage: 'macos-latest'
vmImage: 'macOS-11'
variables:
- name: 'VCPKG_ROOT'
value: $(Build.SourcesDirectory)/vcpkg-root
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines/signing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
- job: macos_build
displayName: 'MacOS Build'
pool:
vmImage: macOS-10.15
vmImage: macOS-11
variables:
- group: vcpkg-dependency-source-blobs
- name: FMT_TARBALL_URL
Expand Down
3 changes: 2 additions & 1 deletion src/vcpkg/build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -956,10 +956,11 @@ namespace vcpkg::Build
action.public_abi(),
std::move(find_itr->second));

if (error_count != 0)
if (error_count != 0 && action.build_options.backcompat_features == BackcompatFeatures::PROHIBIT)
{
return BuildResult::POST_BUILD_CHECKS_FAILED;
}

for (auto&& feature : action.feature_list)
{
for (auto&& f_pgh : scfl.source_control_file->feature_paragraphs)
Expand Down
16 changes: 11 additions & 5 deletions src/vcpkg/install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,10 @@ namespace vcpkg::Install
static constexpr StringLiteral OPTION_MANIFEST_NO_DEFAULT_FEATURES = "x-no-default-features";
static constexpr StringLiteral OPTION_MANIFEST_FEATURE = "x-feature";
static constexpr StringLiteral OPTION_PROHIBIT_BACKCOMPAT_FEATURES = "x-prohibit-backcompat-features";
static constexpr StringLiteral OPTION_ENFORCE_PORT_CHECKS = "enforce-port-checks";
static constexpr StringLiteral OPTION_ALLOW_UNSUPPORTED_PORT = "allow-unsupported";

static constexpr std::array<CommandSwitch, 15> INSTALL_SWITCHES = {{
static constexpr std::array<CommandSwitch, 16> INSTALL_SWITCHES = {{
{OPTION_DRY_RUN, "Do not actually build or install"},
{OPTION_USE_HEAD_VERSION, "Install the libraries on the command line using the latest upstream sources"},
{OPTION_NO_DOWNLOADS, "Do not download new sources"},
Expand All @@ -540,11 +541,12 @@ namespace vcpkg::Install
{OPTION_CLEAN_BUILDTREES_AFTER_BUILD, "Clean buildtrees after building each package"},
{OPTION_CLEAN_PACKAGES_AFTER_BUILD, "Clean packages after building each package"},
{OPTION_CLEAN_DOWNLOADS_AFTER_BUILD, "Clean downloads after building each package"},
{OPTION_PROHIBIT_BACKCOMPAT_FEATURES,
"(experimental) Fail install if a package attempts to use a deprecated feature"},
{OPTION_ENFORCE_PORT_CHECKS,
"Fail install if a port has detected problems or attempts to use a deprecated feature"},
{OPTION_PROHIBIT_BACKCOMPAT_FEATURES, ""},
{OPTION_ALLOW_UNSUPPORTED_PORT, "Instead of erroring on an unsupported port, continue with a warning."},
}};
static constexpr std::array<CommandSwitch, 15> MANIFEST_INSTALL_SWITCHES = {{
static constexpr std::array<CommandSwitch, 17> MANIFEST_INSTALL_SWITCHES = {{
{OPTION_DRY_RUN, "Do not actually build or install"},
{OPTION_USE_HEAD_VERSION, "Install the libraries on the command line using the latest upstream sources"},
{OPTION_NO_DOWNLOADS, "Do not download new sources"},
Expand All @@ -559,6 +561,9 @@ namespace vcpkg::Install
{OPTION_CLEAN_PACKAGES_AFTER_BUILD, "Clean packages after building each package"},
{OPTION_CLEAN_DOWNLOADS_AFTER_BUILD, "Clean downloads after building each package"},
{OPTION_MANIFEST_NO_DEFAULT_FEATURES, "Don't install the default features from the manifest."},
{OPTION_ENFORCE_PORT_CHECKS,
"Fail install if a port has detected problems or attempts to use a deprecated feature"},
{OPTION_PROHIBIT_BACKCOMPAT_FEATURES, ""},
{OPTION_ALLOW_UNSUPPORTED_PORT, "Instead of erroring on an unsupported port, continue with a warning."},
}};

Expand Down Expand Up @@ -797,7 +802,8 @@ namespace vcpkg::Install
const KeepGoing keep_going =
to_keep_going(Util::Sets::contains(options.switches, OPTION_KEEP_GOING) || only_downloads);
const bool prohibit_backcompat_features =
Util::Sets::contains(options.switches, (OPTION_PROHIBIT_BACKCOMPAT_FEATURES));
Util::Sets::contains(options.switches, (OPTION_PROHIBIT_BACKCOMPAT_FEATURES)) ||
Util::Sets::contains(options.switches, (OPTION_ENFORCE_PORT_CHECKS));
const auto unsupported_port_action = Util::Sets::contains(options.switches, OPTION_ALLOW_UNSUPPORTED_PORT)
? Dependencies::UnsupportedPortAction::Warn
: Dependencies::UnsupportedPortAction::Error;
Expand Down
Loading

0 comments on commit 4a4bc95

Please sign in to comment.