Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[krabsetw] Update to 4.3.1 #33640

Merged
merged 2 commits into from
Oct 3, 2023
Merged

[krabsetw] Update to 4.3.1 #33640

merged 2 commits into from
Oct 3, 2023

Conversation

dahapls
Copy link
Contributor

@dahapls dahapls commented Sep 7, 2023

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@dahapls
Copy link
Contributor Author

dahapls commented Sep 7, 2023

#33640

@dahapls
Copy link
Contributor Author

dahapls commented Sep 7, 2023

@microsoft-github-policy-service agree

@MonicaLiu0311 MonicaLiu0311 added the category:port-update The issue is with a library, which is requesting update new revision label Sep 8, 2023
@MonicaLiu0311
Copy link
Contributor

An error occurred while testing usage, but was not caused by your PR, would you like to fix it?

usage:

krabsetw is header-only and can be used from CMake via:

    find_path(KRABSETW_INCLUDE_DIRS "krabs.hpp")
    target_include_directories(main PRIVATE ${KRABSETW_INCLUDE_DIRS})

When testing usage, the following error occurs:

>------ Build All started: Project: CMakeUsage, Configuration: x64-Debug ------
  MSBuild version 17.6.3+07e294721 for .NET Framework
  
    Checking Build System
    Building Custom Rule C:/Users/monica/source/repos/CMakeUsage/CMakeUsage/CMakeLists.txt
    CMakeUsage.cpp
G:\vcpkg\installed\x64-windows\include\krabs\kt.hpp(108,9): error C2440: 'return': cannot convert from 'const char [17]' to 'std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t>>' [C:\Users\monica\source\repos\CMakeUsage\out\build\x64-debug\CMakeUsage\main.vcxproj]
  G:\vcpkg\installed\x64-windows\include\krabs\kt.hpp(108,9): message : No constructor could take the source type, or constructor overload resolution was ambiguous [C:\Users\monica\source\repos\CMakeUsage\out\build\x64-debug\CMakeUsage\main.vcxproj]

Build All failed.
CMakeFindUsage.cpp
#include <iostream>
#include "krabs.hpp"

using namespace std;

int main()
{
cout << "Hello CMake." << endl;
return 0;
}

CMakeLists.txt
cmake_minimum_required (VERSION 3.8)

set(CMAKE_TOOLCHAIN_FILE "E:/krabsetw/scripts/buildsystems/vcpkg.cmake")

project ("CMakeFindUsage")

add_library (main "CMakeFindUsage.cpp")

find_path(KRABSETW_INCLUDE_DIRS "krabs.hpp")
target_include_directories(main PRIVATE ${KRABSETW_INCLUDE_DIRS})

@MonicaLiu0311
Copy link
Contributor

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".

@MonicaLiu0311 MonicaLiu0311 marked this pull request as draft September 13, 2023 06:49
@dahapls
Copy link
Contributor Author

dahapls commented Sep 18, 2023

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".

I apologize for late reply, I was out of town last week, I'll take a look and see if I can figure out the testing stage error this week

@dahapls
Copy link
Contributor Author

dahapls commented Sep 28, 2023

Hi Monica, I've found the issue with the testing usage. It was missing the preprocessor definition for UNICODE that prevents it from building successfully (https://github.com/microsoft/krabsetw/blob/550f5edfdb6464567d7a618702316183d31746e5/krabs/README.md?plain=1#L5C15-L5C15). Adding add_compile_definitions(UNICODE) to CMakeLists.txt lets it build without errors.

@dahapls dahapls marked this pull request as ready for review September 28, 2023 09:34
@BillyONeal
Copy link
Member

Can you add a usage file which says that UNICODE needs to be on? See https://learn.microsoft.com/en-us/vcpkg/maintainers/handling-usage-files

@BillyONeal
Copy link
Member

BillyONeal commented Oct 3, 2023

Actually nevermind this is just an update, thanks!

@BillyONeal BillyONeal merged commit 0cccee1 into microsoft:master Oct 3, 2023
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Oct 7, 2023
clementperon pushed a commit to clementperon/vcpkg that referenced this pull request Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants