Closed Bug 1208160 Opened 6 years ago Closed 5 years ago

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

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: mccr8, Assigned: arai)

References

Details

Attachments

(1 file, 1 obsolete file)

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."
Summary: ./mach build fails with unhelpful error message when upgrading Xcode → Unaccepted Xcode license agreement prints unhelpful "Could not find a suitable make implementation" message
If someone can provide the actual output of make when it is prompting for the license, we can add detection for this.

Anyone?
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.
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
Assignee: nobody → arai.unmht
Attachment #8671896 - Flags: review?(gps)
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 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.
Attachment #8671896 - Flags: review?(gps)
Thank you for reviewing :)
Changed to use the way b)
Attachment #8671896 - Attachment is obsolete: true
Attachment #8672799 - Flags: review?(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.
Attachment #8672799 - Flags: review?(gps) → review+
https://hg.mozilla.org/integration/fx-team/rev/2ea6aa72a052153f429461f54d31b56633707d97
Bug 1208160 - Show information when Xcode requires accepting license agreement; r=gps
https://hg.mozilla.org/mozilla-central/rev/2ea6aa72a052
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Duplicate of this bug: 1075238
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.