Skip to content

Commit 36ada37

Browse files
author
Stewart Miles
committed
Added support for synchronous resolution.
* Added RunOnMainThread.TryExecuteAll to allow users to pump the job queue when a job is scheduled from the main thread. * Fixed re-entrant behavior in RunOnMainThread when running a series of jobs on the main thread in batch mode. This caused the stack to grow for each executed job. * Modified CommandLine and CommandLineDialog classes to support synchronous execution. * Added integration test case for synchronous resolution. * Deprecated UnityCompat.InBatchMode and removed duplicate implementation in JarResolverLib. UnityCompat.InBatchMode is kept in place for now as it's a public API in use by some other plugins. Fixes #120 Bug: 112757664 Change-Id: I380bab695fdf98e70a0a5dbbeeeb6b95b2db7e6d
1 parent 574b945 commit 36ada37

File tree

7 files changed

+159
-57
lines changed

7 files changed

+159
-57
lines changed

source/JarResolverLib/src/Google.JarResolver/PlayServicesSupport.cs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -99,26 +99,6 @@ public enum LogLevel {
9999
"or the \"Unity > Preferences > External Tools\" menu option on OSX. " +
100100
"Alternatively, set the ANDROID_HOME environment variable");
101101

102-
/// <summary>
103-
/// Delegate used to determine whether an AAR should be exploded.
104-
/// </summary>
105-
/// <param name="aarPath">Path to the AAR file to examine.</param>
106-
/// <returns>True if the AAR should be exploded, false otherwise.</returns>
107-
public delegate bool ExplodeAar(string aarPath);
108-
109-
/// <summary>
110-
/// Whether the editor was launched in batch mode.
111-
/// </summary>
112-
internal static bool InBatchMode {
113-
get {
114-
#if UNITY_EDITOR
115-
return System.Environment.CommandLine.Contains("-batchmode");
116-
#else
117-
return true;
118-
#endif // UNITY_EDITOR
119-
}
120-
}
121-
122102
// Map of common dependencies to Android SDK packages.
123103
private static List<KeyValuePair<Regex, string>> CommonPackages =
124104
new List<KeyValuePair<Regex, string>> {
@@ -226,7 +206,7 @@ internal static PlayServicesSupport CreateInstance(
226206
/// logging enabled.</param>
227207
internal static void Log(string message, LogLevel level = LogLevel.Info,
228208
bool verbose = false) {
229-
if (logger != null && (!verbose || verboseLogging || InBatchMode)) {
209+
if (logger != null && (!verbose || verboseLogging)) {
230210
logger(message, level);
231211
}
232212
}

source/PlayServicesResolver/src/CommandLine.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ public static StreamData Empty
119119
/// </summary>
120120
/// <param name="toolPath">Tool to execute.</param>
121121
/// <param name="arguments">String to pass to the tools' command line.</param>
122-
/// <param name="completionDelegate">Called when the tool completes.</param>
122+
/// <param name="completionDelegate">Called when the tool completes. This is always
123+
/// called from the main thread.</param>
123124
/// <param name="workingDirectory">Directory to execute the tool from.</param>
124125
/// <param name="envVars">Additional environment variables to set for the command.</param>
125126
/// <param name="ioHandler">Allows a caller to provide interactive input and also handle
@@ -132,10 +133,10 @@ public static void RunAsync(
132133
Action action = () => {
133134
Result result = Run(toolPath, arguments, workingDirectory, envVars: envVars,
134135
ioHandler: ioHandler);
135-
completionDelegate(result);
136+
RunOnMainThread.Run(() => { completionDelegate(result);});
136137
};
137138
if (ExecutionEnvironment.InBatchMode) {
138-
action();
139+
RunOnMainThread.Run(action);
139140
} else {
140141
Thread thread = new Thread(new ThreadStart(action));
141142
thread.Start();

source/PlayServicesResolver/src/CommandLineDialog.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,7 @@ public void CommandLineToolCompletion(CommandLine.Result result)
104104
String.Format("Command completed: {0}", result.message),
105105
level: LogLevel.Verbose);
106106
this.result = result;
107-
if (ExecutionEnvironment.InBatchMode) {
108-
RunOnMainThread.Run(() => { SignalComplete(); });
109-
}
107+
SignalComplete();
110108
}
111109

112110
/// <summary>

source/PlayServicesResolver/src/PlayServicesResolver.cs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ namespace GooglePlayServices
2020
using System.Collections.Generic;
2121
using System.IO;
2222
using System.Text.RegularExpressions;
23+
using System.Threading;
2324
using System.Xml;
2425
using Google;
2526
using Google.JarResolver;
@@ -815,6 +816,20 @@ private static void OnPostprocessAllAssets(string[] importedAssets,
815816
}
816817
}
817818

819+
/// <summary>
820+
/// If auto-resolution is enabled, run resolution synchronously before building the
821+
/// application.
822+
/// </summary>
823+
[UnityEditor.Callbacks.PostProcessSceneAttribute(0)]
824+
private static void OnPostProcessScene() {
825+
if (Resolver.AutomaticResolutionEnabled()) {
826+
Log("Starting auto-resolution before scene build...");
827+
bool success = ResolveSync(false);
828+
Log(String.Format("Android resolution {0}.", success ? "succeeded" : "failed"),
829+
level: LogLevel.Verbose);
830+
}
831+
}
832+
818833
/// <summary>
819834
/// Schedule auto-resolution.
820835
/// </summary>
@@ -1145,6 +1160,16 @@ private static bool DeleteFiles(IEnumerable<string> filenames)
11451160
return deletedFiles;
11461161
}
11471162

1163+
/// <summary>
1164+
/// Signal completion of a resolve job.
1165+
/// </summary>
1166+
/// <param name="completion">Action to call.</param>
1167+
private static void SignalResolveJobComplete(Action completion) {
1168+
resolutionJobs.RemoveAll((jobInfo) => { return jobInfo == null; });
1169+
completion();
1170+
ExecuteNextResolveJob();
1171+
}
1172+
11481173
/// <summary>
11491174
/// Execute the next resolve job on the queue.
11501175
/// </summary>
@@ -1190,6 +1215,31 @@ public static void Resolve(Action resolutionComplete = null,
11901215
}, false);
11911216
}
11921217

1218+
/// <summary>
1219+
/// Resolve dependencies synchronously.
1220+
/// </summary>
1221+
/// <param name="forceResolution">Whether resolution should be executed when no dependencies
1222+
/// have changed. This is useful if a dependency specifies a wildcard in the version
1223+
/// expression.</param>
1224+
/// <returns>true if successful, false otherwise.</returns>
1225+
public static bool ResolveSync(bool forceResolution) {
1226+
bool successful = false;
1227+
var completeEvent = new ManualResetEvent(false);
1228+
ScheduleResolve(forceResolution, (success) => {
1229+
successful = success;
1230+
completeEvent.Set();
1231+
}, false);
1232+
// We poll from this thread to pump the update queue if the scheduled job isn't
1233+
// executed immediately.
1234+
while (true) {
1235+
RunOnMainThread.TryExecuteAll();
1236+
if (completeEvent.WaitOne(100 /* 100ms poll interval */)) {
1237+
break;
1238+
}
1239+
}
1240+
return successful;
1241+
}
1242+
11931243
/// <summary>
11941244
/// Schedule resolution of dependencies. If resolution is currently active this queues up
11951245
/// the requested resolution action to execute when the current resolution is complete.
@@ -1206,25 +1256,25 @@ private static void ScheduleResolve(bool forceResolution,
12061256
bool isAutoResolveJob) {
12071257
bool firstJob;
12081258
lock (resolutionJobs) {
1209-
firstJob = resolutionJobs.Count == 0;
1210-
12111259
// Remove the scheduled action which enqueues an auto-resolve job.
12121260
RunOnMainThread.Cancel(autoResolveJobId);
12131261
autoResolveJobId = 0;
12141262
// Remove any enqueued auto-resolve jobs.
12151263
resolutionJobs.RemoveAll((jobInfo) => {
12161264
return jobInfo != null && jobInfo.IsAutoResolveJob;
12171265
});
1266+
firstJob = resolutionJobs.Count == 0;
12181267

12191268
resolutionJobs.Add(
12201269
new ResolutionJob(
12211270
isAutoResolveJob,
12221271
() => {
12231272
ResolveUnsafe(resolutionComplete: (success) => {
1224-
if (resolutionCompleteWithResult != null) {
1225-
resolutionCompleteWithResult(success);
1226-
}
1227-
ExecuteNextResolveJob();
1273+
SignalResolveJobComplete(() => {
1274+
if (resolutionCompleteWithResult != null) {
1275+
resolutionCompleteWithResult(success);
1276+
}
1277+
});
12281278
},
12291279
forceResolution: forceResolution);
12301280
}));
@@ -1300,7 +1350,7 @@ private static void ResolveUnsafe(Action<bool> resolutionComplete = null,
13001350
succeeded ? "Succeeded" : "Failed",
13011351
lastError), level: LogLevel.Verbose);
13021352
if (resolutionComplete != null) {
1303-
resolutionComplete(succeeded);
1353+
RunOnMainThread.Run(() => { resolutionComplete(succeeded); });
13041354
}
13051355
});
13061356
});
@@ -1377,7 +1427,9 @@ internal static void OnSettingsChanged() {
13771427
}
13781428
previousInstallAndroidPackages =
13791429
GooglePlayServices.SettingsDialog.InstallAndroidPackages;
1380-
PlayServicesSupport.verboseLogging = GooglePlayServices.SettingsDialog.VerboseLogging;
1430+
PlayServicesSupport.verboseLogging =
1431+
GooglePlayServices.SettingsDialog.VerboseLogging ||
1432+
ExecutionEnvironment.InBatchMode;
13811433
logger.Verbose = GooglePlayServices.SettingsDialog.VerboseLogging;
13821434
if (Resolver != null) {
13831435
PatchAndroidManifest(UnityCompat.ApplicationId, null);

source/PlayServicesResolver/src/UnityCompat.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ public static int GetAndroidTargetSDKVersion() {
119119
/// <summary>
120120
/// Returns whether the editor is running in batch mode.
121121
/// </summary>
122+
[ObsoleteAttribute("InBatchMode is obsolete, use ExecutionEnvironment.InBatchMode instead")]
122123
public static bool InBatchMode { get { return ExecutionEnvironment.InBatchMode; } }
123124

124125
private static Type AndroidSDKToolsClass {

source/PlayServicesResolver/test/resolve_async/Assets/PlayServicesResolver/Editor/TestResolveAsync.cs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,15 @@ static TestResolveAsync() {
171171
null, testCase, testCaseComplete);
172172
}
173173
},
174+
new TestCase {
175+
Name = "ResolveForGradleBuildSystemSync",
176+
Method = (testCase, testCaseComplete) => {
177+
ClearDependencies();
178+
SetupDependencies(testCase, testCaseComplete);
179+
Resolve("Gradle", false, "ExpectedArtifacts/NoExport/Gradle",
180+
null, testCase, testCaseComplete, synchronous: true);
181+
}
182+
},
174183
new TestCase {
175184
Name = "ResolveForInternalBuildSystem",
176185
Method = (testCase, testCaseComplete) => {
@@ -386,10 +395,10 @@ private static void ExecuteNextTestCase() {
386395
/// Called when the Version Handler has enabled all managed plugins in a project.
387396
/// </summary>
388397
public static void VersionHandlerReady() {
398+
UnityEngine.Debug.Log("VersionHandler is ready.");
389399
Google.VersionHandler.UpdateCompleteMethods = null;
390400
// If this has already been initialize this session, do not start tests again.
391401
if (SetInitialized()) return;
392-
UnityEngine.Debug.Log("Plugin is ready.");
393402
// Start executing tests.
394403
ExecuteNextTestCase();
395404
}
@@ -478,9 +487,11 @@ private static void SetupDependencies(TestCase testCase,
478487
/// should be selected.</param>
479488
/// <param name="testCase">Object executing this method.</param>
480489
/// <param name="testCaseComplete">Called with the test result.</param>
490+
/// <param name="synchronous">Whether the resolution should be executed synchronously.</param>
481491
private static void Resolve(string androidBuildSystem, bool exportProject,
482492
string expectedAssetsDir, string targetAbis,
483-
TestCase testCase, Action<TestCaseResult> testCaseComplete) {
493+
TestCase testCase, Action<TestCaseResult> testCaseComplete,
494+
bool synchronous = false) {
484495
// Set the Android target ABIs.
485496
AndroidAbisCurrentString = targetAbis;
486497
// Try setting the build system if this version of Unity supports it.
@@ -527,11 +538,18 @@ private static void Resolve(string androidBuildSystem, bool exportProject,
527538
});
528539
}, true);
529540
};
530-
Google.VersionHandler.InvokeStaticMethod(
531-
AndroidResolverClass, "Resolve", args: null,
532-
namedArgs: new Dictionary<string, object>() {
533-
{"resolutionCompleteWithResult", completeWithResult}
534-
});
541+
if (synchronous) {
542+
bool success = (bool)Google.VersionHandler.InvokeStaticMethod(
543+
AndroidResolverClass, "ResolveSync", args: new object[] { true },
544+
namedArgs: null);
545+
completeWithResult(success);
546+
} else {
547+
Google.VersionHandler.InvokeStaticMethod(
548+
AndroidResolverClass, "Resolve", args: null,
549+
namedArgs: new Dictionary<string, object>() {
550+
{"resolutionCompleteWithResult", completeWithResult}
551+
});
552+
}
535553
}
536554

537555
/// <summary>

0 commit comments

Comments
 (0)