Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to build and run on FreeBSD. #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kiwichris
Copy link

Tested on FreeBSD 11.0.

@tgorochowik
Copy link
Member

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
  1. 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?

  2. 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
Copy link
Author

kiwichris commented May 10, 2017

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
Copy link
Member

mgielda commented Jun 24, 2018

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants