Re: Darwin target

["Yann E. MORIN" <yann dot morin dot 1998 at free dot fr>]

Yann, All,

On Wednesday 14 November 2012 Diorcet Yann wrote:
> Ray Donnelly and me have started to work on adding darwin target (OS X 
> and iOS) in crosstool-ng.
> Currently, it's already working: you can create a toolchain (C,C++, 
> Objective c) for OS X using XCode's SDKs.
> It's far to be done. Based on the Ray Donnelly's 
> work(https://github.com/mingwandroid/toolchain4), we will add a better 
> cctools/ld64 (binutils replacing for MACH-O).
> You can follow the progress on the following 
> github:https://github.com/diorcety/crosstool-ng.

OK, as promised on IRC, here's my initial review:

1) Split changes into smaller hunks.

  a) fix the efl2flt config ('if BARE_METAL' --> '## depends on BARE_METAL')
  b) ditto for the sstrip thingy
  c) add the EXTRA_{C,LD}FLAGS_FOR_{HOST,BUILD} config options
     - propagate to affected script, of course
  d) introduce the multi-binutils feature
     - don't add any new alternative, only the infrastructure to support
       multiple binutils
  e) introduce the /generic/ companion-libs build script
  f) add Darwin as a new kernel
     - depends on EXPERIMENTAL
     - propagate the "depends on !DARWIN" to affected components
  g) add cctools as alternate binutils
  h) you get the idea...

  Basically: one changeset introduces a minimal, self-contained, semantically
  coherent change. See in your linux source tree:
    Documentation/SubmittingPatches: 3) Separate your changes

2) Patches in patches/compomnent/version
   - add an explanation at the top of the patch (if the reason is not known
     but you got the patch from another source, point to that source)
   
3) gcc Apple version
   a) no need to for the 'if CC_APPLE' test; just append the location for
      the Apple version to the existing list
   b) in gcc.sh, use 'if KERNEL == DARWIN' to check if the core compilers
      are needed; thus, no need for option CC_GCC_APPLE

4) Companion libs & tools
   - what is util-linux/libuuid required for?
   - ditto openssl?

5) patches/cctools/806/0001-nondarwin.patch
   - Whaoo... 2.3 MiB for a single patch... :-(
   - where's that patch coming from?  --> add origin at top of patch
   - why is it needed?  --> add (short) explanation at top of patch
   - can it be replaced by an equivalent patch series?


Otherwise, the code looks pretty nice for a work-in-progress! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

--
For unsubscribe information see http://sourceware.org/lists.html#faq