1208160 - Unaccepted Xcode license agreement prints unhelpful "Could not find a suitable make implementation" message

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Andrew McCreight [:mccr8]]

I upgraded Xcode recently, but hadn't done the whole license dance.

When I ran ./mach build I got this:

Error running mach:

    ['build']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Exception: Could not find a suitable make implementation.

  File "/Users/amccreight/mc/python/mozbuild/mozbuild/mach_commands.py", line 382, in build
    silent=not verbose)
  File "/Users/amccreight/mc/python/mozbuild/mozbuild/base.py", line 514, in _run_make
    args = self._make_path()
  File "/Users/amccreight/mc/python/mozbuild/mozbuild/base.py", line 633, in _make_path
    raise Exception('Could not find a suitable make implementation.')

Then ./mach just sat there hanging. I had no idea what the problem was, but fortunately somebody pointed out that this means that I hadn't accepted the Xcode license agreement. It would be better if this situation was detected and some useful message was printed out.

Note that if I manually run clang at the command line I get this message, which tells me what to do: "Logical-Framework:mc amccreight$ clang: "Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo."

Comment 1

[Gregory Szorc [:gps]]

If someone can provide the actual output of make when it is prompting for the license, we can add detection for this.

Anyone?

Comment 2

[Andrew McCreight [:mccr8]]

http://stackoverflow.com/questions/26197347/agreeing-to-the-xcode-ios-license-requires-admin-privileges-please-re-run-as-r

This is like what the output I recall from clang, which presumably is the same thing as what make produces.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Show information when Xcode requires accepting license agreement.

Added code to show information when the output of make contains "Agreeing to the Xcode" (either stdout or stderr).

Tested with following command.  It might be better to test on actual environment tho.

#!/bin/sh
echo "Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo." > /dev/stderr
exit 1

Comment 4

[Tooru Fujisawa [:arai]]

tested the patch with updated Xcode.  here's the output.


Error running mach:

    ['build']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Exception: Xcode requires accepting to the license agreement.
Please run Xcode and accept the license agreement.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8671896 [details] [diff] [review]
Show information when Xcode requires accepting license agreement.

Review of attachment 8671896 [details] [diff] [review]:
-----------------------------------------------------------------

I tested this when I upgraded to 10.11 the other day and it appears to work. However, the code needs some cosmetic cleanup before it gets an r+.

::: python/mozbuild/mozbuild/base.py
@@ +603,5 @@
> +        def is_xcode_lisense_error(cmd):
> +            if not self._is_osx():
> +                return False
> +
> +            p = subprocess.Popen(cmd,

You don't need to execute the command again...

@@ +621,5 @@
>                  cmd = [make, '-f', baseconfig]
>                  if self._is_windows():
>                      cmd.append('HOST_OS_ARCH=WINNT')
>                  try:
>                      subprocess.check_call(cmd, stdout=open(os.devnull, 'wb'),

Change this to subprocess.check_output ...

@@ +624,5 @@
>                  try:
>                      subprocess.check_call(cmd, stdout=open(os.devnull, 'wb'),
>                          stderr=subprocess.STDOUT)
>                  except subprocess.CalledProcessError:
> +                    if is_xcode_lisense_error(cmd):

except subprocess.CalledProcessError as e:
    check_xcode_license(e.output)

@@ +628,5 @@
> +                    if is_xcode_lisense_error(cmd):
> +                        return ValidationResult.XCODE_LISENSE_ERROR
> +                    return ValidationResult.ERROR
> +                return ValidationResult.OK
> +            return ValidationResult.ERROR

This isn't a very Python pattern. Integer status codes are so C (and every language that cargo culted them) :)

The preferred way to do this is a) separate exceptions for each error (if raising an exception is the end goal) b) returning a tuple or other datastructure. e.g. "return False, True" to indicate "not valid make and it is an Xcode license error."

@@ +635,3 @@
>          possible_makes = ['gmake', 'make', 'mozmake', 'gnumake']
>  
>          if 'MAKE' in os.environ:

This branch is now complicated enough that I'd prefer always doing the possible_makes.insert(0, make) and checking for os.path.isabs() as part of the "for test in possible_makes" iteration below.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Show information when Xcode requires accepting license agreement.

Thank you for reviewing :)
Changed to use the way b)

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8672799 [details] [diff] [review]
Show information when Xcode requires accepting license agreement.

Review of attachment 8672799 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I'll land this for you.

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/fx-team/rev/2ea6aa72a052153f429461f54d31b56633707d97
Bug 1208160 - Show information when Xcode requires accepting license agreement; r=gps

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2ea6aa72a052