What's the status on this?

@dakl this needs to be reviewed and tested. I don't personally have access to a mac, so I cannot test or do any of the sanity checks I would normally for a code review. Here are some details that might help things along:

Review

Anyone can do a code review. If you have access to a mac that is better since you can quickly address small questions yourself.

Things to consider for any reviewer (in order):

1. What is the proposed change trying to fix? Is the approach a good one? - I'm not familiar with the file that was edited but it looks like it is only used for the build of the executable that may be used to speed up path resolution. It doesn't look like this is related to the Python build.
2. Is there anything basically wrong, e.g. syntax or typos? - One note, shobj-conf is a shell script but the fix here looks like it is trying to invoke a subcommand using ${}. Not sure what shell that is compatible with but it isn't bash. Maybe $() was meant instead?
3. Is the fix compatible with all relevant macOS and XCode versions? What versions was it tested on?
4. What are the failure scenarios and how are they handled? e.g. xcrun not available, --show-sdk-path invalid option. Would there at least be something in the logs so we'd be able to debug it if someone has an issue?

Testing

Since this repo does not currently have a CI setup that tests builds, we rely on people on various platforms and configurations to report whether the fix worked for them. A 👍 is nice, but a comment stating that you were able to pull this PR locally and successfully compile (including your macOS and XCode version) is better.

Thanks for commenting, @chrahunt!

I'll just respond to a couple points with clarification:

2. If I'm being honest, ${} is just syntax I copied from elsewhere in this file. From what I can tell it is standard bash syntax for variable expansion (different from $(), which evaluates a command in a subshell). It seems likely to me that if this were problematic, that would have surfaced before now.
   https://stackoverflow.com/questions/27472540/difference-between-and-in-bash

Whoops, I have read through my code and my answer again, and I recognize the error you were calling out! ${} is not appropriate for the xcrun invocation, which is a command and needs $() syntax. Thanks @chrahunt!

4. I think this is a good question to raise. I'm not a MacOS guru, and I'm not sure what exactly the conditions are for xcrun to be available. However, I figured it was better to propose a solution and start the conversation than leave things in their current, broken state.

I really want to get this fixed, so I will try to find some time to test it since I am a full time macOS user.

I just need to figure out how to undo the installation of the sdk to the legacy location that I did previously:

sudo installer -pkg /Library/Developer/CommandLineTools/Packages/macOS_SDK_headers_for_macOS_10.14.pkg -target /

On 10.14.1 this patch still exhibits bad pyenv install behavior
CFLAGS="-I$(xcrun --show-sdk-path)/usr/include" pyenv install -v 3.6.7 works fine, whereas pyenv install on this branch still fails with zlib error

I do not have the global sdk installation Josh mentions above, and I'm happy to test any iterations you might have to fix this

Thanks @jeffb4! I will try to find some time!

Can you please add the change from #1333 and make sure the tests pass? Thanks!

Firstly: really sorry to let this sit so long, haven't done much python at work recently, which means my related side work also takes a hit.

Secondly: The approach in this PR doesn't work because if you trace the path of the scripts calling shobj-conf (which is what's modified here) upward, you get one path:

test/run
    -> src/configure
        -> shobj-conf

So basically at no point during pyenv install is the file modified here called.

I'm working on this right now actually, have it working, but it needs polishing (helpful errors if there is no xcrun, potentially falling back to homebrew zlib, idk)

Closing in favor of #1339

Thank you for your valiant effort, @andrew-humu 😄