[Fix] MacOS build when xcode is not installed

[EruEri]

On MacOS, there is two way to install tools you need to develop (make, git, cc, ...). EIther install Xcode or something called Command Line Tools. But the Command Line Tools version doesn't include all the necessary programs to build Apple applications.

Currently the src/Makefile.OCaml assumes that if the OSARCH is Darwin so it can build the macui but if the xcode version of xcodebuild is not installed, any usage of xcodebuild will trigger the following message and stop the build process with an error.

$ xcodebuild
xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

[gdt]

Thanks for contributing!

Could you rebase into fewer (perhaps 1 if there isn't reason for more) commits and force-push? That needs to happen anyway, so reviewers might as well look at what you think should go in, vs an intermediate state. I prefer to wait until then to read the diff, as I don't really have enough cycles for unison maintenance. (I do have a mac I can test on.)

When rebasing, please ensure that the rationale for how things are ends up in comments and the rationale for the change is fully in the commit message. The union of the commit messages doesn't read like that now ;-) The PR comments will be lost on merge.

Compiler tools on mac are a bit of a mess. I guess you are talking about having CLT but not the rest. Please make sure the explanation of the problem can be followed by someone who is a little hazy on exactly how oddly Apple does this. My belief is that even people with CS degrees who have macs are generally a little unclear on CLT/XCode and how things relate, now and in earlier versions - it seems to keep changing.

Perhaps we should instead change the logic so that the mac gui is not built by default, but only when an explicit target is requested. I'm not 100% comfortable with "mac gui is built on mac, except when it isn't".

We could also consider a better error message for the gui build, so it fails more cleanly and the invoker rapidly understands.

[EruEri]

I only keep one commit.

Yes I talk about the command line tools without xcode, sorry if I wasn't clear.

[EruEri]

Do you think we should change the default and only build the mac gui if it's explicitly intended or provided a better error message when it fails ?

[gdt]

I am not really sure. But I don't really like silently not building it if there's isn't enough xcode support. I know there is a tradition of enabling options if things are present in build systems, but as a packager that's more of a bug.

So far, we have had "build mac gui if a mac". So that argues that the lesser change is to keep doing that, and turn a hard-to-read error into "Can't build Unison.app because XCode is not installed:" or something more useful.

The big question is how normal it is to have a setup where you can build C programs but don't have XCode, and how people in that situation got there. And also how this relates to following the instructions in INSTALL.txt.
That argues that that INSTALL.txt should say that CLT is enough to build unison/unison-gui and that you need XCode for Unison.app. And then to unbundle macui from the default build.

[EruEri]

But currently it causes the opam install of unison (not unison-gui) to fail. But we can assume that if we build unison without gui we don't want to build the Unison.app

# context     2.1.6 | macos/x86_64 | ocaml.5.2.0 | https://opam.ocaml.org#25082ca0
# path        ~/.opam/default/.opam-switch/build/unison.2.53.4
# command     ~/.opam/opam-init/hooks/sandbox.sh build make NATIVE=true -j 3
# exit-code   2
# env-file    ~/.opam/log/unison-684-e03f6b.env
# output-file ~/.opam/log/unison-684-e03f6b.out
### output ###
# (cd uimac; \
# [...]
# 	 printf "MARKETING_VERSION = 2.53.4\nOCAMLLIBDIR = /Users/erueri/.opam/default/lib/ocaml\nOCAMLLIB_UNIX = -lunix${LIB_SUFFIX}\nOCAMLLIB_STR = -lcamlstr${LIB_SUFFIX}" > ExternalSettings.xcconfig)
# fsmonitor implementation is not available or not configured for this system. fsmonitor will not be built.
# ocamlopt: ubase/umarshal.ml ---> ubase/umarshal.cmx
# ocamlopt -g -I lwt -I ubase -I system -I +unix -I +str -I system/generic -I lwt/generic -c /Users/erueri/.opam/default/.opam-switch/build/unison.2.53.4/src/ubase/umarshal.ml
# ocamlopt: ubase/rx.ml ---> ubase/rx.cmx
# ocamlopt -g -I lwt -I ubase -I system -I +unix -I +str -I system/generic -I lwt/generic -c /Users/erueri/.opam/default/.opam-switch/build/unison.2.53.4/src/ubase/rx.ml
# (cd uimac; xcodebuild -arch x86_64   SYMROOT=build)
# xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance
# make[1]: *** [macexecutable] Error 1
# make[1]: *** Waiting for unfinished jobs....
# make: *** [src] Error 2

[EruEri]

One example to get in this situation is to install homebrew, homebrew requires you to install the Command line tools but after that if you are not interested in building Apple apps (IOS, MacOS, ...) you won't install XCode. Furthermore Xcode takes a lot of place, so I think if you don't need it you won't install it

[gdt]

I'd prefer to align this with "On macOS, don't build native GUI if XCode is not installed" and leave the "non-XCode version of xcodebuild" confusing detail to the detail. (The real point of 1st-line commit messages is to have the first 54 characters give enough of a clue that someone can get the big picture and know if they want to read the whole commit or not. I know I'm being picky, but that's partially for future me looking back, and partially for others :-)

[tleedjarv]

I can't comment on the Mac particulars but overall I'm fine with this patch. Just a couple of nit-picks. Move the comment into the first if just next to the second if. Double check indentation and tabs vs spaces.

[EruEri]

I move the comment, Is it what you meant ?

[gdt]

Looks good to me - thanks for engaging with review/fixups. I have enabled the workflow again to run regression tests.

[tleedjarv]

I move the comment, Is it what you meant ?

Yes, thank you.

I've only looked at the patch in GH but indendation and spaces-vs-tabs look broken to me. Please fix before it gets merged.

[gdt]

There were spaces before the tab ;-) To just finish I just removed them and forced-push.

[gdt]

Thanks again for being cooperative with our software process!

[EruEri]

There were spaces before the tab ;-) To just finish I just removed them and forced-push.

Thanks you

[EruEri]

Thanks again for being cooperative with our software process!

You are welcome, it's a pleasure to help.

Thanks you @gdt, @tleedjarv for your time.