Upgrade macOS/Travis default image to xcode6.4#6
Upgrade macOS/Travis default image to xcode6.4#6jakirkham merged 12 commits intoconda-forge:masterfrom
Conversation
cfep-02.md
Outdated
| ## Rationale | ||
|
|
||
| * The current default image (`beta-xcode6.1`) is deprecated | ||
| and will cease to be supported/provided by Travis [@jakirkham: citation needed], |
There was a problem hiding this comment.
@inducer -- here is the ref: travis-ci/travis-ci#6765 (comment)
|
This seems inevitable to me. One reasonable approach to testing (not saying I actually have time to implement it, unfortunately) might be to build a couple test packages on xcode 6.4 that target 10.9, and use the xcode 6.1 while it is still around to try running test suites for those packages. That won't catch everything, but at some point we are really constrained in what we can do by what the CI services make it easy to do. |
|
Matt Craig notifications@github.com writes:
Thanks, updated! |
| * [Travis image support policy][travis] | ||
|
|
||
| [impl]: https://github.com/conda-forge/conda-forge.github.io/issues/249#issuecomment-256207392 | ||
| [travis]: https://github.com/travis-ci/travis-ci/issues/6765#issuecomment-256703076 |
There was a problem hiding this comment.
Other relevant references.
cfep-02.md
Outdated
| for that image) by hand. | ||
|
|
||
| * Even with this change, this would implicitly drop support for | ||
| macOS 10.7 and 10.8. |
There was a problem hiding this comment.
We never have had support for these as we were always using the beta-xcode6.1 image, which only has the 10.9 and 10.10 SDKs.
|
jakirkham notifications@github.com writes:
Added, thx! |
cfep-02.md
Outdated
|
|
||
| * We won't be able to compile [clang | ||
| 3.8/3.9](https://github.com/conda-forge/staged-recipes/pull/1481) without | ||
| this change. |
There was a problem hiding this comment.
Also in need of this image are
| ``` | ||
|
|
||
| * Adapt `conda smithy` so that it applies that edit on rerender. | ||
| * Apply the change in the `staged-recipes` master branch. |
There was a problem hiding this comment.
Would link this PR ( conda-forge/staged-recipes#1094 ).
| osx_image: xcode6.4 | ||
| ``` | ||
|
|
||
| * Adapt `conda smithy` so that it applies that edit on rerender. |
cfep-02.md
Outdated
|
|
||
| ## Backward Compatibility | ||
|
|
||
| * The default deployment target for `xcode6.4` is macOS 10.11. |
There was a problem hiding this comment.
Should be 10.10 as it doesn't have the 10.11 SDK FWICT.
|
|
||
| * The current default image (`beta-xcode6.1`) is deprecated | ||
| and will cease to be [supported/provided by Travis][travis], | ||
| so this change will have to happen at some point anyway. |
There was a problem hiding this comment.
We have until January 20th, 2017. Would actually include that date verbatim as I don't think many people know this.
|
@jakirkham Please feel free to push directly to the text. Added you to the repo. |
|
|
||
| * Adapt `conda smithy` so that it applies that edit on rerender. | ||
| * Apply the change in the `staged-recipes` master branch. | ||
| * Rebuild all packages with this? (TBD? Opinions?) |
There was a problem hiding this comment.
Don't think this is required. However, the most recent document I can find on C++ ABI compatibility is discussing 10.3.8 and before vs. 10.3.9 and after. Those OSes (and their associated compilers) are so ancient this seems irrelevant, but have included the link anyways.
|
Thanks for writing all of this up, @inducer ! This is very helpful. Hopefully this will help get this important discussion moving. |
|
Would also appreciate your feedback on this. |
|
@jakirkham Thanks for your comments. I think I've got most of the massaged into the text. |
|
+1 |
| * [@jakirkham on implications][impl] | ||
| * [Travis image support policy][travis] | ||
| * [Travis CI changes default to Mac OS 10.11 XCode 7.3]( https://blog.travis-ci.com/2016-10-04-osx-73-default-image-live/ ) | ||
| * [Travis CI image restructuring]( https://blog.travis-ci.com/2016-09-15-new-default-osx-image-coming/ ) |
There was a problem hiding this comment.
Probably should add this recent one too.
xref: https://blog.travis-ci.com/2016-11-17-retiring-some-osx-images/
|
jakirkham notifications@github.com writes:
Done. |
|
Also brought this up at an informal meeting last week. It should come up in the formal meeting this week. I'd recommend anyone interested in this issue to go if they can. Unfortunately it falls on Thanksgiving US. So if people are unable to make it, I completely understand. Sadly I know I won't be able to make it either. Details linked below. xref: https://conda-forge.hackpad.com/conda-forge-meetings-2YkV96cvxPG#:h=2016-11-24:-General-Discussion |
|
I've started seeing this message on Travis: which suggests that time is of the essence, and Travis' previous messaging about 6.1 being safe until January might no longer be accurate. I also opened conda-forge/conda-smithy#391, sibling to conda-forge/staged-recipes#1094, which can be merged once this is agreed upon. And based on the info linked here, I wrote this script for inspecting the deployment target of dylibs, inspired a bit by |
|
|
||
| * The default deployment target for `xcode6.4` is macOS 10.10. | ||
| It would need to be forced to 10.9 (the lowest supported one | ||
| for that image) by hand. |
There was a problem hiding this comment.
"by hand" is perhaps an overstatement - the toolchain package (mentioned above) already does this, and staged-recipes already includes a note in the build section that any package that compiles things should pull in toolchain as a build dependency.
There was a problem hiding this comment.
After some discussion at our meeting yesterday, we have added PR ( conda-forge/conda-forge-build-setup-feedstock#46 ), which forces the minimum build target on OS X to 10.9. While personally I still recommend the toolchain package and it does have broad usage throughout conda-forge, having the deployment target set in conda-forge-build-setup guarantees that everything builds in a compatible way. This should help further reduce the burden on feedstock maintainers by setting compatibility at the beginning of each build.
|
OSX development isn't my primary area of expertise, but FWIW I think this is a great idea. |
|
Also have been seeing that @minrk . It is unclear to me whether this is a warning that they added to all the images being deprecated (of which there are several) or if it has meaning in this case. I think we still get the image until January. However, the date it drops has become a bit fuzzy. |
|
To help ease some concerns, the Travis CI xref: travis-ci/travis-ci#6765 (comment) cc @minrk |
|
@jakirkham thanks for checking! I think this should be a pretty safe upgrade to make. Any recipe that pulls in toolchain should already be set. The main thing to do is to ensure that, as packages are updated, make sure toolchain is pulled in as a dependency for those that don't have this already. We can do this at the human level (easiest, highest chance of occasional broken package on 10.9-only), or wait until a verification script like the one I linked above can be run automatically on build on OS X. I'm happy to start a PR for that, but I'm not sure yet which is the right repo to do it. |
|
What's the criterion for officially calling this 'accepted'? Is there an official voting process? IMO, this is mostly a no-brainer, since the choice is mainly between "Do we wait a month until Travis forces our hand, or do we do this step-by-step on our own terms?' |
pelson
left a comment
There was a problem hiding this comment.
This looks great to me @inducer. Thanks for taking the time to write this down. We still lack governance on "consensus", but given there is a cliff edge approaching, we need to time-bound the process so that we have a decision in the next few weeks (i.e. before 2017) so that we can start to implement/percolate the change across conda-forge.
The huge unknown is still
* Rebuild all packages with this? (TBD? Opinions?)
Likely not necessary given C++ ABI Compatibility.
If we can resolve this one piece of information definitively, we will know roughly how much work this whole proposal will entail (and indeed, how much work is lurking for us if we do drop off the cliff on the 20th Jan).
cfep-02.md
Outdated
| * Rebuild all packages with this? (TBD? Opinions?) | ||
|
|
||
| Likely not necessary given [C++ ABI Compatibility][cxxabi]. | ||
| * Suggest that all new packages include the `toolchain` so that |
There was a problem hiding this comment.
I am against the use of any un-versioned package that can change the build characteristics like that. We already have have feedstocks that have to override those variables to build properly. IMO those belong in the recipe and are, ultimately, the maintainers decision.
With that said I no longer have a stake on this as we diverged from conda-forge a long time ago, since the toolchain was first added. Unfortunately most of earth-science software we build locally must use gcc and must provide libgomp and others at run time to ensure they will run anywhere.
There was a problem hiding this comment.
So what do you suggest? Would you prefer a versioned dependency on toolchain? (And isn't each maintainer still at liberty to decide whether they want a versioned or unversioned dependency?) Or should each maintainer figure out setting things like the build target by hand? (And who catches it if someone forgets?)
There was a problem hiding this comment.
`> So what do you suggest?
Recommend but do not enforce. Many add the toolchain without even knowing what is the impact of those flags to the builds.
And who catches it if someone forgets?
Correct me if I am wrong but the build target is not too bad if an old one is used, right?
Anyways, like I said above, I no longer have a stake on this and I don't really care what is decided here. I am following @pelson's own workflow to re-build everything for my day job. Also, many of the users that had issues with the toolchain, that broke the GIS stack, already left conda-forge.
There was a problem hiding this comment.
I still advocate the toolchain and it is versioned. We can pin the versions in the feedstocks. We can even add the pinned version to the pinning script. That all being said, I feel like Filipe and I have had this exact conversation over and over without a different result. So am not eager to dive into it again.
Instead I would like to find a reasonable middle ground that everyone can agree on and is easy to implement. As we do actually need to set the Mac OS X deployment target. My proposal would be to set the MACOSX_DEPLOYMENT_TARGET in conda-forge-build-setup as show in PR ( conda-forge/conda-forge-build-setup-feedstock#46 ). @msarahan helped me with a PR to whitelist the MACOSX_DEPLOYMENT_TARGET in conda-build. So this will work with conda-build 2.0.12+.
I think this proposal is nice for a few reasons. First it sets the deployment target everywhere immediately. So we don't have to worry about people who are unaware of this change accidentally breaking compatibility. Second it consolidates the effort between upgrading to conda-build 2 and upgrading the Travis CI image. We need conda-build 2 to leverage this feature. So the two efforts should not be "competing" for time, but working together. Third it moves the discussion away from compiler preferences to maintaining compatibility of binaries. As compatibility is the real concern we are dealing with here, it makes sense to make that the focus and this seems to the easiest most straightforward way to do that.
There was a problem hiding this comment.
👍 I've removed the toolchain wording and replaced it by a summary of what you've suggested.
| in packages that do not meet conda-forge's promise of OS compatibility | ||
| back to 10.9. | ||
|
|
||
| * Multiple packages need this to move forward: |
There was a problem hiding this comment.
Could you add a one-sentence context that summarises the need?
|
What's our timeline to resolve this? January 21 is only a few weeks away |
|
I put it on the meeting agenda. |
cfep-02.md
Outdated
|
|
||
| <table> | ||
| <tr><td> Title </td><td> Upgrade default macOS Travis image to `xcode6.4` </td> | ||
| <tr><td> Status </td><td> Draft </td></tr> |
There was a problem hiding this comment.
Do we need to change the status?
jakirkham
left a comment
There was a problem hiding this comment.
LGTM
Not sure if we need to change the status to "Accepted", but this looks like a complete proposal ready to merge.
|
Thanks for your work on this, @inducer . |
|
Thanks everyone. |
|
Updated the header to reflect acceptance. |
|
Any final thoughts, @conda-forge/core ? |
|
Looks great! |
|
Alright, given as we have 10 days to get this sorted and I'm not hearing any complaints about this revision, am going to go ahead and merge it. Though if there are corrections, please feel free to raise issues and PRs. |
|
Thanks @inducer for putting this together and thanks everyone for the thorough review. 👍 |

Here's a quick straw man for a CFEP to get the Travis image changed.
cc @jakirkham @conda-forge/core