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

Implement "print-search-dirs" argument #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

falhumai96
Copy link

@falhumai96 falhumai96 commented Mar 7, 2022

Implement print-search-dirs argument to list (currently) install "binary" script and the search libraries (required by libtool when building shared libraries (https://git.savannah.gnu.org/cgit/libtool.git/tree/m4/libtool.m4?id=33615a45a6926666f95da8a0dc28fc9a8c62d117#n2320)). This argument requires the presence of a Windows <-> Unix path tool to be present to be used to convert, for e.g., LIB environment variable with Windows libraries search paths to its equivalent in the Unix system it is running on (e.g. Cygwin with native Windows or Linux with Wine). It can be either guessed from the current Unix system or set using the CCCL_WINPATH_BIN environment variable.

The operations of print-search-dirs require the manipulation of Windows paths, namely separating the Windows path prefix from the path, to shorten the Unix version of the path, and parsing the Windows path prefix from the path is a bit tedious. So, I relied on Python's pathlib's PureWindowsPath (and also PurePosixPath for manipulating Unix paths) to resolve this issue without implementing this functionality in pure Bash scripting. However, I would have to add a Python requirement to the script as well, with CCCL_PYTHON_BIN environment variable to set it to a custom Python installation path or resolved from the PATH environment variable. Though recent versions of Linux or Cygwin installation already comes with Python (even native Windows Python should work in this case), and Python is being as available by default as Perl. Also, the library I require, which is pathlib, is already part of the standard library of recent Python versions.

…ntly) "install" "binary" script and the search "libraries" (required by "libtool" when building shared libraries).
cccl Outdated
# the Windows drive letter is present in the path. For instance, a path with "c:" in Wine will be converted
# to a path with "drive_c". The reason for avoiding paths with ":" is that ":" is generally unescapable in Unix
# (e.g. in "PATH" environment variable).
unix_global_library_path="$(realpath $($winpath_bin -u $libpath))"
Copy link
Author

Choose a reason for hiding this comment

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

I don't like to use realpath here, but it is the only way I can think of to remove the : from the Wine drives, for e.g.

I believe this argument is mostly necessary for libtool to search for libraries when creating dynamic libraries, and the issue when going to the parent folder (i.e. ../) won't happen, since only the directories in libraries will be searched.

Copy link
Author

Choose a reason for hiding this comment

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

I resolved this issue by shortening only the Unix portion of the Windows path prefix, by parsing out the prefix using Python's pathlib's PureWindowsPath.

…r custom Python path, where the Python installation must have "pathlib" installed. Update the logic for resolving the short version of the Unix path of the Windows library directory by only shortening the prefix of the Unix version of the Windows path, using Python's "pathlib"'s "PureWindowsPath" to extract the Windows path prefix from the path. Use "winpath" to convert the Unix paths in arguments to Windows paths before sending them to Windows command arguments (e.g. in "-L" or "-I" arguments). Perform other minor code cleanups.
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.

1 participant