Revision: 904 http://trac.macosforge.org/projects/darwinbuild/changeset/904 Author: wsiegrist@apple.com Date: 2010-11-01 13:34:40 -0700 (Mon, 01 Nov 2010) Log Message: ----------- Correctly handle rolling back roots when the installed root caused an object change. Modified Paths: -------------- branches/PR-8604911/darwinup/Depot.cpp branches/PR-8604911/darwinup/File.cpp branches/PR-8604911/darwinup/File.h branches/PR-8604911/testing/darwinup/dest.tar.gz branches/PR-8604911/testing/darwinup/rep_file_dir.tar.gz branches/PR-8604911/testing/darwinup/rep_flink_dir.tar.gz branches/PR-8604911/testing/darwinup/rep_link_dir.tar.gz branches/PR-8604911/testing/darwinup/run-tests.sh Modified: branches/PR-8604911/darwinup/Depot.cpp =================================================================== --- branches/PR-8604911/darwinup/Depot.cpp 2010-10-28 15:30:25 UTC (rev 903) +++ branches/PR-8604911/darwinup/Depot.cpp 2010-11-01 20:34:40 UTC (rev 904) @@ -458,6 +458,32 @@ actual->info_set(FILE_INFO_ROLLBACK_DATA); file->info_set(FILE_INFO_INSTALL_DATA); } + // if actual is a dir and file is not, recurse to save its children + if (S_ISDIR(actual->mode()) && !S_ISDIR(file->mode())) { + IF_DEBUG("[analyze] directory being replaced by file, save children\n"); + const char* sub_argv[] = { actual->path(), NULL }; + FTS* subfts = fts_open((char**)sub_argv, + FTS_PHYSICAL | FTS_COMFOLLOW | FTS_XDEV, + fts_compare); + FTSENT* subent = fts_read(subfts); // throw away actual + while ((subent = fts_read(subfts)) != NULL) { + IF_DEBUG("saving child: %s\n", subent->fts_path); + // skip post-order visits + if (subent->fts_info == FTS_DP) { + continue; + } + File* subact = FileFactory(subent->fts_path); + subact->info_set(FILE_INFO_BASE_SYSTEM); + if (subent->fts_info != FTS_D) { + IF_DEBUG("saving file data\n"); + subact->info_set(FILE_INFO_ROLLBACK_DATA); + } + if (!dryrun) { + res = this->insert(rollback, subact); + } + *rollback_files += 1; + } + } preceding = actual; } @@ -527,41 +553,7 @@ IF_DEBUG("[analyze] external changes but file same as actual\n"); state = 'E'; } - - // XXX: should this be done in backup_file? - // If we're going to need to squirrel away data, create - // the directory hierarchy now. - if (INFO_TEST(actual->info(), FILE_INFO_ROLLBACK_DATA)) { - char path[PATH_MAX]; - char* backup_dirpath; - - // we need the path minus our destination prefix for moving to the archive - strlcpy(path, actual->path() + strlen(m_prefix) - 1, sizeof(path)); - - const char* dir = dirname(path); - assert(dir != NULL); - - char *uuidpath; - char uuidstr[37]; - uuid_unparse_upper(rollback->uuid(), uuidstr); - - asprintf(&uuidpath, "%s/%s", m_archives_path, uuidstr); - assert(uuidpath != NULL); - join_path(&backup_dirpath, uuidpath, dir); - assert(backup_dirpath != NULL); - - if (!dryrun) res = mkdir_p(backup_dirpath); - if (res != 0 && errno != EEXIST) { - fprintf(stderr, "%s:%d: %s: %s (%d)\n", - __FILE__, __LINE__, backup_dirpath, strerror(errno), errno); - } else { - res = 0; - } - free(backup_dirpath); - free(uuidpath); - } - - + if ((state != ' ' && preceding_flags != FILE_INFO_IDENTICAL) || INFO_TEST(actual->info(), FILE_INFO_BASE_SYSTEM | FILE_INFO_ROLLBACK_DATA)) { *rollback_files += 1; @@ -619,12 +611,42 @@ IF_DEBUG("[backup] backup_file: %s , %s \n", file->path(), context->archive->m_name); if (INFO_TEST(file->info(), FILE_INFO_ROLLBACK_DATA)) { - char *path; // the file's path + char *path; // the file's path char *dstpath; // the path inside the archives char *relpath; // the file's path minus the destination prefix char *uuidpath; // archives path plus the uuid char uuidstr[37]; + // If we're going to need to squirrel away data, create + // the directory hierarchy now. + char backup_path[PATH_MAX]; + char* backup_dirpath; + + // we need the path minus our destination prefix for moving to the archive + IF_DEBUG("[backup] file->path() = %s \n", file->path()); + strlcpy(backup_path, file->path(), sizeof(backup_path)); + IF_DEBUG("[backup] backup_path = %s \n", backup_path); + + const char* dir = dirname(backup_path); + assert(dir != NULL); + IF_DEBUG("[backup] dir = %s \n", dir); + + uuid_unparse_upper(context->archive->uuid(), uuidstr); + asprintf(&uuidpath, "%s/%s", context->depot->m_archives_path, uuidstr); + assert(uuidpath != NULL); + IF_DEBUG("[backup] uuidpath = %s \n", uuidpath); + join_path(&backup_dirpath, uuidpath, dir); + assert(backup_dirpath != NULL); + + IF_DEBUG("mkdir_p: %s\n", backup_dirpath); + res = mkdir_p(backup_dirpath); + if (res != 0 && errno != EEXIST) { + fprintf(stderr, "%s:%d: %s: %s (%d)\n", + __FILE__, __LINE__, backup_dirpath, strerror(errno), errno); + } else { + res = 0; + } + // we need the path minus our destination path for moving to the archive size_t prefixlen = strlen(context->depot->m_prefix); if (strncmp(context->archive->m_name, "<Rollback>", strlen("<Rollback>")) == 0) { @@ -637,17 +659,14 @@ relpath += prefixlen - 1; } - uuid_unparse_upper(context->archive->uuid(), uuidstr); - asprintf(&uuidpath, "%s/%s", context->depot->m_archives_path, uuidstr); - assert(uuidpath != NULL); join_path(&dstpath, uuidpath, relpath); assert(dstpath != NULL); IF_DEBUG("[backup] path = %s \n", path); IF_DEBUG("[backup] relpath = %s \n", relpath); IF_DEBUG("[backup] dstpath = %s \n", dstpath); - IF_DEBUG("[backup] uuidpath = %s \n", uuidpath); + ++context->files_modified; // XXX: res = file->backup() @@ -656,9 +675,14 @@ if (res != 0) fprintf(stderr, "%s:%d: backup failed: %s: %s (%d)\n", __FILE__, __LINE__, dstpath, strerror(errno), errno); + + // XXX: we cant propagate error from callback, but its safe to die here + assert(res == 0); + free(path); free(dstpath); free(uuidpath); + free(backup_dirpath); } return res; } @@ -677,6 +701,8 @@ } if (res != 0) fprintf(stderr, "%s:%d: install failed: %s: %s (%d)\n", __FILE__, __LINE__, file->path(), strerror(errno), errno); + // XXX remove assert + assert(res == 0); return res; } @@ -919,8 +945,16 @@ state = 'U'; IF_DEBUG("[uninstall] restoring\n"); if (!dryrun && res == 0) { - res = preceding->install(context->depot->m_archives_path, - context->depot->m_prefix); + if (INFO_TEST(flags, FILE_INFO_TYPE_DIFFERS) && + S_ISDIR(preceding->mode())) { + // use rename instead of mkdir so children are restored + res = preceding->dirrename(context->depot->m_archives_path, + context->depot->m_prefix); + + } else { + res = preceding->install(context->depot->m_archives_path, + context->depot->m_prefix); + } } } else if (INFO_TEST(flags, FILE_INFO_MODE_DIFFERS) || INFO_TEST(flags, FILE_INFO_GID_DIFFERS) || Modified: branches/PR-8604911/darwinup/File.cpp =================================================================== --- branches/PR-8604911/darwinup/File.cpp 2010-10-28 15:30:25 UTC (rev 903) +++ branches/PR-8604911/darwinup/File.cpp 2010-11-01 20:34:40 UTC (rev 904) @@ -228,6 +228,11 @@ return res; } +int File::dirrename(const char* prefix, const char* dest) { + // only used for directories + assert(0); +} + int File::remove() { // not implemented fprintf(stderr, "%s:%d: call to abstract function File::remove\n", @@ -341,6 +346,14 @@ : File(serial, archive, info, path, mode, uid, gid, size, digest) {}; int Directory::install(const char* prefix, const char* dest) { + return this->_install(prefix, dest, false); +} + +int Directory::dirrename(const char* prefix, const char* dest) { + return this->_install(prefix, dest, true); +} + +int Directory::_install(const char* prefix, const char* dest, bool use_rename) { // We create a new directory instead of renaming the // existing one, since that would move the entire // sub-tree, and lead to a lot of ENOENT errors. @@ -352,12 +365,36 @@ mode_t mode = this->mode() & ALLPERMS; uid_t uid = this->uid(); gid_t gid = this->gid(); - - IF_DEBUG("[install] mkdir(%s, %04o)\n", dstpath, mode); - if (res == 0) res = mkdir(dstpath, mode); - // mkdir is limited by umask, so ensure mode is set - if (res == 0) res = chmod(dstpath, mode); + if (use_rename) { + // determine source path under archives directory for rename + char srcpath[PATH_MAX]; + const char* path = this->path(); + Archive* archive = this->archive(); + char* dirpath = archive->directory_name(prefix); + IF_DEBUG("[install] dirpath is %s\n", dirpath); + if (is_directory(dirpath) == 0) { + IF_DEBUG("[install] expanding archive for directory rename\n"); + res = archive->expand_directory(prefix); + } + if (res == 0 && dirpath) { + ssize_t len = snprintf(srcpath, sizeof(srcpath), "%s/%s", dirpath, path); + if ((size_t)len > sizeof(srcpath)) { + fprintf(stderr, "ERROR: [install] path too long: %s/%s\n", + dirpath, path); + return -1; + } + } + if (is_regular_file(dstpath)) unlink(dstpath); + if (res == 0) IF_DEBUG("[install] rename(%s, %s)\n", srcpath, dstpath); + if (res == 0) res = rename(srcpath, dstpath); + } else { + IF_DEBUG("[install] mkdir(%s, %04o)\n", dstpath, mode); + res = mkdir(dstpath, mode); + // mkdir is limited by umask, so ensure mode is set + if (res == 0) res = chmod(dstpath, mode); + } + if (res && errno == EEXIST) { if (is_directory(dstpath)) { // this is expected in normal cases, so no need to force Modified: branches/PR-8604911/darwinup/File.h =================================================================== --- branches/PR-8604911/darwinup/File.h 2010-10-28 15:30:25 UTC (rev 903) +++ branches/PR-8604911/darwinup/File.h 2010-11-01 20:34:40 UTC (rev 904) @@ -172,7 +172,9 @@ // Installs the file from the archive into the prefix // i.e., for regular files: // rename(prefix + this->archive()->uuid() + this->path(), dest + this->path()); - virtual int install(const char* prefix, const char* dest); + virtual int install(const char* prefix, const char* dest); + // only used for directories + virtual int dirrename(const char* prefix, const char* dest); // Sets the mode, uid, and gid of the file in the dest path // XXX: rename as repair()? @@ -242,6 +244,8 @@ Directory(Archive* archive, FTSENT* ent); Directory(uint64_t serial, Archive* archive, uint32_t info, const char* path, mode_t mode, uid_t uid, gid_t gid, off_t size, Digest* digest); virtual int install(const char* prefix, const char* dest); + virtual int dirrename(const char* prefix, const char* dest); + int _install(const char* prefix, const char* dest, bool use_rename); virtual int remove(); }; Modified: branches/PR-8604911/testing/darwinup/dest.tar.gz =================================================================== (Binary files differ) Modified: branches/PR-8604911/testing/darwinup/rep_file_dir.tar.gz =================================================================== (Binary files differ) Modified: branches/PR-8604911/testing/darwinup/rep_flink_dir.tar.gz =================================================================== (Binary files differ) Modified: branches/PR-8604911/testing/darwinup/rep_link_dir.tar.gz =================================================================== (Binary files differ) Modified: branches/PR-8604911/testing/darwinup/run-tests.sh =================================================================== --- branches/PR-8604911/testing/darwinup/run-tests.sh 2010-10-28 15:30:25 UTC (rev 903) +++ branches/PR-8604911/testing/darwinup/run-tests.sh 2010-11-01 20:34:40 UTC (rev 904) @@ -354,6 +354,8 @@ $DARWINUP -f install $PREFIX/rep_file_dir is_dir $DEST/rep_file is_file $DEST/rep_file/subfile +is_dir $DEST/rep_file/subdir +is_file $DEST/rep_file/subdir/subsubfile $DARWINUP uninstall newest is_file $DEST/rep_file echo "DIFF: diffing original test files to dest (should be no diffs) ..." @@ -371,22 +373,30 @@ echo "========== TEST: Forcing object change: directory to file ==========" is_dir $DEST/rep_dir is_file $DEST/rep_dir/subfile +is_dir $DEST/rep_dir/subdir +is_file $DEST/rep_dir/subdir/subsubfile $DARWINUP -f install $PREFIX/rep_dir_file is_file $DEST/rep_dir $DARWINUP uninstall newest is_dir $DEST/rep_dir is_file $DEST/rep_dir/subfile +is_dir $DEST/rep_dir/subdir +is_file $DEST/rep_dir/subdir/subsubfile echo "DIFF: diffing original test files to dest (should be no diffs) ..." $DIFF $ORIG $DEST 2>&1 echo "========== TEST: Forcing object change: directory to symlink ==========" is_dir $DEST/rep_dir is_file $DEST/rep_dir/subfile +is_dir $DEST/rep_dir/subdir +is_file $DEST/rep_dir/subdir/subsubfile $DARWINUP -f install $PREFIX/rep_dir_link is_link $DEST/rep_dir $DARWINUP uninstall newest is_dir $DEST/rep_dir is_file $DEST/rep_dir/subfile +is_dir $DEST/rep_dir/subdir +is_file $DEST/rep_dir/subdir/subsubfile echo "DIFF: diffing original test files to dest (should be no diffs) ..." $DIFF $ORIG $DEST 2>&1 @@ -403,7 +413,9 @@ is_link $DEST/rep_link $DARWINUP -f install $PREFIX/rep_link_dir is_dir $DEST/rep_link -is_file $DEST/rep_link/subfile +is_file $DEST/rep_link/anotherfile +is_dir $DEST/rep_link/anotherdir +is_file $DEST/rep_link/anotherdir/anothersubfile $DARWINUP uninstall newest is_link $DEST/rep_link echo "DIFF: diffing original test files to dest (should be no diffs) ..." @@ -423,6 +435,8 @@ $DARWINUP -f install $PREFIX/rep_flink_dir is_dir $DEST/rep_flink is_file $DEST/rep_flink/subfile +is_dir $DEST/rep_flink/subdir +is_file $DEST/rep_flink/subdir/subsubfile $DARWINUP uninstall newest is_link $DEST/rep_flink echo "DIFF: diffing original test files to dest (should be no diffs) ..."