Revision: 918 http://trac.macosforge.org/projects/darwinbuild/changeset/918 Author: wsiegrist@apple.com Date: 2011-02-04 15:53:53 -0800 (Fri, 04 Feb 2011) Log Message: ----------- Stop forcing all uninstall ops since this disabled the build check safety. Instead, we pass down the reverse_files flag to denote uninstall operations which allow type changes to happen. Add tests for the build check. Modified Paths: -------------- branches/PR-8817822/darwinup/Depot.cpp branches/PR-8817822/darwinup/File.cpp branches/PR-8817822/darwinup/File.h branches/PR-8817822/darwinup/main.cpp branches/PR-8817822/testing/darwinup/run-tests.sh Modified: branches/PR-8817822/darwinup/Depot.cpp =================================================================== --- branches/PR-8817822/darwinup/Depot.cpp 2011-02-04 22:37:04 UTC (rev 917) +++ branches/PR-8817822/darwinup/Depot.cpp 2011-02-04 23:53:53 UTC (rev 918) @@ -695,7 +695,9 @@ if (INFO_TEST(file->info(), FILE_INFO_INSTALL_DATA)) { ++context->files_modified; - res = file->install(context->depot->m_archives_path, context->depot->m_prefix); + res = file->install(context->depot->m_archives_path, + context->depot->m_prefix, + context->reverse_files); } else { res = file->install_info(context->depot->m_prefix); } @@ -947,11 +949,13 @@ 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); + context->depot->m_prefix, + context->reverse_files); } else { res = preceding->install(context->depot->m_archives_path, - context->depot->m_prefix); + context->depot->m_prefix, + context->reverse_files); } } } else if (INFO_TEST(flags, FILE_INFO_MODE_DIFFERS) || Modified: branches/PR-8817822/darwinup/File.cpp =================================================================== --- branches/PR-8817822/darwinup/File.cpp 2011-02-04 22:37:04 UTC (rev 917) +++ branches/PR-8817822/darwinup/File.cpp 2011-02-04 23:53:53 UTC (rev 918) @@ -147,7 +147,7 @@ free(dig); } -int File::install(const char* prefix, const char* dest) { +int File::install(const char* prefix, const char* dest, bool uninstall) { extern uint32_t force; int res = 0; Archive* archive = this->archive(); @@ -160,6 +160,10 @@ char* dstpath; join_path(&dstpath, dest, path); + // object changes are expected for some uninstall operations, + // otherwise require force flag + bool allow_change = (uninstall || force); + if (dirpath) { ssize_t len = snprintf(srcpath, sizeof(srcpath), "%s/%s", dirpath, path); if ((size_t)len > sizeof(srcpath)) { @@ -176,7 +180,7 @@ if (is_directory(dirpath) == 0) { IF_DEBUG("[install] File::install on-demand archive expansion\n"); res = archive->expand_directory(prefix); - if (res == 0) res = this->install(prefix, dest); + if (res == 0) res = this->install(prefix, dest, uninstall); } else { // archive was already expanded, so // the file is truly missing (worry). @@ -184,13 +188,13 @@ fprintf(stderr, "%s:%d: %s: %s (%d)\n", __FILE__, __LINE__, srcpath, strerror(errno), errno); } - } else if (force && errno == ENOTDIR) { + } else if (allow_change && errno == ENOTDIR) { // a) some part of destination path does not exist // b) from is a directory, but to is not IF_DEBUG("[install] File::install ENOTDIR\n"); fprintf(stderr, "%s:%d: %s: %s (%d)\n", __FILE__, __LINE__, dstpath, strerror(errno), errno); - } else if (force && errno == EISDIR) { + } else if (allow_change && errno == EISDIR) { // to is a directory, but from is not IF_DEBUG("[install] replacing directory with a file\n"); IF_DEBUG("[install] removefile(%s)\n", dstpath); @@ -206,7 +210,7 @@ if (res == -1) fprintf(stderr, "%s:%d: %s: %s (%d)\n", __FILE__, __LINE__, dstpath, strerror(errno), errno); - } else if (force && errno == ENOTEMPTY) { + } else if (allow_change && errno == ENOTEMPTY) { // to is a directory and is not empty IF_DEBUG("[install] File::install ENOTEMPTY\n"); fprintf(stderr, "%s:%d: %s: %s (%d)\n", @@ -228,7 +232,7 @@ return res; } -int File::dirrename(const char* prefix, const char* dest) { +int File::dirrename(const char* prefix, const char* dest, bool uninstall) { // only used for directories assert(0); } @@ -345,15 +349,15 @@ Digest* digest) : 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::install(const char* prefix, const char* dest, bool uninstall) { + return this->_install(prefix, dest, uninstall, false); } -int Directory::dirrename(const char* prefix, const char* dest) { - return this->_install(prefix, dest, true); +int Directory::dirrename(const char* prefix, const char* dest, bool uninstall) { + return this->_install(prefix, dest, uninstall, true); } -int Directory::_install(const char* prefix, const char* dest, bool use_rename) { +int Directory::_install(const char* prefix, const char* dest, bool uninstall, 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. @@ -362,6 +366,10 @@ char* dstpath; join_path(&dstpath, dest, this->path()); + // object changes are expected for some uninstall operations, + // otherwise require force flag + bool allow_change = (uninstall || force); + mode_t mode = this->mode() & ALLPERMS; uid_t uid = this->uid(); gid_t gid = this->gid(); @@ -403,7 +411,7 @@ if (res == -1) fprintf(stderr, "%s:%d: %s: %s (%d)\n", __FILE__, __LINE__, dstpath, strerror(errno), errno); - } else if (force) { + } else if (allow_change) { // this could be bad, so require the force option IF_DEBUG("[install] original node is a file, we need to replace " \ "with a directory \n"); @@ -415,7 +423,7 @@ __FILE__, __LINE__, dstpath, strerror(errno), errno); } - } else if (force && res == -1 && errno == ENOTDIR) { + } else if (allow_change && res == -1 && errno == ENOTDIR) { // some part of destination path is not a directory IF_DEBUG("[install] Directory::install ENOTDIR \n"); } else if (res == -1) { Modified: branches/PR-8817822/darwinup/File.h =================================================================== --- branches/PR-8817822/darwinup/File.h 2011-02-04 22:37:04 UTC (rev 917) +++ branches/PR-8817822/darwinup/File.h 2011-02-04 23:53:53 UTC (rev 918) @@ -172,9 +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, bool uninstall); // only used for directories - virtual int dirrename(const char* prefix, const char* dest); + virtual int dirrename(const char* prefix, const char* dest, bool uninstall); // Sets the mode, uid, and gid of the file in the dest path // XXX: rename as repair()? @@ -243,9 +243,9 @@ struct Directory : File { 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 install(const char* prefix, const char* dest, bool uninstall); + virtual int dirrename(const char* prefix, const char* dest, bool uninstall); + int _install(const char* prefix, const char* dest, bool uninstall, bool use_rename); virtual int remove(); }; Modified: branches/PR-8817822/darwinup/main.cpp =================================================================== --- branches/PR-8817822/darwinup/main.cpp 2011-02-04 22:37:04 UTC (rev 917) +++ branches/PR-8817822/darwinup/main.cpp 2011-02-04 23:53:53 UTC (rev 918) @@ -245,10 +245,6 @@ res = depot->process_archive(argv[0], argv[i]); } else if (strcmp(argv[0], "uninstall") == 0) { if (i==1 && depot->initialize(true)) exit(15); - // uninstall is always in force mode so it can - // uninstall archives that were installed under - // force mode - force = 1; res = depot->process_archive(argv[0], argv[i]); } else if (strcmp(argv[0], "verify") == 0) { if (i==1 && depot->initialize(true)) exit(16); Modified: branches/PR-8817822/testing/darwinup/run-tests.sh =================================================================== --- branches/PR-8817822/testing/darwinup/run-tests.sh 2011-02-04 22:37:04 UTC (rev 917) +++ branches/PR-8817822/testing/darwinup/run-tests.sh 2011-02-04 23:53:53 UTC (rev 918) @@ -97,6 +97,18 @@ done fi +echo "========== TEST: Test uninstall build check safety ==========" +$DARWINUP install $PREFIX/root2 +sqlite3 $DEST/.DarwinDepot/Database-V100 "UPDATE archives SET osbuild = '$(sw_vers -buildVersion)X'" +set +e +$DARWINUP uninstall root2 +if [ $? -eq 0 ]; then exit 1; fi +set -e +$DARWINUP -f uninstall root2 +echo "DIFF: diffing original test files to dest (should be no diffs) ..." +$DIFF $ORIG $DEST 2>&1 + + echo "========== TEST: Try installing a symlink-to-directory ==========" ln -s root2 $PREFIX/root_link # test without trailing slash