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

[libxdf] Add new port #32816

Merged
merged 10 commits into from
Aug 3, 2023
Merged

[libxdf] Add new port #32816

merged 10 commits into from
Aug 3, 2023

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented Jul 29, 2023

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jul 31, 2023
@JonLiu1993
Copy link
Member

@myd7349, Thanks for your PR, if this PR is ready for review please let me know.

@JonLiu1993
Copy link
Member

When I tested the usage, I get this error;

CMakeLists.txt

cmake_minimum_required (VERSION 3.8)

project(test)

find_package(libxdf CONFIG REQUIRED)

# Add source to this project's executable.
add_executable (test "usageTest.cpp")

target_link_libraries(test PRIVATE XDF::xdf)

usageTest.cpp

#include <stdio.h>
#include <xdf.h>

int main()
{
    printf("%s\n", "HELLO WORLD");
    return 0;
}

Config:

1> CMake generation started for configuration: 'x64-Debug'.
1> Command line: "C:\WINDOWS\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES\MICROSOFT VISUAL STUDIO\2022\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Visual Studio 17 2022" -A x64  -DCMAKE_CONFIGURATION_TYPES:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="C:\Users\test\source\repos\usageTest\out\install\x64-Debug" -DCMAKE_TOOLCHAIN_FILE=F:/Feature-test/libxdf/vcpkg/scripts/buildsystems/vcpkg.cmake "C:\Users\test\source\repos\usageTest" 2>&1"
1> Working directory: C:\Users\test\source\repos\usageTest\out\build\x64-Debug
1> [CMake] -- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.19045.
1> [CMake] -- The C compiler identification is MSVC 19.36.32537.0
1> [CMake] -- The CXX compiler identification is MSVC 19.36.32537.0
1> [CMake] -- Detecting C compiler ABI info
1> [CMake] -- Detecting C compiler ABI info - done
1> [CMake] -- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.36.32532/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting C compile features
1> [CMake] -- Detecting C compile features - done
1> [CMake] -- Detecting CXX compiler ABI info
1> [CMake] -- Detecting CXX compiler ABI info - done
1> [CMake] -- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.36.32532/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting CXX compile features
1> [CMake] -- Detecting CXX compile features - done
1> [CMake] -- Configuring done (7.5s)
1> [CMake] -- Generating done (0.0s)
1> [CMake] -- Build files have been written to: C:/Users/test/source/repos/usageTest/out/build/x64-debug
1> Extracted CMake variables.
1> Extracted source files and headers.
1> Extracted code model.
1> Extracted toolchain configurations.
1> Extracted includes paths.
1> CMake generation finished.

Build

>------ Build All started: Project: usageTest, Configuration: x64-Debug ------
  MSBuild version 17.6.3+07e294721 for .NET Framework
  
    Checking Build System
    Building Custom Rule C:/Users/test/source/repos/usageTest/usageTest/CMakeLists.txt
C:\Users\test\source\repos\usageTest\out\build\x64-Debug\usageTest\LINK : fatal error LNK1104: cannot open file 'pugixml.lib' 

Build All failed.

@JonLiu1993
Copy link
Member

I found that the upstream find_package (pugixml 1.9 QUIET) has obvious version requirements for the pugixml port. I added find_package (pugixml CONFIG REQUIRED) to the CMakeLists to successfully find pugixml.lib

@myd7349
Copy link
Contributor Author

myd7349 commented Aug 1, 2023

Hi! @JonLiu1993 Thanks for your review.

I have created a PR to address this issue: xdf-modules/libxdf#38.

@myd7349 myd7349 marked this pull request as ready for review August 1, 2023 13:43
JonLiu1993
JonLiu1993 previously approved these changes Aug 2, 2023
@JonLiu1993
Copy link
Member

Tested usage successfully by libxdf:x64-windows:

libxdf provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(libxdf CONFIG REQUIRED)
    target_link_libraries(main PRIVATE XDF::xdf)

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Aug 2, 2023
@@ -0,0 +1,18 @@
{
"name": "libxdf",
"version-date": "2023-08-02",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the upstream version is 0.99.8.

Suggested change
"version-date": "2023-08-02",
"version": "0.99.8",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! @JavierMatosD There was a bug in 0.99.8 and have been fixed by xdf-modules/libxdf#38. That why I upgrade from 0.99.8 to latest commit. Without this fix, a 60~ LOC patch file is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a stable release is preferred, then I can revert the last two commits of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! We can drop the patch when we update the port next :)

@JavierMatosD JavierMatosD marked this pull request as draft August 2, 2023 18:27
This reverts commit a9a791b.

Revert "[libxdf] Update to latest commit"

This reverts commit 834ae87.
@JavierMatosD JavierMatosD marked this pull request as ready for review August 3, 2023 16:53
@JavierMatosD JavierMatosD merged commit 790f778 into microsoft:master Aug 3, 2023
15 checks passed
@myd7349 myd7349 deleted the libxdf-init branch August 4, 2023 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants