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

RFC: full darwin build #78

Closed
wants to merge 4 commits into from
Closed

Conversation

rivimey
Copy link
Contributor

@rivimey rivimey commented Apr 15, 2018

Folks,
I have been working for several days on building bareos on MacOS using cmake and the CLion IDE, and the attached patches are the result. It does basically work... for me anyway!

I marked this as RFC because I expect you to say "that's too big"; I know. The purpose of this PR is to ask how you would like it broken up, and if the overall direction is suitable.

My intention is for this PR not to be accepted but to create from it one or more other PRs which can be.

Prerequisites:

  • brew install cmake openssl readline
    Optional
  • brew install jansson sqlite
  • brew install --devel mariadb

Current areas of concern:

  • I've been struggling somewhat to get the include files in the right order: on mac we need to use 'brew' packages a fair bit and they must be -I and -L included before system versions. General guidance on the approach would be welcome.
  • there is a #define conflict between python 2.7 include files and lex.h which I have endeavored to fix by renaming symbols like T_NAME to BCT_NAME (bareos conf token), but perhaps have broken it further instead!
  • I based the darwin port on the freebsd port, including the "init.d" files which are not useful. Not quite sure how best to proceed because this is a packaging issue: creating a DMG file with an installl script using launchd would be best but I've no experience there.
  • mac native "rpcgen" doesn't behave quite the same as the cmake rules expect, which is why there are changes in that area. In particular it leaves the new files in CWD (unless "-o") and doesn't support the -M flag.
  • I have disabled gettext (HAVE_NLS) because it caused issues. Possibly this can be reinstated using brew's gettext package but I've not tried.

Other notable changes:

  • gitignore for the files generated by template
  • additional cmake logging
  • work on cmake readline: on osx we need gnu readline not the mac "readline" (which is really libedit) and similarly we need openssl instead of native libssl;
  • mac/cmake uses "dylib" extension only for SHARED libraries; for MODULE libraries it uses "so", so the bareos plugins should not be .dylibs
  • the device code in spool.cc causes clang to emit a warning. I've "patched" it by adding a comment, but I am inclined to agree with the compiler.
  • fixed an issue in message.cc where a stack-based buffer was printed without being written to, resulting in random chars written.
  • ndmconn_xdr_nmb was calling a function with fewer args than the compiler wanted. This is a known issue with a known fix (pass 0).
  • I have added some comments - NOTUSED etc - for the DEVICE class where CLion/clang said the definition was not implemented or not used or both. I don't know if the inspection is reliable, but thought it might be useful to raise these.

@joergsteffens
Copy link
Member

joergsteffens commented Apr 16, 2018

Hello Ruth,

thank you for your contribution. After only a quick look to your commit, I've seen a couple of thinks a really like and see as an advantage for Bareos in general, not only the Mac version.

But let me start with some basic questions:

there is a #define conflict between python 2.7 include files and lex.h which I have endeavored to fix by renaming symbols like T_NAME to BCT_NAME (bareos conf token), but perhaps have broken it further instead!

Yes, I have seen it before, but ignored it. Your approach looks good so far.

"init.d"

we use plist for bareos-fd. This should also be possible for bareos-dir and bareos-sd.

fixed an issue in message.cc where a stack-based buffer was printed without being written to, resulting in random chars written.

Excellent. Do you want to create a separate PR for this? Maybe moving the initialization more to the top of the function. We can then accept this immediately and add you to the contributors.

mac/cmake uses "dylib" extension only for SHARED libraries; for MODULE libraries it uses "so", so the bareos plugins should not be .dylibs

okay, good to know.

I will discuss the cmake changes with @pstorz later this week.

regards,
Jörg

@rivimey
Copy link
Contributor Author

rivimey commented Apr 16, 2018

After only a quick look to your commit, I've seen a couple of thinks a really like and see as an advantage for Bareos in general, not only the Mac version.

I am glad it looks interesting.

you use a darwin subdirectory. Currently we use the osx subdirectory for Darwin/MacOS X, see https://github.com/bareos/bareos/tree/master/platforms/osx, used to build the MacOS DMG file. Also see http://formulae.brew.sh/formula/bareos-client. However, these are not yet adapted to bareos >= 18 (cmake), but still are set to autoconf (bareos <= 17).
is there a reason you use darwin instead?

I am aware of the prior 'osx' platform and its use for client builds, but I used "darwin" because that's what cmake identified the target as. I am relatively new to cmake so didn't want to fight the system. If there is a reason to change I have no objection.

currently we only provide the bareos client (bareos-fd and bconsole) for Darwin. Are you really plan to use the Bareos Director and SD on Darwin? Do you really see a need for it?

I did wonder myself but I decided to see what issues building everything would throw up. In the end, the include directory ordering issues, closely followed by rpcgen was the most painful to sort. Mostly other things built without issue (NB: I don't have a test suite).

My personal reason for doing this was that my dev machine is Mac and I want to make some changes to the director, and decided to try using CLion on mac rather than set up a VM for Linux. My server is indeed Ubuntu Linux.

It seems to me though that having all platforms on the same build system, and at least capable of the same results, is a win.

if yes,
is it typical, that people have tape drives/tape libraries connected to it? (if not, we could skip the chio-changer)

I haven't looked into that area. If the changer is an issue to port it would make sense to me to avoid it.

do you see the need for NDMP on Mac? Otherwise we could skip the NDMP part, included the rpcgen stuff. Your current changes break the Linux build (see https://travis-ci.org/bareos/bareos/builds/366891161?utm_source=github_status&utm_medium=notification)

I think the rpcgen fail can be fixed easily enough. I don't know enough about NDMP to know when it's useful in any context :-)

I'll throw together some smaller, distinct PRs in the next few days and see how they go.

@joergsteffens
Copy link
Member

I integrated 5 of your PRs into our internal master. I planed to push them to github immediately, however found out that the Bareos team is currently reorganize their git repositories.
On the plus side, in the near future there will be only one Bareos repository, and not different ones for regression, webui, ...
Another advantage in the near future will be, that we will drop our internal git repository and only use the one at github.
Anyhow, the disadvantage is, that it will be around 2 weeks until we can update the github repository, as some preparation are required, so the merge of your PRs will be first seen then.

@rivimey
Copy link
Contributor Author

rivimey commented Apr 19, 2018

Thanks Joerg, that's great. I obviously have not created a distinct PR for the mac/darwin port, partly because I was waiting to see what else happened and partly because of the whole macosx/darwin thing.

  • Are you in principle happy to accept such a PR if I was to make one?
  • Could you or another team member have a look to see whether I have made any obvious goofs in the changes made in this PR? As I said earlier, I am not a cmake expert and it seems there are as usual many paths to select from.
  • Would it be better to wait for the grand reorg before any of this happens?

@joergsteffens
Copy link
Member

Hello Ruth,

everything that improves Bareos and do not break anything is welcome. If I see this correctly the remaining of this PR can be separated in 4 parts:

  • cmake additions specific for Darwin
  • cmake additions to find libraries
  • copying platforms/freebsd to platforms/Darwin
  • NDMP

cmake additions specific for Darwin:
these should not hurt.

cmake additions to find libraries:
looks okay for me, but @pstorz should check this.

copying platforms/freebsd to platforms/Darwin:
I think, duplicating the freebsd code does not gain us anything. I don't think these files are required on Darwin. I think it would be better, to move the platform/osx directory to platforms/darwin and make copies of platforms/osx/files/org.bareos.bareos-fd.plist.in for the Director and SD. I also noticed, that during the change from autoconf to cmake, the original osx Makefile gets lost, see https://github.com/bareos/bareos/blob/bareos-17.2/platforms/osx/Makefile.in

While the Bareos will continue to provide a locally build Mac Client, both Makefile and plist files are not required when build using homebrew.

NDMP:
I saw, you created the /src/ndmp/ndmos_darwin.c file to be able to compile Bareos with NDMP. @pstorz had to comment on this. I can't see why this files are needed. I propose to skip NDMP for the moment or at least make this a separate PR.

Would it be better to wait for the grand reorg before any of this happens?

Until now, your changes have been easy to integrate. Only some minor conflicts. As the remaining of this PR only modifies cmake files and adds some new files, I don't think you have to wait.
If you plan to make more changes to the C files, you better wait.

I assign this PR for the remaining questions to @pstorz.

@joergsteffens
Copy link
Member

One more thing: the preferred database backend of Bareos is Postgresql, not mysql or sqlite.

@pstorz
Copy link
Member

pstorz commented Jun 19, 2018

Hello,
is this still relevant?

As we have merged the interesting parts I would like to close this PR.

Best regards

Philipp

@rivimey
Copy link
Contributor Author

rivimey commented Jun 20, 2018

I didn't get to the darwin-specific parts of the change. If its still useful I will try to tease them into shape and submit separately.

@rivimey rivimey closed this Jun 20, 2018
@rivimey rivimey deleted the darwin-second_try branch August 9, 2018 23:43
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.

None yet

3 participants