[pacman-dev] [PATCH 0/7] [RFC] extract_single_file cleanup
This is my attempt to clean up the behemoth extract_single_file function. In order to do this I have made the following user-visible changes that I would like feedback on: * skip mtree, noupgrade, noextract checks for db files * remove .pacorig support, use .pacnew instead * extract backup files directly to .pacnew instead of .paccheck This takes extract_single_file down from 340 lines to 225 and considerably reduces the number of nested conditionals Andrew Gregory (7): extract_single_file: consolidate needbackup checks extract_single_file: reduce indentation extract_single_file: factor out db file extraction extract_single_file: use full path in messages remove support for .pacorig files extract_single_file: use .pacnew for check files extract_single_file: consolidate extraction logic contrib/README | 2 +- contrib/pacdiff.sh.in | 9 +- doc/pacman.8.txt | 5 +- lib/libalpm/add.c | 413 ++++++++++++++++------------------------ lib/libalpm/alpm.h | 15 +- src/pacman/callback.c | 16 -- test/pacman/README | 1 - test/pacman/pmrule.py | 3 - test/pacman/tests/upgrade015.py | 1 - test/pacman/tests/upgrade016.py | 5 +- test/pacman/tests/upgrade027.py | 1 - test/pacman/tests/upgrade028.py | 1 - test/pacman/tests/upgrade029.py | 1 - 13 files changed, 171 insertions(+), 302 deletions(-) -- 2.1.1
We need to know if a file needs to be backed up for all extracted files.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
We need to know if a file needs to be backed up for all extracted files.
Ack.
Signed-off-by: Andrew Gregory
--- lib/libalpm/add.c | 53 ++++++++++++++--------------------------------------- 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index bbf2a51..ca029b2 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -147,16 +147,15 @@ static int try_rename(alpm_handle_t *handle, const char *src, const char *dest) static int extract_single_file(alpm_handle_t *handle, struct archive *archive, struct archive_entry *entry, alpm_pkg_t *newpkg, alpm_pkg_t *oldpkg) { - const char *entryname; - mode_t entrymode; + const char *entryname = archive_entry_pathname(entry); + mode_t entrymode = archive_entry_mode(entry); + alpm_backup_t *backup = _alpm_needbackup(entryname, newpkg); char filename[PATH_MAX]; /* the actual file we're extracting */ int needbackup = 0, notouch = 0; const char *hash_orig = NULL; char *entryname_orig = NULL; int errors = 0; - - entryname = archive_entry_pathname(entry); - entrymode = archive_entry_mode(entry); + struct stat lsbuf;
if(strcmp(entryname, ".INSTALL") == 0) { /* the install script goes inside the db */
OK
@@ -216,7 +215,6 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, * 6- skip extraction, dir already exists. */
- struct stat lsbuf; if(llstat(filename, &lsbuf) != 0) { /* cases 1,2: file doesn't exist, skip all backup checks */ } else {
OK
@@ -269,21 +267,13 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, if(_alpm_fnmatch_patterns(handle->noupgrade, entryname) == 0) { notouch = 1; } else { - alpm_backup_t *backup; - /* go to the backup array and see if our conflict is there */ - /* check newpkg first, so that adding backup files is retroactive */ - backup = _alpm_needbackup(entryname, newpkg); - if(backup) { + alpm_backup_t *oldbackup; + if(oldpkg && (oldbackup = _alpm_needbackup(entryname, oldpkg))) { + hash_orig = oldbackup->hash; + needbackup = 1; + } else if(backup) { + /* allow adding backup files retroactively */ needbackup = 1; - } - - /* check oldpkg for a backup entry, store the hash if available */ - if(oldpkg) { - backup = _alpm_needbackup(entryname, oldpkg); - if(backup) { - hash_orig = backup->hash; - needbackup = 1; - } } } }
OK
@@ -312,16 +302,9 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, hash_pkg = alpm_compute_md5sum(checkfile);
/* update the md5 hash in newpkg's backup (it will be the new original) */ - alpm_list_t *i; - for(i = alpm_pkg_get_backup(newpkg); i; i = i->next) { - alpm_backup_t *backup = i->data; - char *newhash; - if(!backup->name || strcmp(backup->name, entryname_orig) != 0) { - continue; - } - STRDUP(newhash, hash_pkg, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + if(backup) { FREE(backup->hash); - backup->hash = newhash; + STRDUP(backup->hash, hash_pkg, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); }
_alpm_log(handle, ALPM_LOG_DEBUG, "checking hashes for %s\n", entryname_orig);
OK
@@ -468,17 +451,9 @@ needbackup_cleanup: }
/* calculate an hash if this is in newpkg's backup */ - alpm_list_t *i; - for(i = alpm_pkg_get_backup(newpkg); i; i = i->next) { - alpm_backup_t *backup = i->data; - char *newhash; - if(!backup->name || strcmp(backup->name, entryname_orig) != 0) { - continue; - } - _alpm_log(handle, ALPM_LOG_DEBUG, "appending backup entry for %s\n", entryname_orig); - newhash = alpm_compute_md5sum(filename); + if(backup) { FREE(backup->hash); - backup->hash = newhash; + backup->hash = alpm_compute_md5sum(filename); } } free(entryname_orig);
OK.
Puts all of the conflict cases at the same level.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
Puts all of the conflict cases at the same level.
Signed-off-by: Andrew Gregory
--- This is almost entirely whitespace reduction; the actual changes are much more visible with --ignore-space-change.
Thanks for the hint! Ack.
alpm's database files (.INSTALL, .MTREE, etc.) should be extracted no
matter what; skip mtree/needbackup/noextract/noupgrade checks for them.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
alpm's database files (.INSTALL, .MTREE, etc.) should be extracted no matter what; skip mtree/needbackup/noextract/noupgrade checks for them.
Signed-off-by: Andrew Gregory
Ack.
--- lib/libalpm/add.c | 60 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index a03af82..e5f5d42 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -144,6 +144,29 @@ static int try_rename(alpm_handle_t *handle, const char *src, const char *dest) return 0; }
+static int extract_db_file(alpm_handle_t *handle, struct archive *archive, + struct archive_entry *entry, alpm_pkg_t *newpkg, const char *entryname) +{ + char filename[PATH_MAX]; /* the actual file we're extracting */ + const char *dbfile; + if(strcmp(entryname, ".INSTALL") == 0) { + dbfile = "install"; + } else if(strcmp(entryname, ".CHANGELOG") == 0) { + dbfile = "changelog"; + } else if(strcmp(entryname, ".MTREE") == 0) { + dbfile = "mtree"; + } else if(*entryname == '.') { + /* reserve all files starting with '.' for future possibilities */ + _alpm_log(handle, ALPM_LOG_DEBUG, "skipping extraction of '%s'\n", entryname); + archive_read_data_skip(archive); + return 0; + } + archive_entry_set_perm(entry, 0644); + snprintf(filename, PATH_MAX, "%s%s-%s/%s", + _alpm_db_path(handle->db_local), newpkg->name, newpkg->version, dbfile); + return perform_extraction(handle, archive, entry, filename, filename); +} +
OK
static int extract_single_file(alpm_handle_t *handle, struct archive *archive, struct archive_entry *entry, alpm_pkg_t *newpkg, alpm_pkg_t *oldpkg) { @@ -157,36 +180,17 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, int errors = 0; struct stat lsbuf;
- if(strcmp(entryname, ".INSTALL") == 0) { - /* the install script goes inside the db */ - snprintf(filename, PATH_MAX, "%s%s-%s/install", - _alpm_db_path(handle->db_local), newpkg->name, newpkg->version); - archive_entry_set_perm(entry, 0644); - } else if(strcmp(entryname, ".CHANGELOG") == 0) { - /* the changelog goes inside the db */ - snprintf(filename, PATH_MAX, "%s%s-%s/changelog", - _alpm_db_path(handle->db_local), newpkg->name, newpkg->version); - archive_entry_set_perm(entry, 0644); - } else if(strcmp(entryname, ".MTREE") == 0) { - /* the mtree file goes inside the db */ - snprintf(filename, PATH_MAX, "%s%s-%s/mtree", - _alpm_db_path(handle->db_local), newpkg->name, newpkg->version); - archive_entry_set_perm(entry, 0644); - } else if(*entryname == '.') { - /* for now, ignore all files starting with '.' that haven't - * already been handled (for future possibilities) */ - _alpm_log(handle, ALPM_LOG_DEBUG, "skipping extraction of '%s'\n", entryname); - archive_read_data_skip(archive); + if(*entryname == '.') { + return extract_db_file(handle, archive, entry, newpkg, entryname); + } +
OK
+ if (!alpm_filelist_contains(&newpkg->files, entryname)) { + _alpm_log(handle, ALPM_LOG_WARNING, _("file not found in file list for package %s. skipping extraction of %s\n"), + newpkg->name, entryname); return 0; - } else { - if (!alpm_filelist_contains(&newpkg->files, entryname)) { - _alpm_log(handle, ALPM_LOG_WARNING, _("file not found in file list for package %s. skipping extraction of %s\n"), - newpkg->name, entryname); - return 0; - } - /* build the new entryname relative to handle->root */ - snprintf(filename, PATH_MAX, "%s%s", handle->root, entryname); } + /* build the new entryname relative to handle->root */ + snprintf(filename, PATH_MAX, "%s%s", handle->root, entryname);
/* if a file is in NoExtract then we never extract it */ if(_alpm_fnmatch_patterns(handle->noextract, entryname) == 0) {
OK.
If an error occurs the actual path being extracted is more useful than
the original path from the package file list. The original path is
still used for checks that use it directly.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
If an error occurs the actual path being extracted is more useful than the original path from the package file list. The original path is still used for checks that use it directly.
Signed-off-by: Andrew Gregory
---
Ack.
lib/libalpm/add.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index e5f5d42..ee92414 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -107,7 +107,7 @@ int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg) }
static int perform_extraction(alpm_handle_t *handle, struct archive *archive, - struct archive_entry *entry, const char *filename, const char *origname) + struct archive_entry *entry, const char *filename) { int ret; const int archive_flags = ARCHIVE_EXTRACT_OWNER | @@ -120,13 +120,13 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive, if(ret == ARCHIVE_WARN && archive_errno(archive) != ENOSPC) { /* operation succeeded but a "non-critical" error was encountered */ _alpm_log(handle, ALPM_LOG_WARNING, _("warning given when extracting %s (%s)\n"), - origname, archive_error_string(archive)); + filename, archive_error_string(archive)); } else if(ret != ARCHIVE_OK) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not extract %s (%s)\n"), - origname, archive_error_string(archive)); + filename, archive_error_string(archive)); alpm_logaction(handle, ALPM_CALLER_PREFIX, "error: could not extract %s (%s)\n", - origname, archive_error_string(archive)); + filename, archive_error_string(archive)); return 1; } return 0;
OK
@@ -164,7 +164,7 @@ static int extract_db_file(alpm_handle_t *handle, struct archive *archive, archive_entry_set_perm(entry, 0644); snprintf(filename, PATH_MAX, "%s%s-%s/%s", _alpm_db_path(handle->db_local), newpkg->name, newpkg->version, dbfile); - return perform_extraction(handle, archive, entry, filename, filename); + return perform_extraction(handle, archive, entry, filename); }
static int extract_single_file(alpm_handle_t *handle, struct archive *archive, @@ -176,7 +176,6 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, char filename[PATH_MAX]; /* the actual file we're extracting */ int needbackup = 0, notouch = 0; const char *hash_orig = NULL; - char *entryname_orig = NULL; int errors = 0; struct stat lsbuf;
@@ -185,10 +184,12 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, }
if (!alpm_filelist_contains(&newpkg->files, entryname)) { - _alpm_log(handle, ALPM_LOG_WARNING, _("file not found in file list for package %s. skipping extraction of %s\n"), + _alpm_log(handle, ALPM_LOG_WARNING, + _("file not found in file list for package %s. skipping extraction of %s\n"), newpkg->name, entryname); return 0; }
OK
+ /* build the new entryname relative to handle->root */ snprintf(filename, PATH_MAX, "%s%s", handle->root, entryname);
@@ -279,10 +280,6 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, } }
- /* we need access to the original entryname later after calls to - * archive_entry_set_pathname(), so we need to dupe it and free() later */ - STRDUP(entryname_orig, entryname, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); - if(needbackup) { char *checkfile; char *hash_local = NULL, *hash_pkg = NULL; @@ -293,7 +290,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); snprintf(checkfile, len, "%s.paccheck", filename);
- if(perform_extraction(handle, archive, entry, checkfile, entryname_orig)) { + if(perform_extraction(handle, archive, entry, checkfile)) { errors++; goto needbackup_cleanup; } @@ -307,7 +304,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, STRDUP(backup->hash, hash_pkg, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); }
- _alpm_log(handle, ALPM_LOG_DEBUG, "checking hashes for %s\n", entryname_orig); + _alpm_log(handle, ALPM_LOG_DEBUG, "checking hashes for %s\n", filename); _alpm_log(handle, ALPM_LOG_DEBUG, "current: %s\n", hash_local); _alpm_log(handle, ALPM_LOG_DEBUG, "new: %s\n", hash_pkg); _alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", hash_orig);
OK
@@ -316,7 +313,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, /* local and new files are the same, updating anyway to get * correct timestamps */ _alpm_log(handle, ALPM_LOG_DEBUG, "action: installing new file: %s\n", - entryname_orig); + filename); if(try_rename(handle, checkfile, filename)) { errors++; } @@ -330,7 +327,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, /* installed file has NOT been changed by user, * update to the new version */ _alpm_log(handle, ALPM_LOG_DEBUG, "action: installing new file: %s\n", - entryname_orig); + filename); if(try_rename(handle, checkfile, filename)) { errors++; } @@ -426,9 +423,8 @@ needbackup_cleanup: unlink(filename); }
- if(perform_extraction(handle, archive, entry, filename, entryname_orig)) { + if(perform_extraction(handle, archive, entry, filename)) { /* error */ - free(entryname_orig); errors++; return errors; } @@ -456,7 +452,6 @@ needbackup_cleanup: backup->hash = alpm_compute_md5sum(filename); } } - free(entryname_orig); return errors; }
OK
Leave user files in place and save new config files with a .pacnew
extension. This reduces the complexity of file extraction and respects
the principle that pacman shouldn't modify files it didn't create.
Signed-off-by: Andrew Gregory
On Wed, Oct 1, 2014 at 2:05 AM, Andrew Gregory
Leave user files in place and save new config files with a .pacnew extension. This reduces the complexity of file extraction and respects the principle that pacman shouldn't modify files it didn't create.
Signed-off-by: Andrew Gregory
---
The idea seems sane, but I might not change the pacdiff program's search function- if people still have pacorig files laying around, it is good for them to be picked up by that program so they can be handled.
contrib/README | 2 +- contrib/pacdiff.sh.in | 9 ++--- doc/pacman.8.txt | 5 ++- lib/libalpm/add.c | 82 ++++++++++++----------------------------- lib/libalpm/alpm.h | 15 +------- src/pacman/callback.c | 16 -------- test/pacman/README | 1 - test/pacman/pmrule.py | 3 -- test/pacman/tests/upgrade015.py | 1 - test/pacman/tests/upgrade016.py | 5 +-- test/pacman/tests/upgrade027.py | 1 - test/pacman/tests/upgrade028.py | 1 - test/pacman/tests/upgrade029.py | 1 - 13 files changed, 34 insertions(+), 108 deletions(-)
On 01/10/14 17:05, Andrew Gregory wrote:
Leave user files in place and save new config files with a .pacnew extension. This reduces the complexity of file extraction and respects the principle that pacman shouldn't modify files it didn't create.
Signed-off-by: Andrew Gregory
--- contrib/README | 2 +- contrib/pacdiff.sh.in | 9 ++--- doc/pacman.8.txt | 5 ++- lib/libalpm/add.c | 82 ++++++++++++----------------------------- lib/libalpm/alpm.h | 15 +------- src/pacman/callback.c | 16 -------- test/pacman/README | 1 - test/pacman/pmrule.py | 3 -- test/pacman/tests/upgrade015.py | 1 - test/pacman/tests/upgrade016.py | 5 +-- test/pacman/tests/upgrade027.py | 1 - test/pacman/tests/upgrade028.py | 1 - test/pacman/tests/upgrade029.py | 1 - 13 files changed, 34 insertions(+), 108 deletions(-)
As Dan pointed out, it would be good to keep pacdiff as-is until a suitable period has passed for the removal of pacorig files. I am undecided whether you should remove "pacorig" from its description still, but leaning towards yes.
diff --git a/contrib/README b/contrib/README index ae33bb2..8b5dd3c 100644 --- a/contrib/README +++ b/contrib/README @@ -16,7 +16,7 @@ sync databases (for safety on rolling release distributions). paccache - a flexible package cache cleaning utility that allows greater control over which packages are removed.
-pacdiff - a simple pacnew/pacorig/pacsave updater for /etc/. +pacdiff - a simple pacnew/pacsave updater for /etc/.
paclist - list all packages installed from a given repository. Useful for seeing which packages you may have installed from the testing repository, diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index ecb6ae2..d94754c 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -1,5 +1,5 @@ #!/bin/bash -# pacdiff : a simple pacnew/pacorig/pacsave updater +# pacdiff : a simple pacnew/pacsave updater # # Copyright (c) 2007 Aaron Griffin
# Copyright (c) 2013-2014 Pacman Development Team @@ -35,7 +35,7 @@ usage() { cat < -A simple program to merge or remove pacnew/pacorig/pacsave files. +A simple program to merge or remove pacnew/pacsave files.
Usage: $myname [-l | -f | -p] [--nocolor]
@@ -77,9 +77,9 @@ print_existing_pacsave(){
cmd() { if (( USE_LOCATE )); then - locate -0 -e -b \*.pacnew \*.pacorig \*.pacsave '*.pacsave.[0-9]*' + locate -0 -e -b \*.pacnew \*.pacsave '*.pacsave.[0-9]*' elif (( USE_FIND )); then - find $diffsearchpath \( -name \*.pacnew -o -name \*.pacorig -o -name \*.pacsave -o -name '*.pacsave.[0-9]*' \) -print0 + find $diffsearchpath \( -name \*.pacnew -o -name \*.pacsave -o -name '*.pacsave.[0-9]*' \) -print0 elif (( USE_PACDB )); then awk '/^%BACKUP%$/ { while (getline) { @@ -88,7 +88,6 @@ cmd() { } }' "${pac_db}"/*/files | while read -r bkup; do print_existing "/$bkup.pacnew" - print_existing "/$bkup.pacorig" print_existing_pacsave "/$bkup.pacsave" done fi diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 152b261..4c05167 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -454,8 +454,9 @@ original=X, current=Y, new=Z::
original=NULL, current=Y, new=Z:: The package was not previously installed, and the file already exists on the - file system. Save the current file with a '.pacorig' extension, install the - new file, and warn the user. + file system. Install the new file with a '.pacnew' extension and warn the + user. The user must then manually merge any necessary changes into the + original file.
Examples
OK.
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index ee92414..74c1595 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -334,69 +334,33 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, } else { /* none of the three files matched another, unpack the new file alongside * the local file */ + char *newpath; + size_t newlen = strlen(filename) + strlen(".pacnew") + 1;
- if(oldpkg) { - char *newpath; - size_t newlen = strlen(filename) + strlen(".pacnew") + 1; - - _alpm_log(handle, ALPM_LOG_DEBUG, - "action: keeping current file and installing" - " new one with .pacnew ending\n"); - - MALLOC(newpath, newlen, - errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); - snprintf(newpath, newlen, "%s.pacnew", filename); - - if(try_rename(handle, checkfile, newpath)) { - errors++; - } else { - alpm_event_pacnew_created_t event = { - .type = ALPM_EVENT_PACNEW_CREATED, - .from_noupgrade = 0, - .oldpkg = oldpkg, - .newpkg = newpkg, - .file = filename - }; - EVENT(handle, &event); - alpm_logaction(handle, ALPM_CALLER_PREFIX, - "warning: %s installed as %s\n", filename, newpath); - } + _alpm_log(handle, ALPM_LOG_DEBUG, + "action: keeping current file and installing" + " new one with .pacnew ending\n");
- free(newpath); - } else { - char *newpath; - size_t newlen = strlen(filename) + strlen(".pacorig") + 1; - - _alpm_log(handle, ALPM_LOG_DEBUG, - "action: saving existing file with a .pacorig ending" - " and installing a new one\n"); - - MALLOC(newpath, newlen, - errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); - snprintf(newpath, newlen, "%s.pacorig", filename); - - /* move the existing file to the "pacorig" */ - if(try_rename(handle, filename, newpath)) { - errors++; /* failed rename filename -> filename.pacorig */ - errors++; /* failed rename checkfile -> filename */ - } else { - /* rename the file we extracted to the real name */ - if(try_rename(handle, checkfile, filename)) { - errors++; - } else { - alpm_event_pacorig_created_t event = { - .type = ALPM_EVENT_PACORIG_CREATED, - .newpkg = newpkg, - .file = filename - }; - EVENT(handle, &event); - alpm_logaction(handle, ALPM_CALLER_PREFIX, - "warning: %s saved as %s\n", filename, newpath); - } - } + MALLOC(newpath, newlen, + errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); + snprintf(newpath, newlen, "%s.pacnew", filename);
- free(newpath); + if(try_rename(handle, checkfile, newpath)) { + errors++; + } else { + alpm_event_pacnew_created_t event = { + .type = ALPM_EVENT_PACNEW_CREATED, + .from_noupgrade = 0, + .oldpkg = oldpkg, + .newpkg = newpkg, + .file = filename + }; + EVENT(handle, &event); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: %s installed as %s\n", filename, newpath); } + + free(newpath); }
needbackup_cleanup:
OK. PS: Wow!!! That is some serious deletions!
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 1cfd4f5..7c64f34 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -445,10 +445,7 @@ typedef enum _alpm_event_type_t { ALPM_EVENT_PACNEW_CREATED, /** A .pacsave file was created; See alpm_event_pacsave_created_t for * arguments */ - ALPM_EVENT_PACSAVE_CREATED, - /** A .pacorig file was created; See alpm_event_pacorig_created_t for - * arguments */ - ALPM_EVENT_PACORIG_CREATED
Add note to README about this change.
+ ALPM_EVENT_PACSAVE_CREATED } alpm_event_type_t;
typedef struct _alpm_event_any_t { @@ -539,15 +536,6 @@ typedef struct _alpm_event_pacsave_created_t { const char *file; } alpm_event_pacsave_created_t;
-typedef struct _alpm_event_pacorig_created_t { - /** Type of event. */ - alpm_event_type_t type; - /** New package. */ - alpm_pkg_t *newpkg; - /** Filename of the file without the .pacorig suffix. */ - const char *file; -} alpm_event_pacorig_created_t; -
And this one.
/** Events. * This is an union passed to the callback, that allows the frontend to know * which type of event was triggered (via type). It is then possible to @@ -564,7 +552,6 @@ typedef union _alpm_event_t { alpm_event_pkgdownload_t pkgdownload; alpm_event_pacnew_created_t pacnew_created; alpm_event_pacsave_created_t pacsave_created; - alpm_event_pacorig_created_t pacorig_created; } alpm_event_t;
And this one.
/** Event callback. */ diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 4993382..6e3fbbe 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -313,22 +313,6 @@ void cb_event(alpm_event_t *event) } } break; - case ALPM_EVENT_PACORIG_CREATED: - { - alpm_event_pacorig_created_t *e = &event->pacorig_created; - if(on_progress) { - char *string = NULL; - pm_sprintf(&string, ALPM_LOG_WARNING, _("%s saved as %s.pacorig\n"), - e->file, e->file); - if(string != NULL) { - output = alpm_list_add(output, string); - } - } else { - pm_printf(ALPM_LOG_WARNING, _("%s saved as %s.pacorig\n"), - e->file, e->file); - } - } - break; /* all the simple done events, with fallthrough for each */ case ALPM_EVENT_FILECONFLICTS_DONE: case ALPM_EVENT_CHECKDEPS_DONE:
OK
diff --git a/test/pacman/README b/test/pacman/README index 8d8354a..6c601b2 100644 --- a/test/pacman/README +++ b/test/pacman/README @@ -310,7 +310,6 @@ its DEPENDS field. FILE_TYPE=path/to/file|type (possible types: dir, file, link) FILE_PACNEW=path/to/file FILE_PACSAVE=path/to/file - FILE_PACORIG=path/to/file
Example: FILE_EXIST=etc/test.conf
OK
diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index 0eec8ea..57f1786 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -148,9 +148,6 @@ def check(self, test): elif case == "PACNEW": if not os.path.isfile("%s.pacnew" % filename): success = 0 - elif case == "PACORIG": - if not os.path.isfile("%s.pacorig" % filename): - success = 0 elif case == "PACSAVE": if not os.path.isfile("%s.pacsave" % filename): success = 0
OK
diff --git a/test/pacman/tests/upgrade015.py b/test/pacman/tests/upgrade015.py index ea6046c..64fe281 100644 --- a/test/pacman/tests/upgrade015.py +++ b/test/pacman/tests/upgrade015.py @@ -12,4 +12,3 @@ self.addrule("PKG_EXIST=dummy") self.addrule("FILE_MODIFIED=etc/dummy.conf") self.addrule("!FILE_PACNEW=etc/dummy.conf") -self.addrule("!FILE_PACORIG=etc/dummy.conf")
OK
diff --git a/test/pacman/tests/upgrade016.py b/test/pacman/tests/upgrade016.py index b6b3f3a..ddf57e8 100644 --- a/test/pacman/tests/upgrade016.py +++ b/test/pacman/tests/upgrade016.py @@ -11,6 +11,5 @@
self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=dummy") -self.addrule("FILE_MODIFIED=etc/dummy.conf") -self.addrule("!FILE_PACNEW=etc/dummy.conf") -self.addrule("FILE_PACORIG=etc/dummy.conf") +self.addrule("!FILE_MODIFIED=etc/dummy.conf") +self.addrule("FILE_PACNEW=etc/dummy.conf")
OK
diff --git a/test/pacman/tests/upgrade027.py b/test/pacman/tests/upgrade027.py index 99087f3..3dd694a 100644 --- a/test/pacman/tests/upgrade027.py +++ b/test/pacman/tests/upgrade027.py @@ -18,5 +18,4 @@ self.addrule("PKG_VERSION=dummy|1.0-2") self.addrule("FILE_PACNEW=etc/dummy.conf") self.addrule("!FILE_PACSAVE=etc/dummy.conf") -self.addrule("!FILE_PACORIG=etc/dummy.conf") self.addrule("FILE_EXIST=etc/dummy.conf")
OK
diff --git a/test/pacman/tests/upgrade028.py b/test/pacman/tests/upgrade028.py index 18a10f5..1e31aa3 100644 --- a/test/pacman/tests/upgrade028.py +++ b/test/pacman/tests/upgrade028.py @@ -18,5 +18,4 @@ self.addrule("PKG_VERSION=dummy|1.0-2") self.addrule("!FILE_PACNEW=etc/dummy.conf") self.addrule("!FILE_PACSAVE=etc/dummy.conf") -self.addrule("!FILE_PACORIG=etc/dummy.conf") self.addrule("FILE_EXIST=etc/dummy.conf")
OK
diff --git a/test/pacman/tests/upgrade029.py b/test/pacman/tests/upgrade029.py index c308f42..eef5c70 100644 --- a/test/pacman/tests/upgrade029.py +++ b/test/pacman/tests/upgrade029.py @@ -20,5 +20,4 @@ self.addrule("PKG_VERSION=dummy|1.0-1") self.addrule("!FILE_PACNEW=etc/dummy.conf") self.addrule("!FILE_PACSAVE=etc/dummy.conf") -self.addrule("!FILE_PACORIG=etc/dummy.conf") self.addrule("FILE_EXIST=etc/dummy.conf")
OK.
Prevents the need to rename the file if we end up keeping it and ensures
that pacnew files always reflect the most recent version by overwriting
stale copies.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
Prevents the need to rename the file if we end up keeping it and ensures that pacnew files always reflect the most recent version by overwriting stale copies.
Signed-off-by: Andrew Gregory
Ack.
Also adds checks that the filename does not exceed PATH_MAX.
Signed-off-by: Andrew Gregory
On 01/10/14 17:05, Andrew Gregory wrote:
Also adds checks that the filename does not exceed PATH_MAX.
Ack.
On 01/10/14 17:05, Andrew Gregory wrote:
This is my attempt to clean up the behemoth extract_single_file function. In order to do this I have made the following user-visible changes that I would like feedback on:
* skip mtree, noupgrade, noextract checks for db files * remove .pacorig support, use .pacnew instead * extract backup files directly to .pacnew instead of .paccheck
This takes extract_single_file down from 340 lines to 225 and considerably reduces the number of nested conditionals
This patchset is great. Deal with the comments on patch 5/7 and rebase (sorry - my fault for the slow review...), and I will pull immediately. Allan
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Dan McGee