[openxlsx] Add new port #41150

motazmuhammad · 2024-09-24T14:51:48Z

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.
[x ] 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.

motazmuhammad · 2024-09-24T14:54:07Z

@motazmuhammad please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree

MonicaLiu0311 · 2024-09-25T10:35:20Z

Why add CMakeLists.txt for libxml? It's already provided upstream.

MonicaLiu0311 · 2024-09-25T10:35:56Z

New port

You can create the yalantinglibs folder in vcpkg/ports/ and add portfile.cmake and vcpkg.json:

vcpkg/ports/testport/
    portfile.cmake
    vcpkg.json

Then run ./vcpkg install testport, if the installation is successful, it proves that there is no problem with the files you added.

Next, run ./vcpkg x-add-verison testport, the vcpkg/verisons/t-/testport.josn file will be added automatically, and a record will be added in vcpkg/verisons/baseline.json.

vcpkg/versions/
    t-/
        testport.josn (automatically generated)
    baseline.json (automatically add records)

The modification needs to meet the requirements: maintainer-guide

ports/openxlsx/vcpkg.json

MonicaLiu0311 · 2024-09-26T02:40:39Z

ports/openxlsx/usage

+ find_package(OpenXLSX CONFIG REQUIRED)
+ target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)


Suggested change

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

OpenXLSX provides CMake targets:

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

The last empty line is a format requirement.

Thank you for clarifying that to me

Follow the pattern from vcpkg tool.

The original indentation was right.

There is confusion about the end of the text. What is not acceptable, and what is indicated on GH by a red (-), is the text ending without a line break. In VS Code it means to add a new empty line. In most UNIX text editors, lines are always ended with a line break.

Suggested change

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

openxlsx provides CMake targets:

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

ports/openxlsx/portfile.cmake

MonicaLiu0311 · 2024-09-26T02:50:15Z

New port

You can create the yalantinglibs folder in vcpkg/ports/ and add portfile.cmake and vcpkg.json:
vcpkg/ports/testport/
    portfile.cmake
    vcpkg.json
Then run ./vcpkg install testport, if the installation is successful, it proves that there is no problem with the files you added.

Next, run ./vcpkg x-add-verison testport, the vcpkg/verisons/t-/testport.josn file will be added automatically, and a record will be added in vcpkg/verisons/baseline.json.
vcpkg/versions/
    t-/
        testport.josn (automatically generated)
    baseline.json (automatically add records)
The modification needs to meet the requirements: maintainer-guide

Reminder: The version file is missing. Please refer to: 39493/files.

This reverts commit ea61266.

…o OpenXLSX

motazmuhammad · 2024-09-27T15:32:30Z

@MonicaLiu0311 Thank you very much for all your insightful comments, I really appreciate them, I updated the fork with your suggested changes ( according to my best understanding). Please let me know if you have further comments, and please be patient with me).

Thank you again

MonicaLiu0311 · 2024-09-29T11:19:59Z

When testing usage, the following error occurs:

E:\test\build>cmake --build .
MSBuild version 17.11.2+c078802d4 for .NET Framework

  1>Checking Build System
  Building Custom Rule E:/test/CMakeLists.txt
  test.cpp
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\include\variant(12): warning STL4038
: The contents of <variant> are available only with C++17 or later. [E:\test\build\main.vcxproj]
E:\openxlsx\installed\x64-windows\include\OpenXLSX\headers\XLXmlParser.hpp(50,10): error C1083: Cannot open include fil
e: 'external/pugixml/pugixml.hpp': No such file or directory [E:\test\build\main.vcxproj]
  (compiling source file '../test.cpp')

test.cpp

#include <iostream>
#include "OpenXLSX.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:/openxlsx/scripts/buildsystems/vcpkg.cmake")
project ("test")
add_library (main "test.cpp")
find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

motazmuhammad · 2024-09-29T19:26:08Z

When testing usage, the following error occurs:

