Skip to content

[cconfig] adding cconfig lib to VCPKG #47480

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.

Sign up for GitHub

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

Jump to bottom
Merged
JavierMatosD merged 26 commits into from
Sep 25, 2025
Merged

[cconfig] adding cconfig lib to VCPKG #47480

JavierMatosD merged 26 commits into from
Sep 25, 2025

Conversation

@Nouridin
Copy link
Contributor

@Nouridin Nouridin commented Sep 24, 2025

  • Changes comply with the maintainer guide.
  • The name of the port matches an existing name on https://repology.org/ or is strongly associated with the component (new port, so may be reviewed).
  • Optional dependencies are resolved in exactly one way. Header-only, no dependencies.
  • The versioning scheme in vcpkg.json matches upstream (1.0.0).
  • The license declaration in vcpkg.json matches upstream (MIT).
  • Installed as the "copyright" file matches upstream (LICENSE copied).
  • Source code of the component installed comes from an authoritative source (GitHub repo).
  • Generated "usage text" is accurate. Minimal header-only instructions provided.
  • Version database is fixed by rerunning ./vcpkg x-add-version --all.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@JavierMatosD
Copy link
Contributor

JavierMatosD commented Sep 24, 2025

@Nouridin thank you for the contribution.

I'm placing this PR in draft while the build errors get resolved. Additionally, I recommend taking a look at our tutorial for packaging libraries for vcpkg -> https://learn.microsoft.com/vcpkg/get_started/get-started-packaging?pivots=shell-bash

I noticed you are adding a test port. Please use https://github.com/microsoft/vcpkg/tree/master/scripts/test_ports/vcpkg-ci-plplot for reference on how to add a test port for vcpkg.

Thanks again!

@JavierMatosD JavierMatosD marked this pull request as draft September 24, 2025 15:26
@Nouridin Nouridin marked this pull request as ready for review September 24, 2025 19:25
@Nouridin
Copy link
Contributor Author

Nouridin commented Sep 24, 2025

your Windows-x86 tests have consumed my mental capacity

JavierMatosD
JavierMatosD reviewed Sep 24, 2025
View reviewed changes
JavierMatosD
JavierMatosD requested changes Sep 24, 2025
View reviewed changes
Copy link
Contributor

@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

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

Please move the test project to a vcpkg-ci-cconfig port and here is an example of a header only port in the registry

vcpkg/ports/commata/portfile.cmake

Line 4 in 8006ed5

REPO furfurylic/commata

@JavierMatosD JavierMatosD marked this pull request as draft September 24, 2025 19:37
@Nouridin
Copy link
Contributor Author

Nouridin commented Sep 24, 2025 •
edited
Loading

@JavierMatosD Hey, all platform jobs passed except arm64_osx, which did not start in the pipeline (it appears skipped/not scheduled for this fork PR).

image

dg0yt
dg0yt reviewed Sep 25, 2025
View reviewed changes
ports/cconfig/portfile.cmake Outdated
@@ -0,0 +1,19 @@
# Fetch source from GitHub
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
# Fetch source from GitHub

ports/cconfig/portfile.cmake Outdated
SHA512 8CE0C0FCA4E55AF9CFD56BA7779F4775703752D328518FE72F242336A7D4DB08B53284CA6148FC65BDBFE7D5BE4F025F49DFC7B13A45E2B69F350E15966C1929
)

# Copy header to include/cconfig/
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
# Copy header to include/cconfig/

ports/cconfig/portfile.cmake Outdated
file(MAKE_DIRECTORY "${CURRENT_PACKAGES_DIR}/include/cconfig")
file(COPY "${SOURCE_PATH}/cconfig.h" DESTINATION "${CURRENT_PACKAGES_DIR}/include/cconfig")

# Install license using the vcpkg helper (required)
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
# Install license using the vcpkg helper (required)

ports/cconfig/portfile.cmake Outdated
Comment on lines 15 to 19

# Remove auto-generated usage folder if present
if(EXISTS "${CURRENT_PACKAGES_DIR}/share/${PORT}/usage")
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/share/${PORT}/usage")
endif()
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
# Remove auto-generated usage folder if present
if(EXISTS "${CURRENT_PACKAGES_DIR}/share/${PORT}/usage")
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/share/${PORT}/usage")
endif()

No such thing. AI slop?

Copy link
Contributor Author

@Nouridin Nouridin Sep 25, 2025

Choose a reason for hiding this comment

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

yes this is AI slop because windows-x86 drove me crazy

ports/cconfig/project/CMakeLists.txt Outdated
Comment on lines 1 to 18
cmake_minimum_required(VERSION 3.10)
project(cconfig-test C)

# Use the include path installed by vcpkg
find_path(CCONFIG_INCLUDE_DIRS "cconfig.h"
PATH_SUFFIXES "cconfig"
HINTS "${CMAKE_CURRENT_LIST_DIR}/../include"
)

if(NOT CCONFIG_INCLUDE_DIRS)
message(FATAL_ERROR "Could not find cconfig.h in the include path")
endif()

# Create a test executable
add_executable(cconfig-test test.c)

# Add include directories
target_include_directories(cconfig-test PRIVATE ${CCONFIG_INCLUDE_DIRS})
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Doesn't belong to this port.

ports/cconfig/project/test.c Outdated
Comment on lines 1 to 7
#include <stdio.h>
#include "cconfig.h"

int main() {
printf("cconfig header included successfully.\n");
return 0;
}
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Doesn't belong to this port.

ports/cconfig/vcpkg.json Outdated
"name": "cconfig",
"version": "1.0.0",
"description": "Header-only config parsing library",
"homepage": "https://github.com/Nouridin/cconfig"
Copy link
Contributor

@dg0yt dg0yt Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
"homepage": "https://github.com/Nouridin/cconfig"
"homepage": "https://github.com/Nouridin/cconfig",
"license": "<add SPDX term here>" | null

@Nouridin Nouridin marked this pull request as ready for review September 25, 2025 09:38
@Nouridin
Copy link
Contributor Author

Nouridin commented Sep 25, 2025

@JavierMatosD I think everything is done now.

JavierMatosD
JavierMatosD approved these changes Sep 25, 2025
View reviewed changes
Copy link
Contributor

@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

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

@Nouridin thank you for the contribution and @dg0yt thank you for helping with this review.

@JavierMatosD JavierMatosD merged commit 484e266 into microsoft:master Sep 25, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JavierMatosD JavierMatosD JavierMatosD approved these changes

+1 more reviewer

@dg0yt dg0yt dg0yt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nouridin @JavierMatosD @dg0yt