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

Add MAKE_DIRECTORY and RESULT to file() #38

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

petk
Copy link
Contributor

@petk petk commented Oct 15, 2024

I'm not sure if the current behavior is intentional and correct me if I'm wrong, but I think that according to other patterns this would be a bit more consistent: MAKE_DIRECTORY is a multi-value keyword, RESULT is a one-value keyword.

Similar to other multi-value keywords in other commands.

MAKE_DIRECTORY is a multi-value keyword, RESULT is a one-value keyword.
@BlankSpruce
Copy link
Owner

It seems that file(MAKE_DIRECTORY) change isn't present in CMake 3.31 release notes:
https://cmake.org/cmake/help/latest/release/3.31.html#commands
even though RESULT is marked as new in 3.31:
https://cmake.org/cmake/help/latest/command/file.html#make-directory

I'll try to check later in CMake's repo if this is meant to land in 3.31 (since it's still in release candidate state) and if it does then your interpretation of how it should be treated is correct.

@BlankSpruce
Copy link
Owner

BlankSpruce commented Oct 15, 2024

Addendum: since all invocations in the form file(FIRST_ARGUMENT ...) are in some way just differently spelled out imaginary commands file_first_argument (like file(MAKE_DIRECTORY ...) === file_make_directory(...) I've treated the list of arguments as a not keyworded sequence of arguments unless there are "actual" other keywords like this new RESULT. If I recall correctly install also suffers from that with one of multiple signatures selected through FIRST_ARGUMENT.

@BlankSpruce
Copy link
Owner

It looks like accidental omission so I opened an issue in CMake's repo: https://gitlab.kitware.com/cmake/cmake/-/issues/26377
Once it's confirmed I'll merge your change along with missing install(PACKAGE_INFO) and then release 0.16.2.

@petk
Copy link
Contributor Author

petk commented Oct 15, 2024

I've also opened a PR here to add the new RESULT option: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9904

@BlankSpruce BlankSpruce merged commit 06c798d into BlankSpruce:master Oct 15, 2024
8 checks passed
@petk petk deleted the patch-make-directory branch October 15, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants