Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added includeCommon, includeLocal, includeRemote parameters for mirakle plugin extension to assign more precise rsync filtering rules set #127

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

LuigiVampa92
Copy link

Hi, thanks for your great plugin! I have a small pull request to offer. It allows to add --inlcude commandline arguments to rsync command call along with the --exclude arguments.

What problem does it solve?

I have made a small repository with scripts that allow you to seamlessly move building process of an apk from a local machine to a remote server (https://github.com/LuigiVampa92/RemoteAndroidBuilds). It uses an optimized set of rsync params that allows to pass just necessary source code from a local machine to a remote server and pass result apks back from server after the build. It also passes generated classes (build/generated) so Intellij Idea code completion and code navigation features would be able to find them and not show any errors if you build project remotely. Still the basic idea is to pass as little data as possible between the machines because in a huge project network throughput can become a very real time-consuming bottleneck.

My scripts used to work great but at a certain AndroidGradlePlugin update (7.0.2 if I figured it out right) Google decided to implicitly split the process of building an apk depending on whether it is build by calling gradle command directly or pressing the RUN button in Android Studio UI. This change broke my scripts. Historically we had a single place inside the gradle project's build directory where apks were stored - build/outputs/apk. No matter which way project was build all apks were dropped there. Now AGP uses standard build/outputs/apk output if it was built by a gradle command and another dedicated directory if it was built by pressing RUN button - build/intermediates/apk.

This is not only confusing and make apks be stored in two different locations at once, second directory is technically not a part of the result output directory, but rather a part of intermediate build files that in my scripts were completely excluded from passing from a remote server to a local machine. In a small project passing that directory is OK, but in a huge project that I have been working with it could easily be from 1 to 2 GB. And all those files have nothing to do with a result artifact that will be run and tested on a device. The build/intermediates directory contains a lot of subdirectories (and they can change with AGP updates) and it is not easy to manually exclude all of them except build/intermediates/apk that is now absolutely required to push an apk to a device or an emulator after build is complete.

Since my scripts does not originally download anything from build/intermediates, when I press RUN button nothing happens and I get an error. If I manually run a gradle command and then adb commands to upload an apk and run the app, everything works, but it is not convenient, because the whole idea was about setting up the remote builds once and forget the commandline forever, just press UI buttons and everything should work seamlessly.

Solution

So the right solution would be to make a set rules for rsync where you first explicitly include what subdirectories you need and then exclude the rest. This plugin already has an extension that allows to conveniently add --exclude rules in a gradle script, but not --include rules. So I added them in a similar way to exclude rules.

With include rules this problem about second apk directory is solved easily this way:

excludeRemote += ["**/build/intermediates/**"]
includeRemote += ["**/build/intermediates/apk/***"]
includeRemote += ["**/build/intermediates/apk_ide_redirect_file/***"]

This changes let me set precisely what I need to download from remote build/intemediates directory to run a build with RUN button, and skip other heavy stuff.

Summary

This PR does not alter any of the current behaviour, only adds an extra option to set rsync filters more precisely. Include rules will not certainly be a very popular option because they are most likely not needed for simple projects, but will certainly be beneficial for those who work with huge projects and that's what is when this plugin is most useful.

I have built and tested the changes locally and they works great.

NOTE! I haven't raised plugin version in this PR. It is still 1.6.0, so you will have to manually make a commit to raise plugin version before publishing an update.

I hope you'll merge this and I'll be able to fix my scripts and keep using your great plugin.

Thank you!

…le plugin extension to assign more precise rsync filtering rules set
@yshinkarev
Copy link

Really needed.
APK doesn't download from remote host.

@LuigiVampa92
Copy link
Author

Really needed.
APK doesn't download from remote host.

Yeap, this pull request will solve the problem you mentioned. You can cherry pick this commit and build the plugin for your local maven repository on your local computer, but that is not very convenient.

Btw, if you are interested I am currently making an Android Studio plugin that integrates Mirakle build plugin into UI (actually a fork of it, with some extra features and a few fixes to add support of windows). It adds a toggle button on a build toolbar where you can switch "build mode" between local and remote, no special scripts required, no changing the code, no adding build plugins etc - just push "remote builds on" button and hit "Run" button on a toolbar - the build will be performed on a remote server.

Here is the GitHub repo. It's empty for now, but I will upload the source code in a couple of days, when I will finish the readme and the plugin will be published in jetbrains plugin repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants