From 372a5b71eab2e4bd420283392488ed5db0616f8c Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 24 Apr 2023 20:11:57 +0000 Subject: [PATCH] Bug 1799002 - Part 2: Remove Android workarounds for Bug 1627796. r=geckoview-reviewers,firefox-build-system-reviewers,glandium,m_kato The underlying Android-Gradle plugin bug has been addressed, so we no longer see the issue, and therefore we don't need this complicated "library set generation ID" workaround at all! Differential Revision: https://phabricator.services.mozilla.com/D175167 --- js/src/old-configure.in | 10 +- .../android/gradle/with_gecko_binaries.gradle | 182 ------------------ old-configure.in | 10 +- 3 files changed, 4 insertions(+), 198 deletions(-) diff --git a/js/src/old-configure.in b/js/src/old-configure.in index 2cc8a50ea8cf..ed8266c862b7 100644 --- a/js/src/old-configure.in +++ b/js/src/old-configure.in @@ -233,15 +233,9 @@ if test "$GNU_CC"; then AC_MSG_RESULT([no]) LDFLAGS=$_SAVE_LDFLAGS) - # While sha1 is deterministic, it is slower. - if test -z "$DEVELOPER_OPTIONS" -o "$OS_TARGET" = "Android"; then - build_id=sha1 - else - build_id=uuid - fi - AC_MSG_CHECKING([for --build-id=$build_id option to ld]) + AC_MSG_CHECKING([for --build-id=uuid option to ld]) _SAVE_LDFLAGS=$LDFLAGS - LDFLAGS="$LDFLAGS -Wl,--build-id=$build_id" + LDFLAGS="$LDFLAGS -Wl,--build-id=uuid" AC_TRY_LINK(,,AC_MSG_RESULT([yes]), AC_MSG_RESULT([no]) LDFLAGS=$_SAVE_LDFLAGS) diff --git a/mobile/android/gradle/with_gecko_binaries.gradle b/mobile/android/gradle/with_gecko_binaries.gradle index 1c7bba560506..009a1f586edc 100644 --- a/mobile/android/gradle/with_gecko_binaries.gradle +++ b/mobile/android/gradle/with_gecko_binaries.gradle @@ -6,38 +6,6 @@ // The JNI wrapper generation tasks depend on the JAR creation task of the :annotations project. evaluationDependsOn(':annotations') -import groovy.util.FileNameFinder -import groovy.transform.Memoized - -import java.nio.file.Path -import java.nio.file.Paths -import java.security.MessageDigest - -// To find the Android NDK directory, there are a few wrinkles. In a compiled -// build, we can use our own `ANDROID_NDK` configure option. But in an -// artifact build, that isn't defined, so we fall back to -// `android.ndkDirectory` -- but that's defined in `local.properties`, which -// may not define it. In that case, fall back to crawling the filesystem. -def getNDKDirectory() { - if (project.mozconfig.substs.ANDROID_NDK) { - return project.mozconfig.substs.ANDROID_NDK - } - try { - if (project.android.ndkDirectory) { - return project.android.ndkDirectory - } - } catch (InvalidUserDataException ex) { - // The NDK is not installed, that's ok. - } - def mozbuild = System.getenv('MOZBUILD_STATE_PATH') ?: "${System.getProperty('user.home')}/.mozbuild" - def files = new FileNameFinder().getFileNames(mozbuild, "android-ndk-*/source.properties") - if (files) { - // It would be nice to sort these by version, but that's too much effort right now. - return project.file(files.first()).parentFile.toString() - } - return null -} - // Whether to include compiled artifacts: `lib/**/*.so` and `assets/omni.ja`. // Multi-locale packaging wants to include compiled artifacts but *not* rebuild // them: see also `rootProject.{machStagePackage,geckoBinariesOnlyIf}`. @@ -47,156 +15,6 @@ def hasCompileArtifacts() { || System.getenv("MOZ_CHROME_MULTILOCALE") // Multi-locale packaging. } -// Get the LLVM bin folder, either from the currently used toolchain or, if -// this is an artifact build, from the Android NDK. -@Memoized -def getLlvmBinFolder() { - // If we have a clang path, just use that - if (project.mozconfig.substs.MOZ_CLANG_PATH) { - return project.file(project.mozconfig.substs.MOZ_CLANG_PATH) - .parentFile - } - - def ndkDirectory = getNDKDirectory() - if (!ndkDirectory) { - // Give up - return null; - } - - // The `**` in the pattern depends on the host architecture. We could compute them or bake them - // in, but this is easy enough. `FileNameFinder` won't return a directory, so we find a file - // and return its parent directory. - return project - .file(new FileNameFinder() - .getFileNames(ndkDirectory.toString(), "toolchains/llvm/prebuilt/**/bin/llvm-*") - .first()) - .parentFile -} - -ext.getLlvmBinFolder = { - // This is the easiest way to expose the memoized function to consumers. - getLlvmBinFolder() -} - -// Bug 1657190: This task works around a bug in the Android-Gradle plugin. When using SNAPSHOT -// builds (and possibly in other situations) the JNI library invalidation process isn't correct. If -// there are two JNI libs `a.so` and `b.so` and a new snapshot updates only `a.so`, then the -// resulting AAR will include both JNI libs but the consuming project's resulting APK will include -// only the modified `a.so`. For GeckoView, it's usually `libxul.so` that is updated. For a -// consumer (like Fenix), this leads to a hard startup crash because `libmozglue.so` will be missing -// when it is read (first, before `libxul.so`) -// -// For !MOZILLA_OFFICIAL builds, we work around this issue to ensure that when *any* library is -// updated then *every* library is updated. We use the ELF build ID baked into each library to -// determine whether any library is updated. We digest (SHA256, for now) all of the ELF build IDs -// and use this as our own Mozilla-specific "library generation ID". We then add our own -// Mozilla-specific ELF section to every library so that they will all be invalidated by the -// Android-Gradle plugin of a consuming project. -class SyncLibsAndUpdateGenerationID extends DefaultTask { - @InputDirectory - File source - - @InputFile - File extraSource - - @OutputDirectory - File destinationDir - - @InputDirectory - File llvmBin = project.ext.getLlvmBinFolder() - - // Sibling to `.note.gnu.build-id`. - @Input - String newElfSection = ".note.mozilla.build-id" - - @TaskAction - void execute() { - Path root = Paths.get(source.toString()) - - def libs = project.fileTree(source.toString()).files.collect { - Path lib = Paths.get(it.toString()) - root.relativize(lib).toString() - } - - def generationId = new ByteArrayOutputStream().withStream { os -> - // Start with the hash of any extra source. - def digest = MessageDigest.getInstance("SHA-256") - extraSource.eachByte(64 * 1024) { byte[] buf, int bytesRead -> - digest.update(buf, 0, bytesRead); - } - def extraSourceHex = new BigInteger(1, digest.digest()).toString(16).padLeft(64, '0') - os.write("${extraSource.toString()} ${extraSourceHex}\n".getBytes("utf-8")); - - // Follow with all the ordered build ID section dumps. - libs.each { lib -> - // This should never fail. If it does, there's a real problem, so an exception is - // reasonable. - project.exec { - commandLine "${llvmBin}/llvm-readobj" - args '--hex-dump=.note.gnu.build-id' - args "${source}/${lib}" - standardOutput = os - } - } - - def allBuildIDs = os.toString() - - // For detailed debugging. - new File("${destinationDir}/${newElfSection}-details").setText(allBuildIDs, 'utf-8') - - allBuildIDs.sha256() - } - - logger.info("Mozilla-specific library generation ID is ${generationId} (see ${destinationDir}/${newElfSection}-details)") - - def generationIdIsStale = libs.any { lib -> - new ByteArrayOutputStream().withStream { os -> - // This can fail, but there's little value letting stderr go to the console in - // general, since it's just noise after a clobber build. - def execResult = project.exec { - ignoreExitValue true - commandLine "${llvmBin}/llvm-readobj" - args "--hex-dump=${newElfSection}" - args "${destinationDir}/${lib}" - standardOutput = os - errorOutput = new ByteArrayOutputStream() - } - - if (execResult.exitValue != 0) { - logger.info("Mozilla-specific library generation ID is missing: ${lib}") - } else if (!os.toString().contains(generationId)) { - logger.info("Mozilla-specific library generation ID is stale: ${lib}") - } else { - logger.debug("Mozilla-specific library generation ID is fresh: ${lib}") - } - execResult.exitValue != 0 || !os.toString().contains(generationId) - } - } - - if (generationIdIsStale) { - project.mkdir destinationDir - new File("${destinationDir}/${newElfSection}").setText(generationId, 'utf-8') - - libs.each { lib -> - project.mkdir project.file("${destinationDir}/${lib}").parent - - new ByteArrayOutputStream().withStream { os -> - // This should never fail: if it does, there's a real problem (again). - project.exec { - commandLine "${llvmBin}/llvm-objcopy" - args "--add-section=${newElfSection}=${destinationDir}/${newElfSection}" - args "${source}/${lib}" - args "${destinationDir}/${lib}" - standardOutput = os - } - } - } - } else { - logger.info("Mozilla-specific library generation ID is fresh!") - } - } -} - ext.configureVariantWithGeckoBinaries = { variant -> if (hasCompileArtifacts()) { // Local (read, not 'official') builds want to reflect developer changes to diff --git a/old-configure.in b/old-configure.in index 9d995f68f3c8..56ec5441a135 100644 --- a/old-configure.in +++ b/old-configure.in @@ -245,15 +245,9 @@ if test "$GNU_CC"; then AC_MSG_RESULT([no]) LDFLAGS=$_SAVE_LDFLAGS) - # While sha1 is deterministic, it is slower. - if test -z "$DEVELOPER_OPTIONS" -o "$OS_TARGET" = "Android"; then - build_id=sha1 - else - build_id=uuid - fi - AC_MSG_CHECKING([for --build-id=$build_id option to ld]) + AC_MSG_CHECKING([for --build-id=uuid option to ld]) _SAVE_LDFLAGS=$LDFLAGS - LDFLAGS="$LDFLAGS -Wl,--build-id=$build_id" + LDFLAGS="$LDFLAGS -Wl,--build-id=uuid" AC_TRY_LINK(,,AC_MSG_RESULT([yes]), AC_MSG_RESULT([no]) LDFLAGS=$_SAVE_LDFLAGS)