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 --- 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} };