Conversation
|
This was already tried to be handled in #198 but it has gotten worse. |
inigomartinez
force-pushed
the
fix-pkg-config
branch
from
ce13823 to
42837bc
Compare
|
As stated in #71662, this approach solves the issue regarding setting variables present in the At the moment the pkg-config file is generated with absolute paths on it and this doesn't allow the use of the define-variable option present in pkg-config program. As also stated in the previous MR, to be able to do this, The MR has been rebased with newer changes to ease its application. |
scop
left a comment
scop
left a comment
There was a problem hiding this comment.
Sorry for having overlooked this. I find it ugly that some systems are (ab)using these variables to do staged installs (for autotools there's DESTDIR which is a better solution to the problem), but I suppose that bird's already out of the cage and even autotools' distcheck does something similar itself, see e.g. jpmens/jo#102
So I suppose we need to cater for those usage patterns. The patch looks ok in principle, but as now we're no longer using pkgdatadir everywhere, I think we should not use it anywhere. After your patch, there are still references to it in these files:
- Makefile.am
- completions/Makefile.am
- helpers/Makefile.am
|
Oh, and I "borrowed" the |
inigomartinez
force-pushed
the
fix-pkg-config
branch
from
42837bc to
bc7f2b8
Compare
|
@scop thank you for reviewing this. I have rebased again the branch with the latest changes.
This looks sensible to me. I have checked the "allow maintainers" check and is enabled so you will be able to also made these changes. |
|
Sure, I'll quite likely get to it sometime, but I believe you'd beat me to it ;) |
inigomartinez
force-pushed
the
fix-pkg-config
branch
from
bc7f2b8 to
f88580d
Compare
|
I have split the changes in two commits. The first one applies the original changes and the second one replaces |
| set (BASH_COMPLETION_HELPERSDIR "@pkgdatadir@/helpers") | ||
| set (BASH_COMPLETION_DATADIR "@datadir@") | ||
| set (BASH_COMPLETION_SYSCONFDIR "@sysconfdir@") | ||
|
|
There was a problem hiding this comment.
I wonder if this does what it intends to do. It introduces BASH_COMPLETION_DATADIR and BASH_COMPLETION_SYSCONFDIR, but they're not used for anything that I can tell, one cannot use them to override install locations because the definitions below don't use them; they have similarly hardwired/expanded paths as they used to.
Could you clarify the intent here?
There was a problem hiding this comment.
Totally my fault sorry due to my lack of knowledge of cmake. I thought that it was related to the bash-completion.pc.in file and that it also needed to have these two variables defined. I have removed these two lines from the commit.
inigomartinez
force-pushed
the
fix-pkg-config
branch
from
f88580d to
c1a3032
Compare
Define our completion and helper dirs using the standard data and sysconf dirs. Enables 3rd party package staged installs by overriding these two dirs, in addition to the already existing mechanisms.
`pkgdatadir` has been replaced in some cases when adding support for variable redefinition in `pkg-config` files. These changes removes all use of `pkgdatadir` to add consistency.
|
Applied, thanks. I took the liberty to modify the extended commentary of the first commit, because it used wording like "proper" which kind of suggests that users are expected to set them which isn't the case except in some specific scenarios. |
It is a good practice to install files relative to our own variables https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/ and it is required on systems like NixOS. Thanks to Iñigo, bash-completion support that since 2.10: scop/bash-completion#344
It is a good practice to install files relative to our own variables https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/ and it is required on systems like NixOS. Thanks to Iñigo, bash-completion support that since 2.10: scop/bash-completion#344 --- https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/1194
It is a good practice to install files relative to our own variables https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/ and it is required on systems like NixOS. Thanks to Iñigo, bash-completion support that since 2.10: scop/bash-completion#344 --- https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/1194
Since bash-completion 2.9, it was no longer possible to override the completionsdir through prefix. [1] In 2.10, the overridability was re-estabilished but this time through datadir variable. [2] This should not really matter except for developers installing the project into a custom prefix or distros using per-package prefixes like NixOS. [1]: scop/bash-completion@81ba2c7 [2]: scop/bash-completion#344 Co-Authored-By: Iñigo Martínez <[email protected]>
Since bash-completion 2.9, it was no longer possible to override the completionsdir through prefix. [1] In 2.10, the overridability was re-estabilished but this time through datadir variable. [2] This should not really matter except for developers installing the project into a custom prefix or distros using per-package prefixes like NixOS. [1]: scop/bash-completion@81ba2c7 [2]: scop/bash-completion#344
Since bash-completion 2.9, it was no longer possible to override the completionsdir through prefix. [1] In 2.10, the overridability was re-estabilished but this time through datadir variable. [2] This should not really matter except for developers installing the project into a custom prefix or distros using per-package prefixes like NixOS. [1]: scop/bash-completion@81ba2c7 [2]: scop/bash-completion#344
2 participants
Using
pkg-configoptions the user has the possibility to redefine key values present in the.pcfiles.Due to this
compatdir,completionsdirandhelpersdirthat are defined in the generated pkg-config.pcfile should be relative paths.This helps any pkg-config user to set these variables to proper directories by using the
define-variablecommand line argument.