Handle all the macOS prerequisites (except NDK/SDK) via prerequisites.py

[misl6]

✂️ Partially sliced from #2586

• Is still experimental
• Moved autoconf, automake, libtool, openssl and pkg-config
• Added some generic tests and specific ones.
• Changed the logic around mandatory and installer_is_supported
• Added SKIP_PREREQUISITES_CHECK so we can skip prerequisites checks during recipe tests.
• brew-installable prerequisites can follow a DRY approach, but we should make sure that also doesn't over-complicate the linux implementation, so I'll leave the improvements for later.

[AndreMiras]

LGTM thanks 👌
One question, is the install running by default then on macOS?
I'm wondering if we want to have this run on demand instead.
I imagine it depends on the users, but while I like things to be easy to get setup, I don't like when scripts install things system wide without my consent.
As an Arch and Gentoo user I may not be the norm here, I thing generally macOS users don't mind too much

[AndreMiras]

Suggested change

	if shutil.which("brew"):
	return True
	else:
	return False
	return shutil.which("brew")

[misl6]

I'm expecting (here and in similar ones), to need a "clean" boolean in future, so returning the sh.which("brew") ouput which is a string doesn't seem a good idea.

How about:

return shutil.which("brew") != ""

[AndreMiras]

Oh yeah my bad, you're right.
Yeah in this case you can either:

return shutil.which("brew") is not None

Or:

return bool(shutil.which("brew"))

[AndreMiras]

Why don't we inline that one too here and in the other places?

Suggested change

	if self._darwin_get_brew_formula_location_prefix("[email protected]", installed=True):
	return True
	else:
	return False
	return self._darwin_get_brew_formula_location_prefix("[email protected]", installed=True)

[AndreMiras]

Another option could be to leverage pytest skipif

Suggested change

	def test_darwin_checker_is_supported(self):
	try:
	self.prerequisite.darwin_checker()
	except Exception as e:
	if e.__str__().startswith("Unsupported prerequisite check"):
	self.fail("Darwin checker should be supported")
	pytest.mark.skipif(sys.platform != "darwin", reason="MacOS test only")
	def test_darwin_checker_is_supported(self):
	self.prerequisite.darwin_checker()

[misl6]

pytest.mark.skipif(sys.platform != "darwin", reason="MacOS test only") skips the test when not on darwin.

This one, instead, checks, if the darwin_checker is raising an error with Unsupported prerequisite check in it, as the darwin_checker, is marked as supported on this prerequisite.

[AndreMiras]

Oh right, what I don't get is why do we bother to try/except/if in this test, it's usually a red flag.
In this case why don't we simply:

assert self.prerequisite.darwin_checker()

[misl6]

With assert self.prerequisite.darwin_checker(), we only check if it fails (which is probably enough ATM), not specifically if it throws a Unsupported prerequisite check on macOS for ... exception.

BTW, re-reading the code, considering that we're testing the darwin_checker function:

python-for-android/tests/test_prerequisites.py

Lines 63 to 69 in eb343a5

	@mock.patch("shutil.which")
	def test_darwin_checker(self, shutil_which):
	shutil_which.return_value = ""
	self.assertFalse(self.prerequisite.darwin_checker())
	shutil_which.return_value = "/opt/homebrew/bin/brew"
	self.assertTrue(self.prerequisite.darwin_checker())

we can probably get rid of this test?

[AndreMiras]

Ah yeah indeed, I leave that up to you either way is fine to me

[misl6]

LGTM thanks 👌
One question, is the install running by default then on macOS?
I'm wondering if we want to have this run on demand instead.
I imagine it depends on the users, but while I like things to be easy to get setup, I don't like when scripts install things system wide without my consent.
As an Arch and Gentoo user I may not be the norm here, I thing generally macOS users don't mind too much

Here, we ask the user permission to install the prerequisite. We only skip the permission-checking when running non-interactively:
(As a macOS user which previously was on Linux, I'm also into the club of users that hates when install-scripts do an installation without informing the user )

python-for-android/pythonforandroid/prerequisites.py

Line 55 in eb343a5

if self.ask_to_install():

python-for-android/pythonforandroid/prerequisites.py

Lines 35 to 51 in eb343a5

	def ask_to_install(self):
	if (
	os.environ.get("PYTHONFORANDROID_PREREQUISITES_INSTALL_INTERACTIVE", "1")
	== "1"
	):
	res = input(
	f"Do you want automatically install prerequisite {self.name}? [y/N] "
	)
	if res.lower() == "y":
	return True
	else:
	return False
	else:
	info(
	"Session is not interactive (usually this happens during a CI run), so let's consider it as a YES"
	)
	return True

[AndreMiras]

Thanks for getting back.
And yes indeed for the interactive mode by default, we're all good.
LGTM, I'd love to see the darwin_checker() inlined, but it's probably matter of taste / nitpick, feel free to address or not

[misl6]

Thanks for getting back.
And yes indeed for the interactive mode by default, we're all good.
LGTM, I'd love to see the darwin_checker() inlined, but it's probably matter of taste / nitpick, feel free to address or not

Addressed. Thank you!