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

feat: append ${ZPFX}/lib/pkg-config to $PKG_CONFIG_PATH #333

Closed
wants to merge 2 commits into from

Conversation

psprint
Copy link
Contributor

@psprint psprint commented Jul 7, 2022

Description

I thought it be good if $ZPFX will get visible to autotools and cmake. In order to accomplish this $ZPFX/lib/pkg-config should be added to $PKG_CONFIG_PATH.

Motivation and Context

Today I've installed a library (pcre2-8) to $ZPFX and then tried to also install a dependent program (uni-ctags) and I've noticed that the library wasn't found. After investigation it was pkg-config to blame, with a solution of prepending $ZPFX library dir to $PKG_CONFIG_PATH. i think that such move is of good reasoning – the variable works like $PATH grouping multiple dirs, so nothing can be lost. After this ctags built correctly.

Usage examples

Some can be found at #332

How Has This Been Tested?

i've removed system pcre2, obtained a build failure, performed the steps described and obtained a build success.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions github-actions bot added the zinit label Jul 7, 2022
@vladdoster vladdoster changed the title Add $ZPFX/lib/pkg-config to $PKG_CONFIG_PATH so that libs are found. [maint]: append "${ZPFX}/lib/pkg-config" to "${PKG_CONFIG_PATH}" Jul 15, 2022
zinit.zsh Outdated Show resolved Hide resolved
@psprint psprint force-pushed the setup-pkg_config_path branch from e6e7a64 to db6e8ce Compare July 16, 2022 14:22
@psprint
Copy link
Contributor Author

psprint commented Jul 16, 2022

I've added the quotes.

PS. I've clicked something so that the review requests were generated, you can ignore them.

@psprint
Copy link
Contributor Author

psprint commented Jul 19, 2022

I wonder if some other environment variables are there that make CMake and AutoTools look more closely at a changed --prefix path when searching for the required dependencies? CMake can have some I suppose.

@psprint
Copy link
Contributor Author

psprint commented Jul 19, 2022

@vladdoster in commit b1da9b2 I've added support for $ZPFX also to:

  • CMake (add $ZPFX to $CMAKE_PREFIX_PATH),
  • Autotools (add a $ZPFX/share/config.site file which sets up CPP…/LDFLAGS but only (useful feature) if --prefix is pointing to $ZPFX)
  • $LD_LIBRARY_PATH to include $ZPFX/lib so that libraries are found when running a program.

The first two changes make CMake and Autotools find libraries installed in ZPFX.

Also, an extra hunk in the commit – typeset -gU PATH path – make $PATH unique.

@psprint psprint force-pushed the setup-pkg_config_path branch 3 times, most recently from 0e4843c to ae31528 Compare July 20, 2022 06:01
@psprint psprint closed this Aug 13, 2022
@psprint psprint reopened this Sep 8, 2022
@psprint
Copy link
Contributor Author

psprint commented Sep 10, 2022

@vladdoster: Chances to merge?

@psprint psprint force-pushed the setup-pkg_config_path branch 2 times, most recently from be94a0e to 92b7475 Compare September 29, 2022 14:55
@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

I've resolved conflicts.

@psprint
Copy link
Contributor Author

psprint commented Oct 31, 2022

Hey could the PR be merged? It is a simple change to make libraries found.

@psprint
Copy link
Contributor Author

psprint commented Dec 27, 2022

When will it be merged, @vladdoster ?

@vladdoster
Copy link
Member

vladdoster commented Jan 16, 2023

@psprint,

The make and configure ices don't really work currently, IIRC.

Using $ZPFX as the prefix (i.e., ./configure --prefix=$ZPFX or make PREFIX=$ZPFX makes command not work correctly:

zi delete <program>

Example

The following is cleaned up correctly:

zi delete shc --yes
zi for \
    null \
    id-as \
    lman \
    lbin'!bin/shc' \
    configure"--prefix=$ZI[PLUGINS_DIR]/shc" \
    make"PREFIX=$ZI[PLUGINS_DIR]/shc install" \
  @neurobin/shc

The following is not cleaned up and seems to install man page in wrong place:

zi delete shc --yes
zi for \
    null \
    id-as \
    lman \
    lbin'!$ZPFX/bin/shc' \
    configure"--prefix=$ZPFX" \
    make"PREFIX=$ZPFX install" \
  @neurobin/shc

Am I missing something?

@psprint
Copy link
Contributor Author

psprint commented Jan 17, 2023

@vladdoster When you set prefix to the dir of the plugin then it's natural that its files get cleaned up when deleting the plugin. To obtain similar effect zi delete could detect uninstall target in Makefile and run it before deleting plugin dir (that contain the Makefile, but not plugin installed files).

@psprint psprint force-pushed the setup-pkg_config_path branch from 9369ade to bea1af4 Compare January 17, 2023 12:00
@psprint
Copy link
Contributor Author

psprint commented Jan 17, 2023

@vladdoster I can see that you like to install to $PWD (plugin dir) instead of $ZPFX. That's an interesting approach that has to be yet better recognized. For example libraries when installed to PWD will not be found, unless a way of adding the local directories in PWD to some variables (don't know which) will be found. I can however see the approach working for some single-binary programs like bat, fd. For now, I'll add make uninstall to zi delete to make $ZPFX installations more clean.

