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

[readline-unix] no absolute paths #22421

Merged

Conversation

autoantwort
Copy link
Contributor

For #20469

This port is 4 days old

@dg0yt
Copy link
Contributor

dg0yt commented Jan 8, 2022

Yeah, and before this one, readline-osx was rushed in with a custom build system, instead of simply using the original build system...

@autoantwort
Copy link
Contributor Author

From INSTALL:

`--with-curses'
This tells readline that it can find the termcap library functions
(tgetent, et al.) in the curses library, rather than a separate
termcap library. Readline uses the termcap functions, but does not
link with the termcap or curses library itself, allowing applications
which link with readline the to choose an appropriate library.
This option tells readline to link the example programs with the
curses library rather than libtermcap.

Seems that readline-unix should depend on libtermcap or curses

@dg0yt
Copy link
Contributor

dg0yt commented Jan 8, 2022

Yes. It depends on system curses on osx and linux.

@autoantwort
Copy link
Contributor Author

Hm but the readline.pc still references termcap instead of curses

@dg0yt
Copy link
Contributor

dg0yt commented Jan 8, 2022

Upstream bug?

@Cheney-W Cheney-W added the category:port-bug The issue is with a library, which is something the port should already support label Jan 10, 2022
@Cheney-W
Copy link
Contributor

Looks like below codes not working correctly, causing the default lib: ltermcap is used instead of lcurses.
readline-8.1\configure.ac:

AC_ARG_WITH(curses, AC_HELP_STRING([--with-curses], [use the curses library instead of the termcap library]), opt_curses=$withval)

if test "$opt_curses" = "yes"; then
	prefer_curses=yes
fi

@Cheney-W
Copy link
Contributor

@autoantwort Could you please help solve this issue?

@autoantwort
Copy link
Contributor Author

@autoantwort Could you please help solve this issue?

I also have read through the configure files and do not really understand why libcurses is not selected, but I don't want to debug it :D

@PhoebeHui
Copy link
Contributor

From INSTALL:

`--with-curses'
This tells readline that it can find the termcap library functions
(tgetent, et al.) in the curses library, rather than a separate
termcap library. Readline uses the termcap functions, but does not
link with the termcap or curses library itself, allowing applications
which link with readline the to choose an appropriate library.
This option tells readline to link the example programs with the
curses library rather than libtermcap.

Seems that readline-unix should depend on libtermcap or curses

vcpkg_configure_make(
SOURCE_PATH "${SOURCE_PATH}"
OPTIOS

Should be:

vcpkg_configure_make(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS

@Cheney-W
Copy link
Contributor

After modified the OPTIONS keyword, --with-curses option could be passed successfully, but the installation still failed with following error:

CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:78 (message):
  /usr/bin/pkg-config --exists readline failed with error code: 1
      ENV{PKG_CONFIG_PATH}: "/home/vxincwa/vcpkg/vcpkg/packages/readline-unix_x64-linux/lib/pkgconfig:/ed/x64-linux/lib/pkgconfig:/home/vxincwa/vcpkg/vcpkg/installed/x64-linux/share/pkgconfig"
      output: Package ncurses was not found in the pkg-config search path.
  Perhaps you should add the directory containing `ncurses.pc'
  to the PKG_CONFIG_PATH environment variable

@autoantwort
Copy link
Contributor Author

vcpkg_configure_make(
SOURCE_PATH "${SOURCE_PATH}"
OPTIOS

That should be an error. At least a warning.

@Cheney-W
Copy link
Contributor

Yes, we will improve the check function for these keywords later.

@autoantwort
Copy link
Contributor Author

Is btw ready

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jan 21, 2022
@strega-nil-ms
Copy link
Contributor

Thanks!

@strega-nil-ms strega-nil-ms merged commit 6fdcbc5 into microsoft:master Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants