[pacman-dev] Make bash-completion optional

Message ID DB7PR10MB2508023E5DAFA233F6AAC16BD2A50@DB7PR10MB2508.EURPRD10.PROD.OUTLOOK.COM
State Rejected, archived
Headers show
Series [pacman-dev] Make bash-completion optional | expand

Commit Message

Wouter Wijsman May 7, 2020, 6:45 p.m. UTC
The bash completion files were the only reason pacman was not able to
build as a non-root user. This patch adds the option to not install
these files. This was needed for my use case, hopefully upstream someone
else has a use for it as well.

Signed-off-by: Wouter Wijsman <wwijsman@live.nl>
---
 meson_options.txt   |  3 +++
 scripts/meson.build | 44 +++++++++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 21 deletions(-)

Comments

Eli Schwartz May 7, 2020, 7:21 p.m. UTC | #1
On 5/7/20 2:45 PM, Wouter Wijsman wrote:
> The bash completion files were the only reason pacman was not able to
> build as a non-root user. This patch adds the option to not install
> these files. This was needed for my use case, hopefully upstream someone
> else has a use for it as well.

The problem should only happen during install, not during build. I
presume that it is trying to install to e.g.
/usr/share/bash-completion/completions/pacman and raising a PermissionError?

To find the correct install dir for the completions, it first tries to
look at pkg-config to see where pkg-config says they should be
installed. It then falls back on --datadir and installs to
${datadir}/bash-completion/completions/

If you have bash-completion installed and its pkg-config file points at
a root-owned location you'll need root to install pacman. If you don't
have bash-completion installed, then your custom prefix might not
actually be picked up by bash-completion.

I wonder what the right solution is here. In autotools land, ./configure
--help has this to say:

Some influential environment variables:
[...]
  bashcompdir value of completionsdir for bash-completion, overriding
              pkg-config


This is part of the builtin functionality of the pkg.m4 macros which
autotools is using. meson doesn't directly support anything like this.
Wouter Wijsman May 7, 2020, 7:40 p.m. UTC | #2
On Thu, 2020-05-07 at 15:21 -0400, Eli Schwartz wrote:
> The problem should only happen during install, not during build. I
> presume that it is trying to install to e.g.
> /usr/share/bash-completion/completions/pacman and raising a
> PermissionError?

That is true, my bad, it only fails upon installation and only if you
deny it root access. /usr/share/bash-completion is used regardless of
prefix for both build systems, so I assumed that was intended,
especially since it wouldn't work if it was in any other place.

Kind regards,
Wouter Wijsman
Allan McRae May 8, 2020, 1:32 a.m. UTC | #3
On 8/5/20 5:40 am, wwijsman@live.nl wrote:
> On Thu, 2020-05-07 at 15:21 -0400, Eli Schwartz wrote:
>> The problem should only happen during install, not during build. I
>> presume that it is trying to install to e.g.
>> /usr/share/bash-completion/completions/pacman and raising a
>> PermissionError?
> 
> That is true, my bad, it only fails upon installation and only if you
> deny it root access. /usr/share/bash-completion is used regardless of
> prefix for both build systems, so I assumed that was intended,
> especially since it wouldn't work if it was in any other place.
> 

So the real error here that --prefix is ignored for bash-completion?

It would be better to solve the problem than to work around it.

A

Patch

diff --git a/meson_options.txt b/meson_options.txt
index 4d8cc300..2927cd15 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -51,6 +51,9 @@  option('gpgme', type : 'feature', value : 'auto',
 option('i18n', type : 'boolean', value : true,
        description : 'enable localization of pacman, libalpm and scripts')
 
+option('bash-completion', type : 'boolean', value : true,
+       description : 'install bash-completion files')
+
 # tools
 option('file-seccomp', type: 'feature', value: 'auto',
 	   description: 'determine whether file is seccomp-enabled')
diff --git a/scripts/meson.build b/scripts/meson.build
index d2466523..12262e98 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -68,25 +68,27 @@  configure_file(
 	output : '@BASENAME@',
 	install_dir : join_paths(DATAROOTDIR, 'pkgconfig'))
 
-custom_target(
-  'bash_completion',
-  command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@' ],
-  input : 'completion/bash_completion.in',
-  output : 'pacman',
-  install : true,
-  install_dir : BASHCOMPDIR)
-
-foreach symlink : ['pacman-key', 'makepkg']
-  meson.add_install_script(MESON_MAKE_SYMLINK,
-                           'pacman',
-                           join_paths(BASHCOMPDIR, symlink))
-endforeach
+if get_option('bash-completion')
+  custom_target(
+    'bash_completion',
+    command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@' ],
+    input : 'completion/bash_completion.in',
+    output : 'pacman',
+    install : true,
+    install_dir : BASHCOMPDIR)
 
-zsh_completion_dir = join_paths(DATAROOTDIR, 'zsh/site-functions')
-custom_target(
-  'zsh_completion',
-  command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@' ],
-  input : 'completion/zsh_completion.in',
-  output : '_pacman',
-  install : true,
-  install_dir : zsh_completion_dir)
+  foreach symlink : ['pacman-key', 'makepkg']
+    meson.add_install_script(MESON_MAKE_SYMLINK,
+                             'pacman',
+                             join_paths(BASHCOMPDIR, symlink))
+  endforeach
+
+  zsh_completion_dir = join_paths(DATAROOTDIR, 'zsh/site-functions')
+  custom_target(
+    'zsh_completion',
+    command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@' ],
+    input : 'completion/zsh_completion.in',
+    output : '_pacman',
+    install : true,
+    install_dir : zsh_completion_dir)
+endif