From 49327806062652db999f3f0a7f360edb3047791a Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Mon, 26 Nov 2018 14:59:09 -0800 Subject: [PATCH 1/5] SYSTEM_ROOT -> get_system_root Add a get_system_root() function in roots.cpp which returns / or /system based on fstab. This factors out the 'if' check from recovery.cpp and fsck_unshare_blocks.cpp. Test: boot recovery, mount system Bug: 118634720 Change-Id: If4947cba3a3f08dbe695fd2b50a9354cd302ee03 --- fsck_unshare_blocks.cpp | 13 ++++--------- recovery.cpp | 11 ++--------- roots.cpp | 9 +++++++++ roots.h | 2 ++ 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/fsck_unshare_blocks.cpp b/fsck_unshare_blocks.cpp index 684958e3..b653d13c 100644 --- a/fsck_unshare_blocks.cpp +++ b/fsck_unshare_blocks.cpp @@ -40,7 +40,6 @@ static constexpr const char* SYSTEM_E2FSCK_BIN = "/system/bin/e2fsck_static"; static constexpr const char* TMP_E2FSCK_BIN = "/tmp/e2fsck.bin"; -static constexpr const char* SYSTEM_ROOT = "/system"; static bool copy_file(const char* source, const char* dest) { android::base::unique_fd source_fd(open(source, O_RDONLY)); @@ -121,14 +120,10 @@ bool do_fsck_unshare_blocks() { std::vector partitions = { "/odm", "/oem", "/product", "/vendor" }; // Temporarily mount system so we can copy e2fsck_static. - bool mounted = false; - if (volume_for_mount_point(SYSTEM_ROOT) == nullptr) { - mounted = ensure_path_mounted_at("/", "/mnt/system") != -1; - partitions.push_back("/"); - } else { - mounted = ensure_path_mounted_at(SYSTEM_ROOT, "/mnt/system") != -1; - partitions.push_back(SYSTEM_ROOT); - } + std::string system_root = get_system_root(); + bool mounted = ensure_path_mounted_at(system_root.c_str(), "/mnt/system"); + partitions.push_back(system_root); + if (!mounted) { LOG(ERROR) << "Failed to mount system image."; return false; diff --git a/recovery.cpp b/recovery.cpp index e17526a9..7e1fa43a 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -78,7 +78,6 @@ static constexpr const char* CACHE_ROOT = "/cache"; static constexpr const char* DATA_ROOT = "/data"; static constexpr const char* METADATA_ROOT = "/metadata"; static constexpr const char* SDCARD_ROOT = "/sdcard"; -static constexpr const char* SYSTEM_ROOT = "/system"; // We define RECOVERY_API_VERSION in Android.mk, which will be picked up by build system and packed // into target_files.zip. Assert the version defined in code and in Android.mk are consistent. @@ -837,14 +836,8 @@ static Device::BuiltinAction prompt_and_wait(Device* device, int status) { } case Device::MOUNT_SYSTEM: // the system partition is mounted at /mnt/system - if (volume_for_mount_point(SYSTEM_ROOT) == nullptr) { - if (ensure_path_mounted_at("/", "/mnt/system") != -1) { - ui->Print("Mounted /system.\n"); - } - } else { - if (ensure_path_mounted_at(SYSTEM_ROOT, "/mnt/system") != -1) { - ui->Print("Mounted /system.\n"); - } + if (ensure_path_mounted_at(get_system_root().c_str(), "/mnt/system") != -1) { + ui->Print("Mounted /system.\n"); } break; diff --git a/roots.cpp b/roots.cpp index dc347848..e68cb20b 100644 --- a/roots.cpp +++ b/roots.cpp @@ -44,6 +44,7 @@ static struct fstab* fstab = nullptr; static bool did_map_logical_partitions = false; +static constexpr const char* SYSTEM_ROOT = "/system"; extern struct selabel_handle* sehandle; @@ -412,3 +413,11 @@ int setup_install_mounts() { bool logical_partitions_mapped() { return did_map_logical_partitions; } + +std::string get_system_root() { + if (volume_for_mount_point(SYSTEM_ROOT) == nullptr) { + return "/"; + } else { + return SYSTEM_ROOT; + } +} diff --git a/roots.h b/roots.h index 702af8de..314eb5f2 100644 --- a/roots.h +++ b/roots.h @@ -55,4 +55,6 @@ int setup_install_mounts(); bool logical_partitions_mapped(); +std::string get_system_root(); + #endif // RECOVERY_ROOTS_H_ From 1eeee45a7df56b84c99006256d500e7971d37c7d Mon Sep 17 00:00:00 2001 From: xunchang Date: Wed, 28 Nov 2018 23:30:48 -0800 Subject: [PATCH 2/5] ImageGenerator: Handle special characters in xml files In specific, the apostrophe appears as "\'"; and a new line appears as "\n\n". We need to handle these two cases accordingly. Bug: 74397117 Test: generate and check the image Change-Id: I67b1ebce7494e4a685a0c7334da58dc6df2ccb29 --- tools/image_generator/ImageGenerator.java | 47 ++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tools/image_generator/ImageGenerator.java b/tools/image_generator/ImageGenerator.java index 19d187d2..50a49845 100644 --- a/tools/image_generator/ImageGenerator.java +++ b/tools/image_generator/ImageGenerator.java @@ -92,7 +92,7 @@ public class ImageGenerator { // Some localized font cannot draw the word "Android" and some PUNCTUATIONS; we need to fall // back to use our default latin font instead. - private static final char[] PUNCTUATIONS = {',', ';', '.', '!' }; + private static final char[] PUNCTUATIONS = {',', ';', '.', '!', '?'}; private static final String ANDROID_STRING = "Android"; @@ -214,8 +214,9 @@ public class ImageGenerator { index + ANDROID_STRING.length()); } - // Adds the attribute to use default font to draw the PUNCTUATIONS ", . !" + // Adds the attribute to use default font to draw the PUNCTUATIONS ", . ; ! ?" for (char punctuation : PUNCTUATIONS) { + // TODO (xunchang) handle the RTL language that has different directions for '?' if (text.indexOf(punctuation) != -1 && !textFont.canDisplay(punctuation)) { int index = 0; while ((index = text.indexOf(punctuation, index)) != -1) { @@ -229,6 +230,11 @@ public class ImageGenerator { mWrappedLines.add(new LineInfo(attributedText, width)); } + + /** Merges two WrappedTextInfo. */ + public void addLines(WrappedTextInfo other) { + mWrappedLines.addAll(other.mWrappedLines); + } } /** Initailizes the fields of the image image. */ @@ -393,15 +399,7 @@ public class ImageGenerator { "Can not find the font file " + fontName + " for language " + language); } - /** - * Wraps the text with a maximum of mImageWidth pixels per line. - * - * @param text the string representation of text to wrap - * @param metrics the metrics of the Font used to draw the text; it gives the width in pixels of - * the text given its string representation - * @return a WrappedTextInfo class with the width of each AttributedString smaller than - * mImageWidth pixels - */ + /** Wraps the text with a maximum of mImageWidth pixels per line. */ private WrappedTextInfo wrapText(String text, FontMetrics metrics) { WrappedTextInfo info = new WrappedTextInfo(); @@ -436,6 +434,29 @@ public class ImageGenerator { return info; } + /** + * Handles the special characters of the raw text embedded in the xml file; and wraps the text + * with a maximum of mImageWidth pixels per line. + * + * @param text the string representation of text to wrap + * @param metrics the metrics of the Font used to draw the text; it gives the width in pixels of + * the text given its string representation + * @return a WrappedTextInfo class with the width of each AttributedString smaller than + * mImageWidth pixels + */ + private WrappedTextInfo processAndWrapText(String text, FontMetrics metrics) { + // Apostrophe is escaped in the xml file. + String processed = text.replace("\\'", "'"); + // The separator "\n\n" indicates a new line in the text. + String[] lines = processed.split("\\\\n\\\\n"); + WrappedTextInfo result = new WrappedTextInfo(); + for (String line : lines) { + result.addLines(wrapText(line, metrics)); + } + + return result; + } + /** * Encodes the information of the text image for |locale|. According to minui/resources.cpp, the * width, height and locale of the image is decoded as: int w = (row[1] << 8) | row[0]; int h = @@ -477,7 +498,7 @@ public class ImageGenerator { throws IOException, FontFormatException { Graphics2D graphics = createGraphics(locale); FontMetrics fontMetrics = graphics.getFontMetrics(); - WrappedTextInfo wrappedTextInfo = wrapText(text, fontMetrics); + WrappedTextInfo wrappedTextInfo = processAndWrapText(text, fontMetrics); int textWidth = 0; for (WrappedTextInfo.LineInfo lineInfo : wrappedTextInfo.mWrappedLines) { @@ -512,7 +533,7 @@ public class ImageGenerator { Graphics2D graphics = createGraphics(locale); FontMetrics fontMetrics = graphics.getFontMetrics(); - WrappedTextInfo wrappedTextInfo = wrapText(text, fontMetrics); + WrappedTextInfo wrappedTextInfo = processAndWrapText(text, fontMetrics); // Marks the start y offset for the text image of current locale; and reserves one line to // encode the image metadata. From a48f00a5e82ccc2729619fba9796775360fbafb7 Mon Sep 17 00:00:00 2001 From: xunchang Date: Mon, 26 Nov 2018 15:40:28 -0800 Subject: [PATCH 3/5] ImageGenerator: ignore the duplicate locales Some language variants have the duplicated translations. Therefore, we can compare the translated string for each locale to the default text string for that language. And the duplicated ones will be skipped to save some space. Bug: 74397117 Test: generate and check the image Change-Id: If51fa8fe700350c06f084f0e74d3698b82d6f177 --- tools/image_generator/ImageGenerator.java | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tools/image_generator/ImageGenerator.java b/tools/image_generator/ImageGenerator.java index 50a49845..fd8e5429 100644 --- a/tools/image_generator/ImageGenerator.java +++ b/tools/image_generator/ImageGenerator.java @@ -41,6 +41,7 @@ import java.io.IOException; import java.text.AttributedString; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -605,11 +606,15 @@ public class ImageGenerator { mDefaultFont = defaultFontMetrics.getFont(); mAndroidStringWidth = defaultFontMetrics.stringWidth(ANDROID_STRING); - Map languageCount = new TreeMap<>(); + // The last country variant should be the fallback locale for a given language. + Map fallbackLocaleMap = new HashMap<>(); int textWidth = 0; for (Locale locale : localizedTextMap.keySet()) { - String language = locale.getLanguage(); - languageCount.put(language, languageCount.getOrDefault(language, 0) + 1); + // Updates the fallback locale if we have a new language variant. Don't do it for en-XC + // as it's a pseudo-locale. + if (!locale.toLanguageTag().equals("en-XC")) { + fallbackLocaleMap.put(locale.getLanguage(), locale); + } textWidth = Math.max(textWidth, measureTextWidth(localizedTextMap.get(locale), locale)); } @@ -617,15 +622,16 @@ public class ImageGenerator { resize(textWidth, mImageHeight); for (Locale locale : localizedTextMap.keySet()) { - Integer count = languageCount.get(locale.getLanguage()); // Recovery expects en-US instead of en_US. String languageTag = locale.toLanguageTag(); - if (count == 1) { - // Make the last country variant for a given language be the catch-all for that + Locale fallbackLocale = fallbackLocaleMap.get(locale.getLanguage()); + if (locale.equals(fallbackLocale)) { + // Makes the last country variant for a given language be the catch-all for that // language. languageTag = locale.getLanguage(); - } else { - languageCount.put(locale.getLanguage(), count - 1); + } else if (localizedTextMap.get(locale).equals(localizedTextMap.get(fallbackLocale))) { + LOGGER.info("Skip parsing text for duplicate locale " + locale); + continue; } drawText(localizedTextMap.get(locale), locale, languageTag); From 45e505a4e9ef1247c5c8acb437b351212cbbd409 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 29 Nov 2018 13:33:07 -0800 Subject: [PATCH 4/5] Update for long long -> off64_t for fstab.length Test: boot Change-Id: I755a5dd48f1d5be2cbd8e494b9c405674b12cebb --- roots.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/roots.cpp b/roots.cpp index 4fcd6feb..8772c577 100644 --- a/roots.cpp +++ b/roots.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -66,7 +67,7 @@ void load_volume_table() { printf("=========================\n"); for (int i = 0; i < fstab->num_entries; ++i) { const Volume* v = &fstab->recs[i]; - printf(" %d %s %s %s %lld\n", i, v->mount_point, v->fs_type, v->blk_device, v->length); + printf(" %d %s %s %s %" PRId64 "\n", i, v->mount_point, v->fs_type, v->blk_device, v->length); } printf("\n"); } From d7b685804507752638ef7572c30428a0fc05118b Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 30 Nov 2018 15:21:53 -0800 Subject: [PATCH 5/5] Fix ensure_path_mounted_at for fsck_unshare_blocks Test: pass Bug: 118634720 Change-Id: Icb2540892bd35659462f56c3883a39759f29eeab --- fsck_unshare_blocks.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsck_unshare_blocks.cpp b/fsck_unshare_blocks.cpp index b653d13c..acc453cd 100644 --- a/fsck_unshare_blocks.cpp +++ b/fsck_unshare_blocks.cpp @@ -121,7 +121,7 @@ bool do_fsck_unshare_blocks() { // Temporarily mount system so we can copy e2fsck_static. std::string system_root = get_system_root(); - bool mounted = ensure_path_mounted_at(system_root.c_str(), "/mnt/system"); + bool mounted = ensure_path_mounted_at(system_root.c_str(), "/mnt/system") != -1; partitions.push_back(system_root); if (!mounted) {