E:\test\build>cmake --build .
MSBuild version 17.11.2+c078802d4 for .NET Framework

  1>Checking Build System
  Building Custom Rule E:/test/CMakeLists.txt
  test.cpp
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\include\variant(12): warning STL4038
: The contents of <variant> are available only with C++17 or later. [E:\test\build\main.vcxproj]
E:\openxlsx\installed\x64-windows\include\OpenXLSX\headers\XLXmlParser.hpp(50,10): error C1083: Cannot open include fil
e: 'external/pugixml/pugixml.hpp': No such file or directory [E:\test\build\main.vcxproj]
  (compiling source file '../test.cpp')

test.cpp
CMakeLists.txt

@MonicaLiu0311 Thank you very much that is fixed now. I have a question, if you dont mind please reply to them.

Right now I am using the head revision (master branch) as evident by the portfile REF master
is that ok, I tried using REF "${VERSION}" however, the correct file was not picked up unless i make it
REF "${VERSION}"
REF "refs/tags/v${VERSION}"
and if I do that, I need to change the SHA512 , do I need to create a seperate SHA for each version? I could not figure this out from the documents.

Thank you again for your patience. I hope to become a regular contributor to vcpkg

MonicaLiu0311 · 2024-09-30T07:51:02Z

ports/openxlsx/portfile.cmake

+ REF master
+ SHA512 429215a961cfcfe8fcb49ee48970dd10dccad09a5c80db2eea2413120a51a2529c9a650584a079756c3b463d53f1fcdec46fdc158ffbc680592896867f60d3ea


Suggested change

REF master

SHA512 429215a961cfcfe8fcb49ee48970dd10dccad09a5c80db2eea2413120a51a2529c9a650584a079756c3b463d53f1fcdec46fdc158ffbc680592896867f60d3ea

REF "v${VERSION}"

SHA512 52205b394383d45c0fb16599ab96453d8a5b9b5cd596096848cc888f47565b2713d9edded06b2ecd7b67736622badf136e4b1becc57bfa5bbdcb1e063a347084

MonicaLiu0311 · 2024-09-30T07:55:31Z

REF "refs/tags/v${VERSION}"

The regular expression for ${VERSION} matches only VERSION semantics. The upstream released version is "v0.3.2" not "0.3.2".

Notes: the patch file is to fix a compilation error in vs2019, 2022, this was already fixed in the master but not committed to the latest release

motazmuhammad · 2024-09-30T19:41:33Z

@MonicaLiu0311 Thank you very much for your comments, I added the changes you recommended. I, now, have two new questions.

Since I am using the v.0.3.2. now I had to add a patch for the code to compile on visual studio. This patch was actually later added to the master branch, but not to the latest release branch. Is that okay? or should the patch be added conditionally ( conditioned on the version number?)
in the context of patching, this patch is not needed for the master release, therefore, the user would not be able to use the master branch via vcpkg, is that okay?

Thank you for your patience

motazmuhammad · 2024-09-30T19:43:29Z

When testing usage, the following error occurs:

E:\test\build>cmake --build .
MSBuild version 17.11.2+c078802d4 for .NET Framework

  1>Checking Build System
  Building Custom Rule E:/test/CMakeLists.txt
  test.cpp
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\include\variant(12): warning STL4038
: The contents of <variant> are available only with C++17 or later. [E:\test\build\main.vcxproj]
E:\openxlsx\installed\x64-windows\include\OpenXLSX\headers\XLXmlParser.hpp(50,10): error C1083: Cannot open include fil
e: 'external/pugixml/pugixml.hpp': No such file or directory [E:\test\build\main.vcxproj]
  (compiling source file '../test.cpp')

test.cpp
CMakeLists.txt

This now works, but please note that to be able to use openxlsx in the cmakelists.txt file one has to add the following line set(CMAKE_CXX_STANDARD 17) as the some of the header files are dependent on c++17, should that be mentioned in the usage file in one way or the other?

dg0yt · 2024-10-01T03:16:51Z

Right now I am using the head revision (master branch) as evident by the portfile
    REF master
