diff options
author | 2024-12-16 18:12:02 -0800 | |
---|---|---|
committer | 2024-12-16 19:25:30 -0800 | |
commit | 5f0df423bf9aeb8879354a178848578b30f82c65 (patch) | |
tree | 411f7027e5ccb9460e3f8aadc9134e2ebf3d3ff1 | |
parent | 09429c7d24cc423d4fe99dad6fe2c92bf3ec2a30 (diff) |
Revert^2 "[res] Better modification time resolution in Idmap"
This reverts commit e2cc267a14a4eccd54b9fe1f7d3c8d860ac80a4f.
Reason for revert: relanding with the macos build fix
Original comment:
We used to track the modification time in seconds, which is both
imprecise (an apk installation + idmap generation can easily
take less time) and forces us to wait for >1s in the tests to
just check if up-to-date checks work.
This change updates the time to nanosecond resolution where
supported (hm, MinGW for Windows, hm), as the underlying
OS API provides
Test: build + atest libandroidfw_tests idmap2_tests + boot
Flag: EXEMPT minor change
Change-Id: I49c36b0a6ae6e677fa1259090da20ccc7a224b99
-rw-r--r-- | libs/androidfw/AssetManager.cpp | 32 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/AssetManager.h | 22 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/Idmap.h | 5 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/misc.h | 35 | ||||
-rw-r--r-- | libs/androidfw/misc.cpp | 57 | ||||
-rw-r--r-- | libs/androidfw/tests/Idmap_test.cpp | 11 | ||||
-rw-r--r-- | tools/aapt/Package.cpp | 11 |
7 files changed, 107 insertions, 66 deletions
diff --git a/libs/androidfw/AssetManager.cpp b/libs/androidfw/AssetManager.cpp index e6182454ad8a..5955915c9fcd 100644 --- a/libs/androidfw/AssetManager.cpp +++ b/libs/androidfw/AssetManager.cpp @@ -1420,18 +1420,20 @@ void AssetManager::mergeInfoLocked(SortedVector<AssetDir::FileInfo>* pMergedInfo Mutex AssetManager::SharedZip::gLock; DefaultKeyedVector<String8, wp<AssetManager::SharedZip> > AssetManager::SharedZip::gOpen; -AssetManager::SharedZip::SharedZip(const String8& path, time_t modWhen) - : mPath(path), mZipFile(NULL), mModWhen(modWhen), - mResourceTableAsset(NULL), mResourceTable(NULL) -{ - if (kIsDebug) { - ALOGI("Creating SharedZip %p %s\n", this, mPath.c_str()); - } - ALOGV("+++ opening zip '%s'\n", mPath.c_str()); - mZipFile = ZipFileRO::open(mPath.c_str()); - if (mZipFile == NULL) { - ALOGD("failed to open Zip archive '%s'\n", mPath.c_str()); - } +AssetManager::SharedZip::SharedZip(const String8& path, ModDate modWhen) + : mPath(path), + mZipFile(NULL), + mModWhen(modWhen), + mResourceTableAsset(NULL), + mResourceTable(NULL) { + if (kIsDebug) { + ALOGI("Creating SharedZip %p %s\n", this, mPath.c_str()); + } + ALOGV("+++ opening zip '%s'\n", mPath.c_str()); + mZipFile = ZipFileRO::open(mPath.c_str()); + if (mZipFile == NULL) { + ALOGD("failed to open Zip archive '%s'\n", mPath.c_str()); + } } AssetManager::SharedZip::SharedZip(int fd, const String8& path) @@ -1453,7 +1455,7 @@ sp<AssetManager::SharedZip> AssetManager::SharedZip::get(const String8& path, bool createIfNotPresent) { AutoMutex _l(gLock); - time_t modWhen = getFileModDate(path.c_str()); + auto modWhen = getFileModDate(path.c_str()); sp<SharedZip> zip = gOpen.valueFor(path).promote(); if (zip != NULL && zip->mModWhen == modWhen) { return zip; @@ -1520,8 +1522,8 @@ ResTable* AssetManager::SharedZip::setResourceTable(ResTable* res) bool AssetManager::SharedZip::isUpToDate() { - time_t modWhen = getFileModDate(mPath.c_str()); - return mModWhen == modWhen; + auto modWhen = getFileModDate(mPath.c_str()); + return mModWhen == modWhen; } void AssetManager::SharedZip::addOverlay(const asset_path& ap) diff --git a/libs/androidfw/include/androidfw/AssetManager.h b/libs/androidfw/include/androidfw/AssetManager.h index ce0985b38986..376c881ea376 100644 --- a/libs/androidfw/include/androidfw/AssetManager.h +++ b/libs/androidfw/include/androidfw/AssetManager.h @@ -280,21 +280,21 @@ private: ~SharedZip(); private: - SharedZip(const String8& path, time_t modWhen); - SharedZip(int fd, const String8& path); - SharedZip(); // <-- not implemented + SharedZip(const String8& path, ModDate modWhen); + SharedZip(int fd, const String8& path); + SharedZip(); // <-- not implemented - String8 mPath; - ZipFileRO* mZipFile; - time_t mModWhen; + String8 mPath; + ZipFileRO* mZipFile; + ModDate mModWhen; - Asset* mResourceTableAsset; - ResTable* mResourceTable; + Asset* mResourceTableAsset; + ResTable* mResourceTable; - Vector<asset_path> mOverlays; + Vector<asset_path> mOverlays; - static Mutex gLock; - static DefaultKeyedVector<String8, wp<SharedZip> > gOpen; + static Mutex gLock; + static DefaultKeyedVector<String8, wp<SharedZip> > gOpen; }; /* diff --git a/libs/androidfw/include/androidfw/Idmap.h b/libs/androidfw/include/androidfw/Idmap.h index e213fbd22ab0..ac75eb3bb98c 100644 --- a/libs/androidfw/include/androidfw/Idmap.h +++ b/libs/androidfw/include/androidfw/Idmap.h @@ -25,8 +25,9 @@ #include "android-base/macros.h" #include "android-base/unique_fd.h" #include "androidfw/ConfigDescription.h" -#include "androidfw/StringPiece.h" #include "androidfw/ResourceTypes.h" +#include "androidfw/StringPiece.h" +#include "androidfw/misc.h" #include "utils/ByteOrder.h" namespace android { @@ -213,7 +214,7 @@ class LoadedIdmap { android::base::unique_fd idmap_fd_; std::string_view overlay_apk_path_; std::string_view target_apk_path_; - time_t idmap_last_mod_time_; + ModDate idmap_last_mod_time_; private: DISALLOW_COPY_AND_ASSIGN(LoadedIdmap); diff --git a/libs/androidfw/include/androidfw/misc.h b/libs/androidfw/include/androidfw/misc.h index 077609d20d55..c9ba8a01a5e9 100644 --- a/libs/androidfw/include/androidfw/misc.h +++ b/libs/androidfw/include/androidfw/misc.h @@ -13,14 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#pragma once -#include <sys/types.h> +#include <time.h> // // Handy utility functions and portability code. // -#ifndef _LIBS_ANDROID_FW_MISC_H -#define _LIBS_ANDROID_FW_MISC_H namespace android { @@ -41,15 +40,35 @@ typedef enum FileType { } FileType; /* get the file's type; follows symlinks */ FileType getFileType(const char* fileName); -/* get the file's modification date; returns -1 w/errno set on failure */ -time_t getFileModDate(const char* fileName); + +// MinGW doesn't support nanosecond resolution in stat() modification time, and given +// that it only matters on the device it's ok to keep it at a seconds level there. +#ifdef _WIN32 +using ModDate = time_t; +inline constexpr ModDate kInvalidModDate = ModDate(-1); +inline constexpr unsigned long long kModDateResolutionNs = 1ull * 1000 * 1000 * 1000; +inline time_t toTimeT(ModDate m) { + return m; +} +#else +using ModDate = timespec; +inline constexpr ModDate kInvalidModDate = {-1, -1}; +inline constexpr unsigned long long kModDateResolutionNs = 1; +inline time_t toTimeT(ModDate m) { + return m.tv_sec; +} +#endif + +/* get the file's modification date; returns kInvalidModDate w/errno set on failure */ +ModDate getFileModDate(const char* fileName); /* same, but also returns -1 if the file has already been deleted */ -time_t getFileModDate(int fd); +ModDate getFileModDate(int fd); // Check if |path| or |fd| resides on a readonly filesystem. bool isReadonlyFilesystem(const char* path); bool isReadonlyFilesystem(int fd); -}; // namespace android +} // namespace android -#endif // _LIBS_ANDROID_FW_MISC_H +// Whoever uses getFileModDate() will need this as well +bool operator==(const timespec& l, const timespec& r); diff --git a/libs/androidfw/misc.cpp b/libs/androidfw/misc.cpp index 93dcaf549a90..32f3624a3aee 100644 --- a/libs/androidfw/misc.cpp +++ b/libs/androidfw/misc.cpp @@ -28,11 +28,13 @@ #include <sys/vfs.h> #endif // __linux__ -#include <cstring> -#include <cstdio> #include <errno.h> #include <sys/stat.h> +#include <cstdio> +#include <cstring> +#include <tuple> + namespace android { /* @@ -73,27 +75,34 @@ FileType getFileType(const char* fileName) } } -/* - * Get a file's modification date. - */ -time_t getFileModDate(const char* fileName) { - struct stat sb; - if (stat(fileName, &sb) < 0) { - return (time_t)-1; - } - return sb.st_mtime; +static ModDate getModDate(const struct stat& st) { +#ifdef _WIN32 + return st.st_mtime; +#elif defined(__APPLE__) + return st.st_mtimespec; +#else + return st.st_mtim; +#endif } -time_t getFileModDate(int fd) { - struct stat sb; - if (fstat(fd, &sb) < 0) { - return (time_t)-1; - } - if (sb.st_nlink <= 0) { - errno = ENOENT; - return (time_t)-1; - } - return sb.st_mtime; +ModDate getFileModDate(const char* fileName) { + struct stat sb; + if (stat(fileName, &sb) < 0) { + return kInvalidModDate; + } + return getModDate(sb); +} + +ModDate getFileModDate(int fd) { + struct stat sb; + if (fstat(fd, &sb) < 0) { + return kInvalidModDate; + } + if (sb.st_nlink <= 0) { + errno = ENOENT; + return kInvalidModDate; + } + return getModDate(sb); } #ifndef __linux__ @@ -124,4 +133,8 @@ bool isReadonlyFilesystem(int fd) { } #endif // __linux__ -}; // namespace android +} // namespace android + +bool operator==(const timespec& l, const timespec& r) { + return std::tie(l.tv_sec, l.tv_nsec) == std::tie(r.tv_sec, l.tv_nsec); +} diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp index 60aa7d88925d..cb2e56f5f5e4 100644 --- a/libs/androidfw/tests/Idmap_test.cpp +++ b/libs/androidfw/tests/Idmap_test.cpp @@ -14,6 +14,9 @@ * limitations under the License. */ +#include <chrono> +#include <thread> + #include "android-base/file.h" #include "androidfw/ApkAssets.h" #include "androidfw/AssetManager2.h" @@ -27,6 +30,7 @@ #include "data/overlayable/R.h" #include "data/system/R.h" +using namespace std::chrono_literals; using ::testing::NotNull; namespace overlay = com::android::overlay; @@ -218,10 +222,13 @@ TEST_F(IdmapTest, OverlayAssetsIsUpToDate) { unlink(temp_file.path); ASSERT_FALSE(apk_assets->IsUpToDate()); - sleep(2); + + const auto sleep_duration = + std::chrono::nanoseconds(std::max(kModDateResolutionNs, 1'000'000ull)); + std::this_thread::sleep_for(sleep_duration); base::WriteStringToFile("hello", temp_file.path); - sleep(2); + std::this_thread::sleep_for(sleep_duration); ASSERT_FALSE(apk_assets->IsUpToDate()); } diff --git a/tools/aapt/Package.cpp b/tools/aapt/Package.cpp index 5e0f87f0dcaf..60c4bf5c4131 100644 --- a/tools/aapt/Package.cpp +++ b/tools/aapt/Package.cpp @@ -292,13 +292,12 @@ bool processFile(Bundle* bundle, ZipFile* zip, } if (!hasData) { const String8& srcName = file->getSourceFile(); - time_t fileModWhen; - fileModWhen = getFileModDate(srcName.c_str()); - if (fileModWhen == (time_t) -1) { // file existence tested earlier, - return false; // not expecting an error here + auto fileModWhen = getFileModDate(srcName.c_str()); + if (fileModWhen == kInvalidModDate) { // file existence tested earlier, + return false; // not expecting an error here } - - if (fileModWhen > entry->getModWhen()) { + + if (toTimeT(fileModWhen) > entry->getModWhen()) { // mark as deleted so add() will succeed if (bundle->getVerbose()) { printf(" (removing old '%s')\n", storageName.c_str()); |