search

antmicro / zynq-mkbootimage

An open source replacement of the Xilinx bootgen application.
BSD 2-Clause "Simplified" License
93 stars 47 forks source link

Add support to build and run on FreeBSD. #4

Open kiwichris opened 7 years ago

kiwichris commented 7 years ago

Tested on FreeBSD 11.0.

tgorochowik commented 7 years ago

Hello Chris,

Thank you for your contribution! We are very glad to know you use mkbootimage on FreeBSD!

We will gladly accept your contribution, but there are some remarks though:

  1. The code has to be split to several commits, e.g:

    • Re-write arg parsing using getopt (I understand argp is problematic on FreeBSD?) The commit message should say why is it being changed.
    • Make it compile on FreeBSD (+ the README)
    • Add the --quiet parameter

  2. Why exactly are the EXTRA_INCLUDES and the LDPATHS variables needed? They are filled by hand while issuing gmake anyway. Is there a reason why the standard ones (CFLAGS for specifying include dirs, and LDFLAGS for specyfing lib dirs) cannot be used?

  3. We need the --version parameter to be kept. We use it for our CI. It is of course okay to print the version in usage too but we don't want the parameter removed. If you want to keep it in usage, then it probably could be prettier, the name and version could be printed first, and then the doc, and probably without a "space" character after the new line - just some minor adjustments.

Thanks! Tom

kiwichris commented 7 years ago

Hi Tom,

Many thanks for the quick response. An even bigger thanks for developing this tool and making it available.

  1. I am happy to split the patch up. I tossed the patch in to make sure the approach was fine.

  2. I am happy to test the standard flags and use them. FYI on FreeBSD PCRE and libelf is installed under /usr/local and not under the OS directories.

  3. Ah OK I did not know there was a --version option happening. I am happy to add this and to make sure it is compatible with the existing option.

mgielda commented 6 years ago

Hi @kiwichris, did you have time to revisit this? Would be great to support FreeBSD, though we don't use it normally ourselves so it would be splendid if you could include @tgorochowik's suggestions and test the changes yourself. You could even go and split this into several PRs, I'm sure some of the changes (like adding a --quiet parameter etc.) are less controversial than others. Also, the README and Makefile have changed here since we last discussed this, so splitting this into a few PRs makes even more sense.

kiwichris commented 6 years ago

Hi, sure I can look at this. It has sat on the back burner for a bit then I got busy, you know usual story, but I would like to get FreeBSD support merged. The RTEMS project also supports MacOS and Windows (MSYS2) so these being supported would be good.