is that ok,

No. REF must identify a particular immutable release, tag or commit. master is a branch name, and the contents may change. (HEAD_REF may point to master.)

I tried using
   REF "${VERSION}" 
however, the correct file was not picked up unless i make it
  REF "refs/tags/v${VERSION}"

This indicates that the tags are named v<version>, and you should use

   REF "v${VERSION}"

and if I do that, I need to change the SHA512 , do I need to create a seperate SHA for each version?

Yes. The SHA512 is to verify the downloaded archive, and it is also used as an source asset cache key.

note that to be able to use openxlsx in the cmakelists.txt file one has to add the following line set(CMAKE_CXX_STANDARD 17) as the some of the header files are dependent on c++17, should that be mentioned in the usage file in one way or the other?

If there is exported CMake config, the C++17 requirement could be in the exported compile features.
https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html#requiring-language-standards

dg0yt · 2024-10-01T03:19:37Z

ports/openxlsx/compilation_fix.patch

+- XLColor color() const
+- {
+- return XLColor();
+- }
+ XLColor color() const { return XLColor(); }


This patch has several such sections which are not needed to fix problems.
Either let the portfile download the original raw patch from upstream's repo,
or create a minimal patch of necessary changes.

dg0yt · 2024-10-01T03:20:15Z

ports/openxlsx/portfile.cmake

+ SHA512 52205b394383d45c0fb16599ab96453d8a5b9b5cd596096848cc888f47565b2713d9edded06b2ecd7b67736622badf136e4b1becc57bfa5bbdcb1e063a347084
+ HEAD_REF master
+ PATCHES
+ compilation_fix.patch


Nit-pick:

Suggested change

compilation_fix.patch

compilation_fix.patch

dg0yt · 2024-10-01T03:22:05Z

ports/openxlsx/portfile.cmake

+set(CMAKE_CXX_STANDARD 17)
+
+vcpkg_cmake_configure(
+ SOURCE_PATH ${SOURCE_PATH}
+ PREFER_NINJA
+ OPTIONS
+ -DOPENXLSX_BUILD_TESTS=OFF
+)


Suggested change

set(CMAKE_CXX_STANDARD 17)

vcpkg_cmake_configure(

SOURCE_PATH ${SOURCE_PATH}

PREFER_NINJA

OPTIONS

-DOPENXLSX_BUILD_TESTS=OFF

)

vcpkg_cmake_configure(

SOURCE_PATH ${SOURCE_PATH}

PREFER_NINJA

OPTIONS

-DCMAKE_CXX_STANDARD=17

-DOPENXLSX_BUILD_TESTS=OFF

)

Portfiles run in script mode. CMAKE_CXX_STANDARD has no effect.
https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#portfiles-are-run-in-script-mode

dg0yt · 2024-10-01T03:22:23Z

ports/openxlsx/portfile.cmake

+
+vcpkg_cmake_configure(
+ SOURCE_PATH ${SOURCE_PATH}
+ PREFER_NINJA


Suggested change

PREFER_NINJA

Default.

dg0yt · 2024-10-01T03:22:48Z

ports/openxlsx/portfile.cmake

+ OPTIONS
+ -DOPENXLSX_BUILD_TESTS=OFF
+)
+vcpkg_cmake_install(DISABLE_PARALLEL)


Why DISABLE_PARALLEL?

dg0yt · 2024-10-01T03:24:20Z

ports/openxlsx/usage

+ find_package(OpenXLSX CONFIG REQUIRED)
+ target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)
+


No red (-) at the end, please.

Suggested change

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

openxlsx provides CMake targets:

find_package(OpenXLSX CONFIG REQUIRED)

target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

motazmuhammad added 8 commits June 22, 2024 16:36

original cmake file

ea61266

Merge branch 'master' of https://github.com/microsoft/vcpkg

711e46d

version database

0472204

remove unnecessary lines

55563e9

necessary updates

e43fb14

add usage file

dd2833d

add usage file

1e24e15

Merge branch 'microsoft:master' into OpenXLSX

664e690

LilyWangLL assigned MonicaLiu0311 Sep 25, 2024

Merge branch 'microsoft:master' into OpenXLSX

85b17ad

MonicaLiu0311 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Sep 25, 2024

MonicaLiu0311 changed the title ~~Open xlsx~~ [openxlsx] Add new port Sep 26, 2024

MonicaLiu0311 requested changes Sep 26, 2024

View reviewed changes

motazmuhammad added 6 commits September 26, 2024 20:22

Revert "original cmake file"

4340fa5

This reverts commit ea61266.

Merge branch 'OpenXLSX' of https://github.com/motazmuhammad/vcpkg int…

c7a19b7

…o OpenXLSX

respond to comments

35c16e7

add a version file and update the baseline.json

47fee1d

change to 0.3.2

b250091

add the versions changes and baseline

0de260f

fix missing dependencies

2e33e06

MonicaLiu0311 reviewed Sep 30, 2024

View reviewed changes

Add a batch file and use the version string

5612e8b

Notes: the patch file is to fix a compilation error in vs2019, 2022, this was already fixed in the master but not committed to the latest release

motazmuhammad added 2 commits September 30, 2024 16:52

add the patch file

8cd999c

fix the version file

8a3debe

Merge branch 'microsoft:master' into OpenXLSX

ebf9f13

dg0yt reviewed Oct 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[openxlsx] Add new port #41150

[openxlsx] Add new port #41150

motazmuhammad commented Sep 24, 2024 •

edited

Loading

motazmuhammad commented Sep 24, 2024

MonicaLiu0311 commented Sep 25, 2024

MonicaLiu0311 commented Sep 25, 2024

MonicaLiu0311 Sep 26, 2024

MonicaLiu0311 Sep 26, 2024

motazmuhammad Sep 26, 2024

dg0yt Sep 26, 2024

MonicaLiu0311 commented Sep 26, 2024

motazmuhammad commented Sep 27, 2024

MonicaLiu0311 commented Sep 29, 2024

motazmuhammad commented Sep 29, 2024

MonicaLiu0311 Sep 30, 2024

MonicaLiu0311 commented Sep 30, 2024 •

edited

Loading

motazmuhammad commented Sep 30, 2024

motazmuhammad commented Sep 30, 2024

dg0yt commented Oct 1, 2024 •

edited

Loading

dg0yt Oct 1, 2024

dg0yt Oct 1, 2024

dg0yt Oct 1, 2024

dg0yt Oct 1, 2024

dg0yt Oct 1, 2024

dg0yt Oct 1, 2024

		find_package(OpenXLSX CONFIG REQUIRED)
		target_link_libraries(main PRIVATE OpenXLSX::OpenXLSX)

		REF master
		SHA512 429215a961cfcfe8fcb49ee48970dd10dccad09a5c80db2eea2413120a51a2529c9a650584a079756c3b463d53f1fcdec46fdc158ffbc680592896867f60d3ea

[openxlsx] Add new port #41150

Are you sure you want to change the base?

[openxlsx] Add new port #41150

Conversation

motazmuhammad commented Sep 24, 2024 • edited Loading

motazmuhammad commented Sep 24, 2024

MonicaLiu0311 commented Sep 25, 2024

MonicaLiu0311 commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MonicaLiu0311 commented Sep 26, 2024

motazmuhammad commented Sep 27, 2024

MonicaLiu0311 commented Sep 29, 2024

motazmuhammad commented Sep 29, 2024

Choose a reason for hiding this comment

MonicaLiu0311 commented Sep 30, 2024 • edited Loading

motazmuhammad commented Sep 30, 2024

motazmuhammad commented Sep 30, 2024

dg0yt commented Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

motazmuhammad commented Sep 24, 2024 •

edited

Loading

MonicaLiu0311 commented Sep 30, 2024 •

edited

Loading

dg0yt commented Oct 1, 2024 •

edited

Loading