Skip to content

Conversation

@DmitriyKirakosyan
Copy link
Contributor

Google replaced 'tools' with 'cmdline-tools'
Android sdk now supports multiple 'cmdline-tools' versions installed SxS

For the sake of simplicity, using latest installed version for now.

Since new 'cmdline-tools' doesnt contain Emulator executable, replaced the searching path with Path.Combine (AndroidSdkPath, "emulator")

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1109288

Google replaced 'tools' with 'cmdline-tools'
Android sdk now supports multiple 'cmdline-tools' versions installed SxS

For the sake of simplicity, using latest installed version for now.

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1109288
Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this still work on older sdk's which just have the tools directory? Is that something we need to care about?

@JonDouglas
Copy link

@dellis1972 For context, we want to support the tools directory only if there is no cmdline-tools directory for backwards compatibility. We'll check if cmdline-tools is on disk first & use that, but if it's not fallback to tools since that folder will no longer exist.

Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok.

DmitriyKirakosyan and others added 3 commits April 30, 2020 00:23
One "semantic" change: `lint` is in `androidSdkToolsBinPath`, *not* `androidSdkToolsPath`:

```
% find cmdline-tools -iname lint
cmdline-tools/latest/bin/lint
cmdline-tools/latest/lib/lint
```
if (Directory.Exists (Path.Combine (toolsDir, "latest")))
return Path.Combine (toolsDir, "latest");

var latestToolsDir = Directory.GetDirectories (toolsDir).Max ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite semantically correct, because Max() sorts lexicographically, not ordinally:

Console.WriteLine (new[]{"1.0", "2.0", "11.0"}.Max ());
// Prints "2.0", because '2' > '1'

Fortunately latest always sorts as the "highest" value…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanpeppers that is correct, will fix it.

sdk.SetPreferredJavaSdkPath (latestJdk.HomePath);
}

public static string GetPreferredAndroidToolsPath(string androidSdkPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully the last comment on this PR, and apologies for not seeing this earlier:

Must this method be static? Most other methods on this class are instance methods, around the AndroidSdkInfo.AndroidSdkPath instance property.

I think a better method more "in keeping" with the current design of the class would be:

public IEnumerable<string> GetCommandLineToolsPaths (string preferredCommandLineToolsVersion)
{
	if (!string.IsNullOrEmpty (preferredCommandLineToolsVersion)) {
		var preferredDir = Path.Combine (AndroidSdkPath, "cmdline-tools", preferredCommandLineToolsVersion);
		if (Directory.Exists (preferredDir))
			return new[] { preferredDir }.Concat (GetCommandLineToolsPaths (null).Where (p => p != preferredDir));
	}
	return GetCommandLineToolsPaths ();
}

public IEnumerable<string> GetCommandLineToolsPaths ()
{
	var cmdlineToolsDir = Path.Combine (AndroidSdkPath, "cmdline-tools");
	if (Directory.Exists (cmdlineToolsDir)) {
		var latestDir = Path.Combine (cmdlineToolsDir, "latest");
		if (Directory.Exists (latestDir))
			yield return latestDir;
		foreach (var d in SortedSubdirectoriesByVersion (cmdlineToolsDir)) {
			var version = Path.GetFileName (d);
			if (version == "latest")
				continue;
			yield return d;
		}
	}
	var toolsDir = Path.Combine (AndroidSdkPath, "tools");
	if (Directory.Exists (toolsDir)) {
		yield return toolsDir;
	}
}

@jonpryor
Copy link
Contributor

Draft commit message subject

[Xamarin.Android.Tools.AndroidSdk] Add support for cmdline-tools (#83)

Body:

Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1109288
Context: https://dl.google.com/android/repository/repository2-1.xml

Google has deprecated the `tools` Android SDK package, replacing it
with the `cmdline-tools` package, which introduces a version
directory component.

Old and busted:

	$(AndroidSdkDirectory)/tools/bin/sdkmanager

New hotness:

	$(AndroidSdkDirectory)/cmdline-tools/latest/bin/sdkmanager

Of particular interest is that `latest` is a *literal value*.  There
is also a `cmdline-tools;1.0` package which creates a
`cmdline-tools/1.0` directory.

Add a new `AndroidSdkInfo.GetCommandLineToolsPaths()` method which
returns the "command-line tools paths", ordered by version and (non-)
obsolescence.  For example, given the directory structure:

  * `$(AndroidSdkDirectory)/tools/bin/sdkmanager`
  * `$(AndroidSdkDirectory)/cmdline-tools/1.0/bin/sdkmanager`
  * `$(AndroidSdkDirectory)/cmdline-tools/latest/bin/sdkmanager`

Then `AndroidSdkInfo.GetCommandLineToolsPaths()` will return, 
in this order:

  * `$(AndroidSdkDirectory)/cmdline-tools/latest`
  * `$(AndroidSdkDirectory)/cmdline-tools/1.0`
  * `$(AndroidSdkDirectory)/tools`

The `latest` version is always preferred, if present, followed by any
actually versioned cmdline-tools directories, followed by the `tools`
directory, if it exists.

Note that "prefixes" are returned.  All utilities are within a nested
`bin` directory, so if you want e.g. the latest `sdkmanager` util,
you would want to do:

	var info = new AndroidSdkInfo (path);
	var latestSdkManager = Path.Combine (
	        info.GetCommandLineToolsPaths ().First (),
	        "bin",
	        "sdkmanager");

@jonpryor jonpryor merged commit f5fcb9f into dotnet:master Apr 30, 2020
jonpryor pushed a commit that referenced this pull request Apr 30, 2020
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1109288
Context: https://dl.google.com/android/repository/repository2-1.xml

Google has deprecated the `tools` Android SDK package, replacing it
with the `cmdline-tools` package, which introduces a version
directory component.

Old and busted:

	$(AndroidSdkDirectory)/tools/bin/sdkmanager

New hotness:

	$(AndroidSdkDirectory)/cmdline-tools/latest/bin/sdkmanager

Of particular interest is that `latest` is a *literal value*.  There
is also a `cmdline-tools;1.0` package which creates a
`cmdline-tools/1.0` directory.

Add a new `AndroidSdkInfo.GetCommandLineToolsPaths()` method which
returns the "command-line tools paths", ordered by version and (non-)
obsolescence.  For example, given the directory structure:

  * `$(AndroidSdkDirectory)/tools/bin/sdkmanager`
  * `$(AndroidSdkDirectory)/cmdline-tools/1.0/bin/sdkmanager`
  * `$(AndroidSdkDirectory)/cmdline-tools/latest/bin/sdkmanager`

Then `AndroidSdkInfo.GetCommandLineToolsPaths()` will return, 
in this order:

  * `$(AndroidSdkDirectory)/cmdline-tools/latest`
  * `$(AndroidSdkDirectory)/cmdline-tools/1.0`
  * `$(AndroidSdkDirectory)/tools`

The `latest` version is always preferred, if present, followed by any
actually versioned cmdline-tools directories, followed by the `tools`
directory, if it exists.

Note that "prefixes" are returned.  All utilities are within a nested
`bin` directory, so if you want e.g. the latest `sdkmanager` util,
you would want to do:

	var info = new AndroidSdkInfo (path);
	var latestSdkManager = Path.Combine (
	        info.GetCommandLineToolsPaths ().First (),
	        "bin",
	        "sdkmanager");

Finally, remove some unnecessary members from `AndroidSdkBase` which
were never used -- and thus are "noise" -- and don't make sense in
the new `cmdline-tools` world, as the cmdline-tools package doesn't
contain them…
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.

4 participants