@psprint psprint force-pushed the setup-pkg_config_path branch from 269bf8f to 1524c3a Compare January 17, 2023 12:36
@psprint
Copy link
Contributor Author

psprint commented Jan 17, 2023

@vladdoster I've switched from tying with -T to a regular [[ $var != *$ZPFX* ]] check (little more sophisticated in the source), no -U flags is being set and no stray array vars like pkg_cfg are left over.

@psprint psprint force-pushed the setup-pkg_config_path branch from 1524c3a to 778e1fb Compare January 17, 2023 12:43
@psprint
Copy link
Contributor Author

psprint commented Jan 17, 2023

@vladdoster Here's info on config.site, it's a cool mechanism that makes libraries and includes found https://www.paxswill.com/autotools-trick-config-dot-site/

@psprint
Copy link
Contributor Author

psprint commented Jan 29, 2023

@vladdoster: I added in #468 make uninstall run to zinit delete … command. Basically, if the uninstall target is there in the Makefile of the removed plugin/snippet, it'll be run as: make -C {plugin/snippet dir} uninstall before removing the dir.

@psprint
Copy link
Contributor Author

psprint commented Jan 31, 2023

@vladdoster come on, could the PR be merged quicker?

@psprint psprint force-pushed the setup-pkg_config_path branch from a1ef8f3 to 7dbc663 Compare February 17, 2023 10:02
@psprint
Copy link
Contributor Author

psprint commented Feb 17, 2023

I've found a bug – CMake var should use ; not : to separate dirs. Could the PR be merged?

@psprint
Copy link
Contributor Author

psprint commented Feb 17, 2023

@vladdoster See documentation of CMAKE_PREFIX_PATH: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html#variable:CMAKE_PREFIX_PATH

It is perfect for what is needed to make $ZPFSX more discoverable.

@vladdoster vladdoster changed the title [maint]: append "${ZPFX}/lib/pkg-config" to "${PKG_CONFIG_PATH}" feat: append ${ZPFX}/lib/pkg-config to ${PKG_CONFIG_PATH} Feb 17, 2023
@vladdoster
Copy link
Member

@psprint,

I'm not purposefully ignoring your PRs. There are failing checks that need to be resolved (i.e., commit message). It is how the CHANGELOG is generated and makes it easier to decipher commits when searching through the git log.

It clearly states what the issues are.

Screenshot 2023-02-17 at 04 56 16

I opened an empty PR which passes the linter. Feel free to use it and amend your commit messages.

@vladdoster vladdoster changed the title feat: append ${ZPFX}/lib/pkg-config to ${PKG_CONFIG_PATH} feat: append ${ZPFX}/lib/pkg-config to $PKG_CONFIG_PATH Feb 17, 2023
@psprint psprint force-pushed the setup-pkg_config_path branch 3 times, most recently from b4bd023 to ce28e8c Compare February 17, 2023 11:37
@psprint psprint force-pushed the setup-pkg_config_path branch from ce28e8c to e307952 Compare February 17, 2023 11:38
@psprint
Copy link
Contributor Author

psprint commented Feb 17, 2023

OK, thanks for pointing where to look for info, I've made the commit message pass linter.

@psprint psprint force-pushed the setup-pkg_config_path branch 3 times, most recently from 3d7a462 to 9826031 Compare February 20, 2023 11:56
@psprint
Copy link
Contributor Author

psprint commented Mar 29, 2023

Ping?

@vladdoster
Copy link
Member

vladdoster commented Apr 4, 2023

@psprint,

I'm not purposefully ignoring your PRs. There are failing checks that need to be resolved (i.e., commit message). It is how the CHANGELOG is generated and makes it easier to decipher commits when searching through the git log.

@psprint,

Please see my previous comment.

@psprint
Copy link
Contributor Author

psprint commented Apr 4, 2023

@vladdoster the problem is that the unit tests are failing randomly and that I cannot use docker on my PCLInuxOS to generate documentation. You've once wrote that the documentation isn't a problem …

@psprint
Copy link
Contributor Author

psprint commented Apr 25, 2023

@vladdoster: It buggs me that one still cannot compile with deps from $ZPFX. The unit test failures are accident, not related to the changes. So after generating zshelldoc docs one could merge the PR?

@psprint psprint force-pushed the setup-pkg_config_path branch from 9826031 to 3650fe1 Compare April 29, 2023 17:44
@psprint
Copy link
Contributor Author

psprint commented Apr 29, 2023

@vladdoster: I've found one other bug (: instead of ;) and fixed it. The code is now without any errors, could it be merged? It opens $ZPFX installed libs to new software installed via zinit.

@psprint
Copy link
Contributor Author

psprint commented Jul 18, 2023

What about this PR? It is important.

@psprint
Copy link
Contributor Author

psprint commented Jul 26, 2023

ping :) The unit test failure is some random one:

__zunit_tmp_test_function:17: exec format error: /Users/runner/work/zinit/zinit/tests/_support/tmp.TlWR1eUBGc/polaris/bin/git-mkver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants