On Sun, Jul 15, 2012 at 05:33:34PM +1000, Allan McRae wrote: > On 06/07/12 21:33, Alex Chamberlain wrote: > > Hi all, > > This is my first time submitting a patch to any open source project, > > so I hope I've done it it right. Apologies in advance for anything I > > have done wrong! > > Great! New contributors are always appreciated. Fairly minor comments > inlined below. >
As I mentioned to Alex in IRC, there's also a lack of a manpage addition to go with this patch. > > Below is a small patch adding an additional command-line flag to pacman, > > which prevents ld-config from being run by libalpm. I was having > > problems installing packages to a loop-mounted ARM image and I think > > preventing ldconfig from running is the best solution. > > > > Kind Regards, > > > > Alex Chamberlain > > If you put all this below the "---" line after the Signoff-off-by, then > it does not affect the commit. > > > I'll start with the comment that even with --noldconfig, all your > install scriptlets probably fail running too. So not running ldconfig > is only one of the issues to solve. > > Did running ldconfig in this situation actually cause pacman have > issues. A very brief look at the code indicates that the call will fail > but pacman will solder on. So I am not sure what this is solving. > > Saying that, preventing ldconfig running is probably a needed step in > allowing pacman to install a system with different architecture, so I > see value in this patch. > > > > Add the --no-ldconfig flag to the command line to prevent > > ldconfig from being run. It is useful for installing > > packages onto a loop- mounted image. > > All the other --no* options have no "-" in the middle so you should be > consistent and go with --noldconfig. > > > Signed-off-by: Alex Chamberlain <[email protected]> > > --- > > lib/libalpm/alpm.h | 3 +++ > > lib/libalpm/handle.c | 14 ++++++++++++++ > > lib/libalpm/handle.h | 1 + > > lib/libalpm/util.c | 23 ++++++++++++----------- > > src/pacman/conf.c | 4 ++++ > > src/pacman/conf.h | 5 ++++- > > src/pacman/pacman.c | 5 +++++ > > 7 files changed, 43 insertions(+), 12 deletions(-) > > > > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h > > index bb792cf..a8fdd11 100644 > > --- a/lib/libalpm/alpm.h > > +++ b/lib/libalpm/alpm.h > > @@ -547,6 +547,9 @@ int alpm_option_set_deltaratio(alpm_handle_t *handle, > > double ratio); > > int alpm_option_get_checkspace(alpm_handle_t *handle); > > int alpm_option_set_checkspace(alpm_handle_t *handle, int checkspace); > > > > +int alpm_option_get_ldconfig(alpm_handle_t *handle); > > +int alpm_option_set_ldconfig(alpm_handle_t *handle, int ldconfig); > > + > > alpm_siglevel_t alpm_option_get_default_siglevel(alpm_handle_t *handle); > > int alpm_option_set_default_siglevel(alpm_handle_t *handle, > > alpm_siglevel_t level); > > > > diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c > > index 816162b..e126421 100644 > > --- a/lib/libalpm/handle.c > > +++ b/lib/libalpm/handle.c > > @@ -44,6 +44,7 @@ alpm_handle_t *_alpm_handle_new(void) > > > > CALLOC(handle, 1, sizeof(alpm_handle_t), return NULL); > > handle->deltaratio = 0.0; > > + handle->ldconfig = 1; > > > > return handle; > > } > > @@ -265,6 +266,12 @@ int SYMEXPORT alpm_option_get_checkspace(alpm_handle_t > > *handle) > > return handle->checkspace; > > } > > > > +int SYMEXPORT alpm_option_get_ldconfig(alpm_handle_t *handle) > > +{ > > + CHECK_HANDLE(handle, return -1); > > + return handle->ldconfig; > > +} > > + > > int SYMEXPORT alpm_option_set_logcb(alpm_handle_t *handle, alpm_cb_log cb) > > { > > CHECK_HANDLE(handle, return -1); > > @@ -600,6 +607,13 @@ int SYMEXPORT alpm_option_set_checkspace(alpm_handle_t > > *handle, int checkspace) > > return 0; > > } > > > > +int SYMEXPORT alpm_option_set_ldconfig(alpm_handle_t *handle, int ldconfig) > > +{ > > + CHECK_HANDLE(handle, return -1); > > + handle->ldconfig = ldconfig; > > + return 0; > > +} > > + > > int SYMEXPORT alpm_option_set_default_siglevel(alpm_handle_t *handle, > > alpm_siglevel_t level) > > { > > diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h > > index c0ad668..cccdf12 100644 > > --- a/lib/libalpm/handle.h > > +++ b/lib/libalpm/handle.h > > @@ -91,6 +91,7 @@ struct __alpm_handle_t { > > double deltaratio; /* Download deltas if possible; a ratio value > > */ > > int usesyslog; /* Use syslog instead of logfile? */ /* TODO > > move to frontend */ > > int checkspace; /* Check disk space before installing */ > > + int ldconfig; /* Should we run ldconfig? Default: 1 */ > > alpm_siglevel_t siglevel; /* Default signature verification level */ > > > > /* error code */ > > diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c > > index c2b5d44..3b1ee6e 100644 > > --- a/lib/libalpm/util.c > > +++ b/lib/libalpm/util.c > > @@ -616,17 +616,18 @@ cleanup: > > int _alpm_ldconfig(alpm_handle_t *handle) > > { > > char line[PATH_MAX]; > > - > > - _alpm_log(handle, ALPM_LOG_DEBUG, "running ldconfig\n"); > > - > > - snprintf(line, PATH_MAX, "%setc/ld.so.conf", handle->root); > > - if(access(line, F_OK) == 0) { > > - snprintf(line, PATH_MAX, "%ssbin/ldconfig", handle->root); > > - if(access(line, X_OK) == 0) { > > - char arg0[32]; > > - char *argv[] = { arg0, NULL }; > > - strcpy(arg0, "ldconfig"); > > - return _alpm_run_chroot(handle, "/sbin/ldconfig", argv); > > + if(handle->ldconfig) { > > whitespace issues... > > > + _alpm_log(handle, ALPM_LOG_DEBUG, "running ldconfig\n"); > > + > > + snprintf(line, PATH_MAX, "%setc/ld.so.conf", handle->root); > > + if(access(line, F_OK) == 0) { > > + snprintf(line, PATH_MAX, "%ssbin/ldconfig", > > handle->root); > > + if(access(line, X_OK) == 0) { > > + char arg0[32]; > > + char *argv[] = { arg0, NULL }; > > + strcpy(arg0, "ldconfig"); > > + return _alpm_run_chroot(handle, > > "/sbin/ldconfig", argv); > > + } > > } > > } > > > > diff --git a/src/pacman/conf.c b/src/pacman/conf.c > > index 4aaacb5..094c141 100644 > > --- a/src/pacman/conf.c > > +++ b/src/pacman/conf.c > > @@ -58,6 +58,8 @@ config_t *config_new(void) > > ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; > > } > > > > + newconfig->ldconfig = 1; > > + > > I'd put that above the if block above it. (being picky!) > > > return newconfig; > > } > > > > @@ -622,6 +624,8 @@ static int setup_libalpm(void) > > alpm_option_set_noupgrades(handle, config->noupgrade); > > alpm_option_set_noextracts(handle, config->noextract); > > > > + alpm_option_set_ldconfig(handle, config->ldconfig); > > + > > return 0; > > } > > > > diff --git a/src/pacman/conf.h b/src/pacman/conf.h > > index 69c955e..389cfa3 100644 > > --- a/src/pacman/conf.h > > +++ b/src/pacman/conf.h > > @@ -95,6 +95,8 @@ typedef struct __config_t { > > > > alpm_list_t *explicit_adds; > > alpm_list_t *explicit_removes; > > + > > + int ldconfig; /* Should ldconfig be called? Default: 1 */ > > } config_t; > > > > /* Operations */ > > @@ -127,7 +129,8 @@ enum { > > OP_PRINTFORMAT, > > OP_GPGDIR, > > OP_DBONLY, > > - OP_FORCE > > + OP_FORCE, > > + OP_NO_LDCONFIG > > OP_NOLDCONFIG <- none of the other options have _ separating the words. > > > }; > > > > /* clean method */ > > diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c > > index 73d5be9..23af6bc 100644 > > --- a/src/pacman/pacman.c > > +++ b/src/pacman/pacman.c > > @@ -201,6 +201,7 @@ static void usage(int op, const char * const myname) > > addlist(_(" --gpgdir <path> set an alternate home > > directory for GnuPG\n")); > > addlist(_(" --logfile <path> set an alternate log > > file\n")); > > addlist(_(" --noconfirm do not ask for any > > confirmation\n")); > > + addlist(_(" --no-ldconfig do not run ldconfig\n")); > > } > > list = alpm_list_msort(list, alpm_list_count(list), options_cmp); > > for(i = list; i; i = alpm_list_next(i)) { > > @@ -436,6 +437,9 @@ static int parsearg_global(int opt) > > break; > > case 'r': check_optarg(); config->rootdir = strdup(optarg); > > break; > > case 'v': (config->verbose)++; break; > > This should only be added to parsearg_trans as it is used for -S, -R, > and -U operations. > > > + case OP_NO_LDCONFIG: > > + config->ldconfig = 0; > > + break; > > default: return 1; > > } > > return 0; > > @@ -628,6 +632,7 @@ static int parseargs(int argc, char *argv[]) > > {"print-format", required_argument, 0, OP_PRINTFORMAT}, > > {"gpgdir", required_argument, 0, OP_GPGDIR}, > > {"dbonly", no_argument, 0, OP_DBONLY}, > > + {"no-ldconfig", no_argument, 0, OP_NO_LDCONFIG}, > > {0, 0, 0, 0} > > }; > > > > > > >
