[pacman-dev] [PATCH 0/4] add pacman-conf utility
Parsing pacman.conf is so hard that even our own scripts don't do it correctly. pacman-conf is a dedicated utility scripts can use to correctly parse values from pacman.conf. This is a conversion of an earlier version of pacconf from pacutils that has been relicensed and renamed to avoid a file conflict. I make no promises that I've correctly adjusted the automake configuration for this type of build, but it works for me. Andrew Gregory (4): extract raw config file parser extract default settings to separate function add pacman-conf utility use pacman-conf in scripts scripts/completion/zsh_completion.in | 4 +- scripts/pacman-db-upgrade.sh.in | 21 +- scripts/pacman-key.sh.in | 2 +- src/pacman/conf.c | 179 +++++++------- src/pacman/conf.h | 2 + src/util/.gitignore | 2 + src/util/Makefile.am | 21 +- src/util/pacman-conf.c | 437 +++++++++++++++++++++++++++++++++++ 8 files changed, 562 insertions(+), 106 deletions(-) create mode 100644 src/util/pacman-conf.c -- 2.15.1
To allow pacman-conf to parse the configuration file without having to
also setup alpm.
Signed-off-by: Andrew Gregory
Default values for configuration settings were being set during alpm
setup and in some cases were never saved back to the original config
struct. Refactoring all default settings into a separate function and
saving them onto the original config struct will allow pacman-conf to
resolve the defaults without having to setup alpm.
Signed-off-by: Andrew Gregory
Parsing pacman's configuration file is non-trivial and extremely
difficult to do correctly from scripts; even our own do it incorrectly.
pacman-conf is a dedicated tool specifically to allow scripts to parse
config files, getting the same value that pacman itself would use.
Signed-off-by: Andrew Gregory
On 14/01/18 02:49, Andrew Gregory wrote:
Parsing pacman's configuration file is non-trivial and extremely difficult to do correctly from scripts; even our own do it incorrectly. pacman-conf is a dedicated tool specifically to allow scripts to parse config files, getting the same value that pacman itself would use.
Signed-off-by: Andrew Gregory
The automake changes resulted in a big warning that is fixed by enabling subdir-objects in configure.ac and ignoring the .dirstamp file it creates. There was also a directory being created in the src/util with the name '$(top_srcdir)'. This is due to using a variable in the paths (no idea why this is an issue), but replacing it with a relative path "fixes" the issue. I'll squash the following into your patch:
From 4c3d7964da2ba4276e4464920fb795e9f973e3ec Mon Sep 17 00:00:00 2001 From: Allan McRae
Date: Thu, 18 Jan 2018 14:54:17 +1000 Subject: [PATCH] [SQUASH] Fix automake usage for pacman-conf
Signed-off-by: Allan McRae
On 18/01/18 14:59, Allan McRae wrote:
On 14/01/18 02:49, Andrew Gregory wrote:
Parsing pacman's configuration file is non-trivial and extremely difficult to do correctly from scripts; even our own do it incorrectly. pacman-conf is a dedicated tool specifically to allow scripts to parse config files, getting the same value that pacman itself would use.
Signed-off-by: Andrew Gregory
The automake changes resulted in a big warning that is fixed by enabling subdir-objects in configure.ac and ignoring the .dirstamp file it creates.
Crap... that breaks "make distcheck". More specifically "make distclean" as it tries to remove the generated object files for the common files twice.
There was also a directory being created in the src/util with the name '$(top_srcdir)'. This is due to using a variable in the paths (no idea why this is an issue), but replacing it with a relative path "fixes" the issue.
I'll squash the following into your patch:
From 4c3d7964da2ba4276e4464920fb795e9f973e3ec Mon Sep 17 00:00:00 2001 From: Allan McRae
Date: Thu, 18 Jan 2018 14:54:17 +1000 Subject: [PATCH] [SQUASH] Fix automake usage for pacman-conf Signed-off-by: Allan McRae
--- .gitignore | 1 + configure.ac | 2 +- src/util/Makefile.am | 20 ++++++++++---------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 499d499b..7399a120 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ *~ *.o +.dirstamp ABOUT-NLS aclocal.m4 autom4te.cache diff --git a/configure.ac b/configure.ac index 86f5bb6e..02afba83 100644 --- a/configure.ac +++ b/configure.ac @@ -60,7 +60,7 @@ AC_CONFIG_AUX_DIR([build-aux]) AC_REQUIRE_AUX_FILE([tap-driver.sh])
AC_CANONICAL_HOST -AM_INIT_AUTOMAKE([1.11 foreign]) +AM_INIT_AUTOMAKE([1.11 foreign subdir-objects]) AM_SILENT_RULES([yes])
LT_INIT diff --git a/src/util/Makefile.am b/src/util/Makefile.am index aa812b99..84598ea0 100644 --- a/src/util/Makefile.am +++ b/src/util/Makefile.am @@ -26,16 +26,16 @@ cleanupdelta_SOURCES = cleanupdelta.c cleanupdelta_LDADD = $(top_builddir)/lib/libalpm/.libs/libalpm.la
pacman_conf_SOURCES = pacman-conf.c \ - $(top_srcdir)/src/pacman/util.h \ - $(top_srcdir)/src/pacman/util.c \ - $(top_srcdir)/src/pacman/ini.h \ - $(top_srcdir)/src/pacman/ini.c \ - $(top_srcdir)/src/pacman/util-common.h \ - $(top_srcdir)/src/pacman/util-common.c \ - $(top_srcdir)/src/pacman/callback.h \ - $(top_srcdir)/src/pacman/callback.c \ - $(top_srcdir)/src/pacman/conf.h \ - $(top_srcdir)/src/pacman/conf.c + ../pacman/util.h \ + ../pacman/util.c \ + ../pacman/ini.h \ + ../pacman/ini.c \ + ../pacman/util-common.h \ + ../pacman/util-common.c \ + ../pacman/callback.h \ + ../pacman/callback.c \ + ../pacman/conf.h \ + ../pacman/conf.c pacman_conf_LDADD = $(top_builddir)/lib/libalpm/.libs/libalpm.la
testpkg_SOURCES = testpkg.c
On 18/01/18 15:29, Allan McRae wrote:
On 18/01/18 14:59, Allan McRae wrote:
On 14/01/18 02:49, Andrew Gregory wrote:
Parsing pacman's configuration file is non-trivial and extremely difficult to do correctly from scripts; even our own do it incorrectly. pacman-conf is a dedicated tool specifically to allow scripts to parse config files, getting the same value that pacman itself would use.
Signed-off-by: Andrew Gregory
The automake changes resulted in a big warning that is fixed by enabling subdir-objects in configure.ac and ignoring the .dirstamp file it creates.
Crap... that breaks "make distcheck". More specifically "make distclean" as it tries to remove the generated object files for the common files twice.
Turns out automake with this subdir-objects option (which will become default next release) is buggy as hell. Some patches have landed upstream, but we can't rely on them in the pacman codebase. Options here.... 1) host pacman-conf in the src/pacman directory 2) Use the makefile to copy the needed src/pacman files into src/util/pacman-conf/... and they get compiled twice. 3) create a small library using the needed files and static link it into pacman-conf To be clear, I only included #3 as I saw several projects do this! I see #1 as the easiest. And probably most correct - the overlap in source used here is such that they should be located in the same place. Allan
Because parsing pacman.conf is so difficult that even we can't do it
right.
Signed-off-by: Andrew Gregory
On 01/13/2018 11:50 AM, Andrew Gregory wrote:
Because parsing pacman.conf is so difficult that even we can't do it right.
Signed-off-by: Andrew Gregory
--- scripts/completion/zsh_completion.in | 4 ++-- scripts/pacman-db-upgrade.sh.in | 21 ++------------------- scripts/pacman-key.sh.in | 2 +- 3 files changed, 5 insertions(+), 22 deletions(-)
Our bash completion should also be updated to use pacman-conf, as currently it uses a glorious hack to find the completions for -Sl which involves verbosely printing all packages ever and using `cut|sort -u` to extract repo names and dedupe them. Of course, this was still more accurate than the zsh config file parsing. I'd also like to see some way of listing group names for completing -Qg, I guess, although that isn't really something to solve via better *configuration* parsing, so it is probably offtopic for this. -- Eli Schwartz
On 14/01/18 09:21, Eli Schwartz wrote:
On 01/13/2018 11:50 AM, Andrew Gregory wrote:
Because parsing pacman.conf is so difficult that even we can't do it right.
Signed-off-by: Andrew Gregory
--- scripts/completion/zsh_completion.in | 4 ++-- scripts/pacman-db-upgrade.sh.in | 21 ++------------------- scripts/pacman-key.sh.in | 2 +- 3 files changed, 5 insertions(+), 22 deletions(-) Our bash completion should also be updated to use pacman-conf, as currently it uses a glorious hack to find the completions for -Sl which involves verbosely printing all packages ever and using `cut|sort -u` to extract repo names and dedupe them.
patches welcome.
On 14/01/18 02:49, Andrew Gregory wrote:
Parsing pacman.conf is so hard that even our own scripts don't do it correctly. pacman-conf is a dedicated utility scripts can use to correctly parse values from pacman.conf. This is a conversion of an earlier version of pacconf from pacutils that has been relicensed and renamed to avoid a file conflict. I make no promises that I've correctly adjusted the automake configuration for this type of build, but it works for me.
Thanks. I have had a quick look at these and I am very happy! My quick scan of the automake changes did not flag any issue, but I will do so in more detail before I commit the changes. (Reminder to myslef: make sure "make distcheck" does what it is supposed to.) We will need a man page (despite the --help output being complete). This will not prevent these patches being committed, but it will be a blocker for release. I assume Andrew has other things in the code base to look at, so this appears to be a "patches welcome" situation. Volunteers? Cheers, Allan
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Eli Schwartz