[pacman-dev] [PATCH 1/2] Refactor _alpm_runscriptlet()
Add an is_archive parameter to reduce the amount of black magic going on. Rework to use fewer PATH_MAX sized local variables, and simplify some of the logic where appropriate in both this function and in the callers where duplicate calls can be replaced by some conditional parameter code. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/add.c | 48 +++++++++++++++++--------------------------- lib/libalpm/remove.c | 4 +- lib/libalpm/trans.c | 53 +++++++++++++++++++++++++++++++++----------------- lib/libalpm/trans.h | 4 +- 4 files changed, 58 insertions(+), 51 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 170d09e..33b85c0 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -452,17 +452,13 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, size_t pkg_current, size_t pkg_count) { int i, ret = 0, errors = 0; - char scriptlet[PATH_MAX]; - int is_upgrade = 0; + int is_upgrade; alpm_pkg_t *oldpkg = NULL; alpm_db_t *db = handle->db_local; alpm_trans_t *trans = handle->trans; ASSERT(trans != NULL, return -1); - snprintf(scriptlet, PATH_MAX, "%s%s-%s/install", - _alpm_db_path(db), newpkg->name, newpkg->version); - /* see if this is an upgrade. if so, remove the old package first */ alpm_pkg_t *local = _alpm_db_get_pkgfromcache(db, newpkg->name); if(local) { @@ -474,30 +470,23 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, goto cleanup; } - EVENT(handle, ALPM_EVENT_UPGRADE_START, newpkg, local); - _alpm_log(handle, ALPM_LOG_DEBUG, "upgrading package %s-%s\n", - newpkg->name, newpkg->version); - /* copy over the install reason */ newpkg->reason = alpm_pkg_get_reason(local); - /* pre_upgrade scriptlet */ - if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - _alpm_runscriptlet(handle, newpkg->origin_data.file, - "pre_upgrade", newpkg->version, local->version); - } + EVENT(handle, ALPM_EVENT_UPGRADE_START, newpkg, local); } else { is_upgrade = 0; - EVENT(handle, ALPM_EVENT_ADD_START, newpkg, NULL); - _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s-%s\n", - newpkg->name, newpkg->version); + } - /* pre_install scriptlet */ - if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - _alpm_runscriptlet(handle, newpkg->origin_data.file, - "pre_install", newpkg->version, NULL); - } + _alpm_log(handle, ALPM_LOG_DEBUG, "%s package %s-%s\n", + is_upgrade ? "upgrading" : "adding", newpkg->name, newpkg->version); + /* pre_install/pre_upgrade scriptlet */ + if(alpm_pkg_has_scriptlet(newpkg) && + !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { + const char *scriptlet_name = is_upgrade ? "pre_upgrade" : "pre_install"; + _alpm_runscriptlet(handle, newpkg->origin_data.file, + scriptlet_name, newpkg->version, NULL, 1); } /* we override any pre-set reason if we have alldeps or allexplicit set */ @@ -661,13 +650,14 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, /* run the post-install script if it exists */ if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - if(is_upgrade) { - _alpm_runscriptlet(handle, scriptlet, "post_upgrade", - newpkg->version, oldpkg ? oldpkg->version : NULL); - } else { - _alpm_runscriptlet(handle, scriptlet, "post_install", - newpkg->version, NULL); - } + char scriptlet[PATH_MAX]; + const char *scriptlet_name; + snprintf(scriptlet, PATH_MAX, "%s%s-%s/install", + _alpm_db_path(db), newpkg->name, newpkg->version); + scriptlet_name = is_upgrade ? "post_upgrade" : "post_install"; + + _alpm_runscriptlet(handle, scriptlet, scriptlet_name, + newpkg->version, oldpkg ? oldpkg->version : NULL, 0); } if(is_upgrade) { diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index a3aa4a5..7f917fa 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -376,7 +376,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle, /* run the pre-remove scriptlet if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL); + _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); } } @@ -453,7 +453,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle, /* run the post-remove script if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL); + _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); } } diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index fbc5fee..cb97a4a 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -275,40 +275,50 @@ static int grep(const char *fn, const char *needle) return 0; } -int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn, - const char *script, const char *ver, const char *oldver) +int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath, + const char *script, const char *ver, const char *oldver, int is_archive) { - char scriptfn[PATH_MAX]; char cmdline[PATH_MAX]; - char tmpdir[PATH_MAX]; char *argv[] = { "sh", "-c", cmdline, NULL }; - char *scriptpath; + char *tmpdir, *scriptfn = NULL, *scriptpath; int retval = 0; + size_t len; - if(_alpm_access(handle, NULL, installfn, R_OK) != 0) { - _alpm_log(handle, ALPM_LOG_DEBUG, "scriptlet '%s' not found\n", installfn); + if(_alpm_access(handle, NULL, filepath, R_OK) != 0) { + _alpm_log(handle, ALPM_LOG_DEBUG, "scriptlet '%s' not found\n", filepath); return 0; } - /* creates a directory in $root/tmp/ for copying/extracting the scriptlet */ - snprintf(tmpdir, PATH_MAX, "%stmp/", handle->root); + if(!is_archive && !grep(filepath, script)) { + /* script not found in scriptlet file; we can only short-circuit this early + * if it is an actual scriptlet file and not an archive. */ + return 0; + } + + /* create a directory in $root/tmp/ for copying/extracting the scriptlet */ + len = strlen(handle->root) + strlen("tmp/alpm_XXXXXX") + 1; + MALLOC(tmpdir, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + snprintf(tmpdir, len, "%stmp/", handle->root); if(access(tmpdir, F_OK) != 0) { _alpm_makepath_mode(tmpdir, 01777); } - snprintf(tmpdir, PATH_MAX, "%stmp/alpm_XXXXXX", handle->root); + snprintf(tmpdir, len, "%stmp/alpm_XXXXXX", handle->root); if(mkdtemp(tmpdir) == NULL) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not create temp directory\n")); + free(tmpdir); return 1; } /* either extract or copy the scriptlet */ - snprintf(scriptfn, PATH_MAX, "%s/.INSTALL", tmpdir); - if(strcmp(script, "pre_upgrade") == 0 || strcmp(script, "pre_install") == 0) { - if(_alpm_unpack_single(handle, installfn, tmpdir, ".INSTALL")) { + len += strlen("/.INSTALL"); + MALLOC(scriptfn, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + snprintf(scriptfn, len, "%s/.INSTALL", tmpdir); + if(is_archive) { + if(_alpm_unpack_single(handle, filepath, tmpdir, ".INSTALL")) { retval = 1; } } else { - if(_alpm_copyfile(installfn, scriptfn)) { + if(_alpm_copyfile(filepath, scriptfn)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not copy tempfile to %s (%s)\n"), scriptfn, strerror(errno)); retval = 1; } @@ -317,8 +327,8 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn, goto cleanup; } - if(!grep(scriptfn, script)) { - /* script not found in scriptlet file */ + if(is_archive && !grep(scriptfn, script)) { + /* script not found in extracted scriptlet file */ goto cleanup; } @@ -338,10 +348,17 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn, retval = _alpm_run_chroot(handle, "/bin/sh", argv); cleanup: - if(_alpm_rmrf(tmpdir)) { - _alpm_log(handle, ALPM_LOG_WARNING, _("could not remove tmpdir %s\n"), tmpdir); + if(scriptfn && unlink(scriptfn)) { + _alpm_log(handle, ALPM_LOG_WARNING, + _("could not remove %s\n"), scriptfn); + } + if(rmdir(tmpdir)) { + _alpm_log(handle, ALPM_LOG_WARNING, + _("could not remove tmpdir %s\n"), tmpdir); } + free(scriptfn); + free(tmpdir); return retval; } diff --git a/lib/libalpm/trans.h b/lib/libalpm/trans.h index 38f45ff..ff944b0 100644 --- a/lib/libalpm/trans.h +++ b/lib/libalpm/trans.h @@ -47,8 +47,8 @@ struct __alpm_trans_t { void _alpm_trans_free(alpm_trans_t *trans); int _alpm_trans_init(alpm_trans_t *trans, alpm_transflag_t flags); -int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn, - const char *script, const char *ver, const char *oldver); +int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath, + const char *script, const char *ver, const char *oldver, int is_archive); #endif /* _ALPM_TRANS_H */ -- 1.7.6.4
Expose the current static get_pkgpath() function internally to the rest of the library as _alpm_local_db_pkgpath(). This allows use of this convenience function in add.c and remove.c when forming the path to the scriptlet location. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/add.c | 8 +++----- lib/libalpm/be_local.c | 19 ++++++++++--------- lib/libalpm/db.h | 1 + lib/libalpm/remove.c | 10 ++++++---- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 33b85c0..653d2bd 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -650,14 +650,12 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, /* run the post-install script if it exists */ if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - char scriptlet[PATH_MAX]; - const char *scriptlet_name; - snprintf(scriptlet, PATH_MAX, "%s%s-%s/install", - _alpm_db_path(db), newpkg->name, newpkg->version); - scriptlet_name = is_upgrade ? "post_upgrade" : "post_install"; + char *scriptlet = _alpm_local_db_pkgpath(db, newpkg, "install"); + const char *scriptlet_name = is_upgrade ? "post_upgrade" : "post_install"; _alpm_runscriptlet(handle, scriptlet, scriptlet_name, newpkg->version, oldpkg ? oldpkg->version : NULL, 0); + free(scriptlet); } if(is_upgrade) { diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 109aaaf..88ecec0 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -475,7 +475,7 @@ static int local_db_populate(alpm_db_t *db) } /* Note: the return value must be freed by the caller */ -static char *get_pkgpath(alpm_db_t *db, alpm_pkg_t *info, const char *filename) +char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, const char *filename) { size_t len; char *pkgpath; @@ -544,7 +544,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) _alpm_log(db->handle, ALPM_LOG_FUNCTION, "loading package data for %s : level=0x%x\n", info->name, inforeq); - pkgpath = get_pkgpath(db, info, NULL); + pkgpath = _alpm_local_db_pkgpath(db, info, NULL); if(!pkgpath || access(pkgpath, F_OK)) { /* directory doesn't exist or can't be opened */ _alpm_log(db->handle, ALPM_LOG_DEBUG, "cannot find '%s-%s' in db '%s'\n", @@ -558,7 +558,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) /* DESC */ if(inforeq & INFRQ_DESC && !(info->infolevel & INFRQ_DESC)) { - char *path = get_pkgpath(db, info, "desc"); + char *path = _alpm_local_db_pkgpath(db, info, "desc"); if(!path || (fp = fopen(path, "r")) == NULL) { _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); free(path); @@ -628,7 +628,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) /* FILES */ if(inforeq & INFRQ_FILES && !(info->infolevel & INFRQ_FILES)) { - char *path = get_pkgpath(db, info, "files"); + char *path = _alpm_local_db_pkgpath(db, info, "files"); if(!path || (fp = fopen(path, "r")) == NULL) { _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); free(path); @@ -685,7 +685,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) /* INSTALL */ if(inforeq & INFRQ_SCRIPTLET && !(info->infolevel & INFRQ_SCRIPTLET)) { - char *path = get_pkgpath(db, info, "install"); + char *path = _alpm_local_db_pkgpath(db, info, "install"); if(access(path, F_OK) == 0) { info->scriptlet = 1; } @@ -714,7 +714,7 @@ int _alpm_local_db_prepare(alpm_db_t *db, alpm_pkg_t *info) } oldmask = umask(0000); - pkgpath = get_pkgpath(db, info, NULL); + pkgpath = _alpm_local_db_pkgpath(db, info, NULL); if((retval = mkdir(pkgpath, 0755)) != 0) { _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not create directory %s: %s\n"), @@ -746,7 +746,7 @@ int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq char *path; _alpm_log(db->handle, ALPM_LOG_DEBUG, "writing %s-%s DESC information back to db\n", info->name, info->version); - path = get_pkgpath(db, info, "desc"); + path = _alpm_local_db_pkgpath(db, info, "desc"); if(!path || (fp = fopen(path, "w")) == NULL) { _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); @@ -857,7 +857,7 @@ int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq char *path; _alpm_log(db->handle, ALPM_LOG_DEBUG, "writing %s-%s FILES information back to db\n", info->name, info->version); - path = get_pkgpath(db, info, "files"); + path = _alpm_local_db_pkgpath(db, info, "files"); if(!path || (fp = fopen(path, "w")) == NULL) { _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); @@ -903,8 +903,9 @@ cleanup: int _alpm_local_db_remove(alpm_db_t *db, alpm_pkg_t *info) { int ret = 0; - char *pkgpath = get_pkgpath(db, info, NULL); + char *pkgpath = _alpm_local_db_pkgpath(db, info, NULL); + /* TODO explicit file removes and then an rmdir? */ ret = _alpm_rmrf(pkgpath); free(pkgpath); if(ret != 0) { diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 88f6c68..224bfbe 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -93,6 +93,7 @@ void _alpm_db_unregister(alpm_db_t *db); int _alpm_local_db_prepare(alpm_db_t *db, alpm_pkg_t *info); int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq); int _alpm_local_db_remove(alpm_db_t *db, alpm_pkg_t *info); +char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, const char *filename); /* cache bullshit */ /* packages */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 7f917fa..44f3ee9 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -359,7 +359,6 @@ int _alpm_remove_single_package(alpm_handle_t *handle, const char *pkgname = oldpkg->name; const char *pkgver = oldpkg->version; alpm_filelist_t *filelist; - char scriptlet[PATH_MAX]; size_t i; if(newpkg) { @@ -370,13 +369,13 @@ int _alpm_remove_single_package(alpm_handle_t *handle, _alpm_log(handle, ALPM_LOG_DEBUG, "removing package %s-%s\n", pkgname, pkgver); - snprintf(scriptlet, PATH_MAX, "%s%s-%s/install", - _alpm_db_path(handle->db_local), pkgname, pkgver); - /* run the pre-remove scriptlet if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { + char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, + oldpkg, "install"); _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); + free(scriptlet); } } @@ -453,7 +452,10 @@ int _alpm_remove_single_package(alpm_handle_t *handle, /* run the post-remove script if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { + char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, + oldpkg, "install"); _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); + free(scriptlet); } } -- 1.7.6.4
participants (1)
-
Dan McGee