Skip to content

cmake: Enabling CURL_COMPLETION_FISH or _ZSH causes build to fail on 8.13.0 #16946

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

Closed
diizzyy opened this issue Apr 3, 2025 · 13 comments
Closed
Labels

Comments

@diizzyy
Copy link
Contributor

diizzyy commented Apr 3, 2025

CMake Error at scripts/CMakeLists.txt:72 (install):
  install FILES given directory "/usr/ports/ftp/curl/work/.build/scripts/" to
  install.

-- Configuring incomplete, errors occurred!
*** Error code 1

curl/libcurl version

curl 8.13.0

operating system

FreeBSD 14.2-RELEASE (amd64)

@vszakats

@diizzyy diizzyy changed the title cmake: Enabling CURL_COMPLETION_FISH or _ZSH causes build to fail cmake: Enabling CURL_COMPLETION_FISH or _ZSH causes build to fail on 8.13.0 Apr 3, 2025
@bagder bagder added the cmake label Apr 3, 2025
@vszakats
Copy link
Member

vszakats commented Apr 3, 2025

Thanks for trying it out.

Readily, I admit not much clue here. The error message doesn't help.

Can you perhaps print out the values of CURL_COMPLETION_FISH_DIR and CURL_COMPLETION_ZSH_DIR before the install() calls via:

message(STATUS "|${CURL_COMPLETION_FISH_DIR}|")
message(STATUS "|${CURL_COMPLETION_ZSH_DIR}|")

or call curl_dumpvars() (strip the output from private things in there if any before pasting here).

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 3, 2025

-- |share/fish/vendor_completions.d|
-- |share/zsh/site-functions|

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 3, 2025

Hmm....

I wonder if it's the Perl script that's causing issues

/usr/ports/ftp/curl/work/curl-8.13.0/scripts # ./completion.pl --opts_dir /usr/ports/ftp/curl/work/curl-8.13.0/docs/cmdline-opts --shell zsh > foo.txt
Unknown option: opts_dir
Usage:
    completion.pl [options...]

        --opts_dir path to cmdline-opts directory
        --shell    zsh/fish
        --help     prints this help

@vszakats
Copy link
Member

vszakats commented Apr 3, 2025

-- |share/fish/vendor_completions.d|
-- |share/zsh/site-functions|

Thanks. These look fine. (I don't have these installed, so could only do artificial tests around here.)

@dfandrich
Copy link
Contributor

dfandrich commented Apr 3, 2025 via email

dfandrich added a commit that referenced this issue Apr 3, 2025
The help text gave the wrong option name.

Reported-by: Daniel Engberg
Ref: #16946
@vszakats
Copy link
Member

vszakats commented Apr 3, 2025

Hmm....

I wonder if it's the Perl script that's causing issues

/usr/ports/ftp/curl/work/curl-8.13.0/scripts # ./completion.pl --opts_dir /usr/ports/ftp/curl/work/curl-8.13.0/docs/cmdline-opts --shell zsh > foo.txt
Unknown option: opts_dir
Usage:
    completion.pl [options...]

        --opts_dir path to cmdline-opts directory
        --shell    zsh/fish
        --help     prints this help

Interesting! The script calls for --opts-dir. How could it become --opts_dir?

But, the completion.pl help screen is wrong, should say --opts-dir. (fixing)

Also the initial error was in the install() line.

@vszakats
Copy link
Member

vszakats commented Apr 3, 2025

Mostly just a shot in the dark ...does this improve things for you?: #16954

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 4, 2025

Yes, that fixes the issue! Thanks!

@vszakats
Copy link
Member

vszakats commented Apr 4, 2025

Nice, thank you too!

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 18, 2025

@vszakats

Hi,

I apologize on my behalf as I seemingly was very tired testing it.
Enabling both ZSH and FISH works, enabling only one causes issues.

Having another look at the scripts/CMakeLists.txt it seems like it doesn't evaluate whether either option is enabled when defining the install section? This seems to work on my end.

https://github.com/curl/curl/blob/master/scripts/CMakeLists.txt#L55
if(CURL_COMPLETION_FISH AND NOT CURL_COMPLETION_FISH_DIR)

https://github.com/curl/curl/blob/master/scripts/CMakeLists.txt#L65
if(CURL_COMPLETION_ZSH AND NOT CURL_COMPLETION_ZSH_DIR)

..and another unrelated note,

include(GNUInstallDirs) in already accessed before scripts dir so there's no need to load it again?
https://github.com/curl/curl/blob/master/CMakeLists.txt#L2059
https://github.com/curl/curl/blob/master/CMakeLists.txt#L2080

Best regards,
Daniel

vszakats added a commit to vszakats/curl that referenced this issue Apr 18, 2025
Also:
- tidy up the if tree.
- drop `include(GNUInstallDirs)` in favor of the upper-level one.

Reported-by: Daniel Engberg
Bug: curl#16946 (comment)
vszakats added a commit to vszakats/curl that referenced this issue Apr 18, 2025
Also:
- tidy up the if tree.
- drop `include(GNUInstallDirs)` in favor of the upper-level one.

Reported-by: Daniel Engberg
Bug: curl#16946 (comment)
Follow-up to c8b0f0c curl#16833
@vszakats
Copy link
Member

No worries, thanks for your reports. Does it look better with?: #17094

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 18, 2025

Yes, tested with all possible combinations. Thanks!

vszakats added a commit that referenced this issue Apr 18, 2025
Also:
- tidy up the `if` tree.
- drop `include(GNUInstallDirs)` in favor of the upper-level one.

Reported-by: Daniel Engberg
Bug: #16946 (comment)
Follow-up to c8b0f0c #16833

Closes #17094
@vszakats
Copy link
Member

Thank you, and merged!

nbaws pushed a commit to nbaws/curl that referenced this issue Apr 26, 2025
Also:
- tidy up the `if` tree.
- drop `include(GNUInstallDirs)` in favor of the upper-level one.

Reported-by: Daniel Engberg
Bug: curl#16946 (comment)
Follow-up to c8b0f0c curl#16833

Closes curl#17094
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants