Skip to content

Commit be6eb7f

Browse files
rmarinhoCopilot
andcommitted
Address review: versioned path discovery, CreateProcessStartInfo, EnvironmentVariableNames, exit code checks
- AvdManagerPath: enumerate versioned cmdline-tools dirs descending (mirrors SdkManager.FindSdkManagerPath pattern), then latest, then legacy tools/bin - Use ProcessUtils.CreateProcessStartInfo with separate args instead of building Arguments string manually (all 3 methods) - Use EnvironmentVariableNames.AndroidHome/.JavaHome constants instead of hard-coded strings - Check exit codes in ListAvdsAsync and DeleteAvdAsync, throw on failure - Return IReadOnlyList<AvdInfo> from ListAvdsAsync - Set Path on AvdInfo returned from CreateAvdAsync for consistency - Extract RequireAvdManagerPath helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bf25705 commit be6eb7f

1 file changed

Lines changed: 50 additions & 42 deletions

File tree

src/Xamarin.Android.Tools.AndroidSdk/Runners/AvdManagerRunner.cs

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -37,52 +37,67 @@ public string? AvdManagerPath {
3737
return null;
3838

3939
var ext = OS.IsWindows ? ".bat" : "";
40-
var cmdlineToolsPath = Path.Combine (sdkPath, "cmdline-tools", "latest", "bin", "avdmanager" + ext);
41-
if (File.Exists (cmdlineToolsPath))
42-
return cmdlineToolsPath;
43-
44-
var toolsPath = Path.Combine (sdkPath, "tools", "bin", "avdmanager" + ext);
40+
var cmdlineToolsDir = Path.Combine (sdkPath, "cmdline-tools");
41+
42+
if (Directory.Exists (cmdlineToolsDir)) {
43+
// Versioned dirs sorted descending, then "latest" as fallback
44+
var searchDirs = Directory.GetDirectories (cmdlineToolsDir)
45+
.Select (Path.GetFileName)
46+
.Where (n => n != "latest" && !string.IsNullOrEmpty (n))
47+
.OrderByDescending (n => Version.TryParse (n, out var v) ? v : new Version (0, 0))
48+
.Append ("latest");
49+
50+
foreach (var dir in searchDirs) {
51+
var toolPath = Path.Combine (cmdlineToolsDir, dir!, "bin", "avdmanager" + ext);
52+
if (File.Exists (toolPath))
53+
return toolPath;
54+
}
55+
}
4556

46-
return File.Exists (toolsPath) ? toolsPath : null;
57+
// Legacy fallback: tools/bin/avdmanager
58+
var legacyPath = Path.Combine (sdkPath, "tools", "bin", "avdmanager" + ext);
59+
return File.Exists (legacyPath) ? legacyPath : null;
4760
}
4861
}
4962

5063
public bool IsAvailable => !string.IsNullOrEmpty (AvdManagerPath);
5164

65+
string RequireAvdManagerPath ()
66+
{
67+
return AvdManagerPath ?? throw new InvalidOperationException ("AVD Manager not found.");
68+
}
69+
5270
void ConfigureEnvironment (ProcessStartInfo psi)
5371
{
5472
var sdkPath = getSdkPath ();
5573
if (!string.IsNullOrEmpty (sdkPath))
56-
psi.EnvironmentVariables ["ANDROID_HOME"] = sdkPath;
74+
psi.EnvironmentVariables [EnvironmentVariableNames.AndroidHome] = sdkPath;
5775

5876
var jdkPath = getJdkPath?.Invoke ();
5977
if (!string.IsNullOrEmpty (jdkPath))
60-
psi.EnvironmentVariables ["JAVA_HOME"] = jdkPath;
78+
psi.EnvironmentVariables [EnvironmentVariableNames.JavaHome] = jdkPath;
6179
}
6280

63-
public async Task<List<AvdInfo>> ListAvdsAsync (CancellationToken cancellationToken = default)
81+
public async Task<IReadOnlyList<AvdInfo>> ListAvdsAsync (CancellationToken cancellationToken = default)
6482
{
65-
if (!IsAvailable)
66-
throw new InvalidOperationException ("AVD Manager not found.");
83+
var avdManagerPath = RequireAvdManagerPath ();
6784

6885
using var stdout = new StringWriter ();
69-
var psi = new ProcessStartInfo {
70-
FileName = AvdManagerPath!,
71-
Arguments = "list avd",
72-
UseShellExecute = false,
73-
CreateNoWindow = true
74-
};
86+
using var stderr = new StringWriter ();
87+
var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, "list", "avd");
7588
ConfigureEnvironment (psi);
76-
await ProcessUtils.StartProcess (psi, stdout, null, cancellationToken).ConfigureAwait (false);
89+
var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken).ConfigureAwait (false);
90+
91+
if (exitCode != 0)
92+
throw new InvalidOperationException ($"avdmanager list avd failed (exit code {exitCode}): {stderr.ToString ().Trim ()}");
7793

7894
return ParseAvdListOutput (stdout.ToString ());
7995
}
8096

8197
public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, string? deviceProfile = null,
8298
bool force = false, CancellationToken cancellationToken = default)
8399
{
84-
if (!IsAvailable)
85-
throw new InvalidOperationException ("AVD Manager not found.");
100+
var avdManagerPath = RequireAvdManagerPath ();
86101
if (string.IsNullOrEmpty (name))
87102
throw new ArgumentNullException (nameof (name));
88103
if (string.IsNullOrEmpty (systemImage))
@@ -97,28 +112,22 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
97112
}
98113

99114
// Detect orphaned AVD directory (folder exists without .ini registration).
100-
// Use --force to overwrite the orphaned directory.
101115
var avdDir = Path.Combine (
102116
Environment.GetFolderPath (Environment.SpecialFolder.UserProfile),
103117
".android", "avd", $"{name}.avd");
104118
if (Directory.Exists (avdDir))
105119
force = true;
106120

107-
var args = $"create avd -n \"{name}\" -k \"{systemImage}\"";
121+
var args = new List<string> { "create", "avd", "-n", name, "-k", systemImage };
108122
if (!string.IsNullOrEmpty (deviceProfile))
109-
args += $" -d \"{deviceProfile}\"";
123+
args.AddRange (new [] { "-d", deviceProfile });
110124
if (force)
111-
args += " --force";
125+
args.Add ("--force");
112126

113127
using var stdout = new StringWriter ();
114128
using var stderr = new StringWriter ();
115-
var psi = new ProcessStartInfo {
116-
FileName = AvdManagerPath!,
117-
Arguments = args,
118-
UseShellExecute = false,
119-
CreateNoWindow = true,
120-
RedirectStandardInput = true
121-
};
129+
var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, args.ToArray ());
130+
psi.RedirectStandardInput = true;
122131
ConfigureEnvironment (psi);
123132

124133
// avdmanager prompts "Do you wish to create a custom hardware profile?" — answer "no"
@@ -127,7 +136,7 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
127136
try {
128137
p.StandardInput.WriteLine ("no");
129138
p.StandardInput.Close ();
130-
} catch (System.IO.IOException) {
139+
} catch (IOException) {
131140
// Process may have already exited
132141
}
133142
}).ConfigureAwait (false);
@@ -142,22 +151,21 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
142151
return new AvdInfo {
143152
Name = name,
144153
DeviceProfile = deviceProfile,
154+
Path = avdDir,
145155
};
146156
}
147157

148158
public async Task DeleteAvdAsync (string name, CancellationToken cancellationToken = default)
149159
{
150-
if (!IsAvailable)
151-
throw new InvalidOperationException ("AVD Manager not found.");
152-
153-
var psi = new ProcessStartInfo {
154-
FileName = AvdManagerPath!,
155-
Arguments = $"delete avd --name \"{name}\"",
156-
UseShellExecute = false,
157-
CreateNoWindow = true
158-
};
160+
var avdManagerPath = RequireAvdManagerPath ();
161+
162+
using var stderr = new StringWriter ();
163+
var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, "delete", "avd", "--name", name);
159164
ConfigureEnvironment (psi);
160-
await ProcessUtils.StartProcess (psi, null, null, cancellationToken).ConfigureAwait (false);
165+
var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken).ConfigureAwait (false);
166+
167+
if (exitCode != 0)
168+
throw new InvalidOperationException ($"Failed to delete AVD '{name}': {stderr.ToString ().Trim ()}");
161169
}
162170

163171
internal static List<AvdInfo> ParseAvdListOutput (string output)

0 commit comments

Comments
 (0)