fix(cmake): parse add_paimon_lib link arguments. by slfan1989 · Pull Request #9 · apache/paimon-cpp · GitHub
Skip to content

fix(cmake): parse add_paimon_lib link arguments. - #9

Merged
leaves12138 merged 1 commit into
apache:mainfrom
slfan1989:paimon-cpp-8
May 25, 2026
Merged

fix(cmake): parse add_paimon_lib link arguments.#9
leaves12138 merged 1 commit into
apache:mainfrom
slfan1989:paimon-cpp-8

Conversation

@slfan1989

@slfan1989 slfan1989 commented May 23, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #8.

add_paimon_lib() uses several link-related ARG_* variables internally, but the corresponding keywords were not declared in cmake_parse_arguments().

This change adds the missing argument names:

  • STATIC_INSTALL_INTERFACE_LIBS
  • SHARED_INSTALL_INTERFACE_LIBS
  • SHARED_PRIVATE_LINK_LIBS

so these options are parsed correctly instead of leaking into preceding link argument lists.

Tests

API and Format

No.

Documentation

No.

Generative AI tooling

@slfan1989

slfan1989 commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

@zjw1111

zjw1111 commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks for fix! LGTM

@zjw1111

zjw1111 commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Could you remove the [PAIMON #8] from the title to maintain consistency with other commit formats?

@slfan1989 slfan1989 changed the title [PAIMON #8] fix(cmake): parse add_paimon_lib link arguments. fix(cmake): parse add_paimon_lib link arguments. May 25, 2026
@slfan1989

slfan1989 commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Could you remove the [PAIMON #8] from the title to maintain consistency with other commit formats?

@zjw1111 Thanks for the suggestion! I have updated the title and removed the [PAIMON #8] prefix to align with the existing commit title format.

@leaves12138

leaves12138 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Reviewed the change. This fixes add_paimon_lib by registering the link/install argument groups that are already referenced later in BuildUtils.cmake, so they are no longer treated as unparsed arguments by cmake_parse_arguments. The patch is small and consistent with the existing CMake helper logic. No blockers from my side.

@slfan1989

slfan1989 commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@zjw1111 @leaves12138 Thank you for reviewing the code. Would it be possible to accept this small improvement?

leaves12138
leaves12138 approved these changes May 25, 2026
View reviewed changes

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The patch correctly registers the existing link/install argument groups for add_paimon_lib and I do not see any blockers.

@leaves12138
leaves12138 merged commit 8bdf77f into apache:main May 25, 2026
@slfan1989

slfan1989 commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leaves12138 leaves12138 leaves12138 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] add_paimon_lib does not parse several link-related arguments

3 participants

@slfan1989 @zjw1111 @leaves12138