|mach gradle-install| has run its course. It's been nice having a light-weight implementation rather than a full-fledged build backend, but it's a manual step that needs to be done at just the right time (after build and package). Since all it is doing is arranging some symlinks and preprocessing, I think we may be able to add a light-weight build subcontext that keeps $OBJDIR/mobile/android/gradle up to date. I know of two things that need to be done: * make the Gradle build lazy about copying omni.ja and .so libraries from the object directory (right now |mach gradle-install| fails if the objdir hasn't been packaged once); * implement the build-backend bits for arranging symlinks. Since files come and go from the gradle directory, this pretty much needs to "own" that directory -- removing extraneous files, ignoring some files, etc. The manifest processing needs to be improved to handle this.

Bug 1176133 - Part 1: Allow ignoring paths that would be removed by FileCopier. r?gps It's not possible (well, easy) to add a manifest type ignoring a pattern in the destination directory. This is because of when a copier tracks destination files with individual entries. Since a copier is not tied to a particular destination directory, it's not possible to walk the destination directory to determine the complete set of files to ignore. Reworking the copiers to handle patterns requires significant effort. The approach in this patch is conceptually simple and intended for internal consumers only. It adds a predicate to selectively not delete unregistered files (the ones we would like to specify with a manifest pattern). There is an unfortunate wrinkle: an unregistered file that would have been deleted can now cause its parent directory to be silently preserved. If we felt strongly, we could say the parent directory was not deleted as well.

Bug 1176133 - Part 2: Use FileCopier and filter predicate in |mach gradle-install| directly. r?gps There's no need to invoke a process here, and even if there was a reason, there's no smooth way to expose a general filter predicate to the command line.

Bug 1176133 - Part 3: Keep all preprocessed_code references in the objdir. r?sebastian This was just an oversight. The Gradle configuration referenced topsrcdir rather than having a symlink via the objdir. This didn't impact the Gradle build, but it did make the preprocessed_code Gradle project appear outside of the root Gradle project in IntelliJ.

Bug 1176133 - Part 4: Copy omni.ja, native libraries, and asset libraries from dist/fennec manually. r?sebastian This allows us to not require dist/fennec/* to exist in the object directory at gradle-install time. It gets us one small step closer to being able to sit down to a fresh source tree and open a Fennec project in IntelliJ.

Bug 1176133 - Part 5: Copy preprocessed code from objdir to build directory during Gradle build. r?sebastian This means we don't require the directory in the object directory at gradle-install time. We're not concerned if the source files are missing, since we have code to ensure they're fresh already; and if they are missing, we'll quickly fail as we try to compile with missing sources.

Bug 1176133 - Part 6: Copy preprocessed resources from objdir to build directory during Gradle build. r?sebastian This means we don't require the directory in the object directory at gradle-install time. We're not concerned if the resource files are missing, since we have code to ensure they're fresh already; and if they are missing, we'll quickly fail as we try to process the resource set.

gps: the changes don't quite get us to removing gradle-install. Follow-up patch to come when I work out the last objdir/pre-processing details. glandium: NI just so you're aware of this; I've been flagging gps for reviews somewhat arbitrarily. sebastian: mcomella: this makes |mach gradle-install| *remove* cruft that's outdated; it gets us closer to installing files like the IntelliJ project configuration XML. One step at a time.

Comment on attachment 8649398 [details] MozReview Request: Bug 1176133 - Part 3: Keep all preprocessed_code references in the objdir. r?sebastian https://reviewboard.mozilla.org/r/16389/#review14777 Ship It!

Comment on attachment 8649400 [details] MozReview Request: Bug 1176133 - Part 4: Copy omni.ja, native libraries, and asset libraries from dist/fennec manually. r?sebastian https://reviewboard.mozilla.org/r/16391/#review14787 Ship It!

Comment on attachment 8649401 [details] MozReview Request: Bug 1176133 - Part 5: Copy preprocessed code from objdir to build directory during Gradle build. r?sebastian https://reviewboard.mozilla.org/r/16393/#review14789 Ship It!

Comment on attachment 8649402 [details] MozReview Request: Bug 1176133 - Part 6: Copy preprocessed resources from objdir to build directory during Gradle build. r?sebastian https://reviewboard.mozilla.org/r/16395/#review14791 Ship It!

Comment on attachment 8649396 [details] MozReview Request: Bug 1176133 - Part 1: Allow ignoring paths that would be removed by FileCopier. r?gps https://reviewboard.mozilla.org/r/16385/#review14917 ::: python/mozbuild/mozpack/copier.py:192 (Diff revision 1) > + remove_unaccounted_predicate=None, How about remove_unaccounted_fn? "predicate" to me sounds like a noun, not a function. Perhaps you know something about computer science that I don't. ::: python/mozbuild/mozpack/copier.py:346 (Diff revision 1) > + if not callable(remove_unaccounted_predicate): > + remove_unaccounted_predicate = None Please do argument validation at top of functions. ::: python/mozbuild/mozpack/copier.py:399 (Diff revision 1) > + if remove_unaccounted_predicate: > + if not remove_unaccounted_predicate(mozpath.relpath(d, destination)): > + result.ignored_directories.add(d) > + continue FileCopier purposefully tries to be ignorant about directories and only cares about files. Passing a directory into remove_unaccounted_predicate breaks this pattern. The code above ensures that directories without files are removed. So this code only prevents subsequent writers of files in "ignored" directories from having to recreate the directory. But they presumably have to do this anyway. So I see no purpose for this complexity. ::: python/mozbuild/mozpack/test/test_copier.py:401 (Diff revision 1) > + return path in ['emptydir', 'populateddir/second'] or \ Nit: use tuples for immutable sequences. Remove emptydir from this since that code will be deleted. ::: python/mozbuild/mozpack/test/test_copier.py:413 (Diff revision 1) > + def test_remove_unaccounted_filter_leaves_directory_that_would_be_deleted(self): This can be deleted once you delete the corresponding code.

Comment on attachment 8649397 [details] MozReview Request: Bug 1176133 - Part 2: Use FileCopier and filter predicate in |mach gradle-install| directly. r?gps https://reviewboard.mozilla.org/r/16387/#review14921 I could give r+. But since you have to rework the first patch (maybe - read comments here), I'll wait to see v2. ::: mobile/android/mach_commands.py:11 (Diff revision 1) > +from mozpack.copier import FileCopier > +from mozpack.manifests import InstallManifest We defer module-level imports in mach_commands.py files for performance reasons. Please leave these imports where they were. ::: mobile/android/mach_commands.py:149 (Diff revision 1) > - manifest_path = os.path.join(self.topobjdir, 'mobile', 'android', 'gradle.manifest') > - with FileAvoidWrite(manifest_path) as f: > - m.write(fileobj=f) > + def should_delete(path): > + return not any(mozpath.match(path, pattern) for pattern in > + ('.deps/**', '.idea/**', '.gradle/**', 'build/**', '*/build/**', '**/*.iml')) This reminds me of code in InstallManifest which expands wildcards at population time to determine what files to copy/symlink. And this reminds me that up until your first commit, the rules for FileCopier were static at call time. You *could* modify the InstallManifest in this method to add ignore entries for all files that you find, using a matcher to quickly find them. Then your first commit doesn't need to exist. I'm not opposed to the functionality you added in the previous commit. But I'm not convinced it is necessary. Am I missing something? ::: mobile/android/mach_commands.py:164 (Diff revision 1) > - if not code: > + COMPLETE = 'From {dest}:\nKept {existing} existing; Added/updated {updated}; ' \ > + 'Removed {rm_files} files and {rm_dirs} directories; ' \ > + 'Ignored {ig_files} files and {ig_dirs} directories.' > + print(COMPLETE.format(dest=destdir, > + existing=result.existing_files_count, > + updated=result.updated_files_count, > + rm_files=result.removed_files_count, > + rm_dirs=result.removed_directories_count, > + ig_files=result.ignored_files_count, > + ig_dirs=result.ignored_directories_count)) Don't feel obligated to do this unless you think it is important.

I'm going to land the patches sebastian reviewed as Bug 1196970.

This is no longer needed, and has been removed \o/ I think Bug 1123416 was the death knell for this.