Minimal changes to get building on OS X with clang and Homebrew

[taoeffect]

Install dependencies with Homebrew and then from within src directory:

make USE_UPNP=1 -j $(sysctl -n hw.ncpu) -f Makefile.osx all

The only concern is how to install specific versions of the dependencies (to avoid accidentally resulting a hard fork).

Unfortunately, I couldn't find "db-4.7.25.NC" via Homebrew. The farthest back they have is 4.8.30. Is that OK?

Also, I haven't tested yet if boost 1.5 builds on OS X or not. 1.55 does, but I'm not sure about 1.5.

[taoeffect]

Note also that this is just for building namecoind (not the QT GUI).

[Midar]

This would be obsoleted if #135 is merged

[taoeffect]

👍 @Midar. commented

[taoeffect]

Update: this was fixed. Safe to merge!

Also, just an FYI, this does contain a breaking change for some compilers. If someone needs me to fix that just let me know and I'll figure out the macro for detecting clang compilers (instead of just commenting this section out).

[Midar]

PR #108 doesn't break compilation with non-Clang compilers BTW.

[cassiniNMC]

LIBS+= \
 -Wl,-dynamic \ 
 -lgthread-2.0 \    

Do we really need gthread?

[Midar]

The other Makefiles have that, too, but it shouldn't be necessary. My PR that introduces a proper build system doesn't have it and it works :).

[indolering]

So this does NOT work for Namecoin-QT, correct?

[taoeffect]

@indolering Correct.

[taoeffect]

I don't expect this to be merged btw. It's just an illustration of the minimal changes needed to build on OS X.

[indolering]

@taoeffect Yeah, I had it working ~8 months ago, then changes to 10.9 and the switch to Clang broke Boost.

[phelixbtc]

It seems OS X is even worse than windows in regard to building. Somebody please make it work! With GUI!
@taoeffect Windows version is also using db-4.8.30.NC

[Midar]

@phelixbtc I got it working with GUI, see my other PR.

[indolering]

@Midar https://github.com/ceptacle/libcoinqt Interested?

[indolering]

@phelixbtc could we get this PR request accepted? Namecoind is better than nothing and Midar's other build system isn't going anywhere.

[taoeffect]

What's libcoind missing that namecoind has? I like libcoind's build system better (and I got it to build on OS X, I should probably submit a PR for that too...)

[JeremyRand]

@taoeffect libcoind is missing a few things, including RPC calls for registering names. There's more info on the forum. I might be able to arrange some bounties for getting libcoind up to par with namecoind feature-wise, if that's something you're interested in.

[taoeffect]

I wouldn't be able to do it (allergic to C++), but I might be able to chip in. Grumble. What about bitcore? Can it do namecoiny things yet? I think @ryancdotorg was working on that...?

[taoeffect]

NM, @ryancdotorg said on IRC that he's not entirely sure what bitcore is... lol. And I can't figure it out either! Is it a liteclient? It doesn't seem to be a full node.

[taoeffect]

OK, so figured out what bitcore is thanks to jgarzik on #bitcore.

Bitcore is:

• A library for doing bitcoin things
• It is not a client.
• The client for bitcore seems to be copay.

Copay is:

• A "wallet".
• Not a full node, or any kind of a node for that matter.
• It uses insight-api

Insight-API seems to be:

• JSON-RPC client to bitcoind
• Plus some type of caching database on top of that (I'm guessing).

So... nm about the bitcore. Looks like all we have is namecoind and libcoin --namecoin, so it seems we'd better focus on libcoind.

[domob1812]

Sorry for the OT, but I think I should comment here regarding libcoin: Yes, exactly, the missing features are registering names and name_list. I'm actually planning to start working on these "really soon" (next week at the latest), so hopefully they will be implemented in the not too distant future. Bounties would, of course, be appreciated, but I'm determined to do it nevertheless.

[phelixbtc]

Is there a consensus this is a step in the right direction? @cassiniNMC?

@taoeffect Could you give example lines for how to install dependencies with proper versions with homebrew? Why did you not expect this to be merged?

[taoeffect]

@taoeffect Could you give example lines for how to install dependencies with proper versions with homebrew?

Like I said, the "proper versions" aren't available on Homebrew, so I used what was available there. boost 1.55 seems to be working. I also installed openssl via Homebrew, as well as berkeley-db. These were installed just using regular brew install [blah] commands (I think). Try this out and let me know if you run into problems (I'm unfortunately rather pressed for time).

Why did you not expect this to be merged?

Because this comment.

[phelixbtc]

@taoeffect OK, we should be able figure these out, thanks!

[cassiniNMC]

this does contain a breaking change for some compilers. If someone needs me to fix that just let me know and I'll figure out the macro for detecting clang compilers

@taoeffect Yes, it would be great if you could find out a suitable macro/pragma for that section in serialize.h

[taoeffect]

K. This is ready to be merged. I got rid of gthread and am requiring miniupnpc to be installed (cause... there's no reason not to), simplified Makefile.osx somewhat, and switched to berkeley-db4.

Compiled and verified.

Will crash if you have previously used this PR to run namecoind because of database downgrade to berkeley db 4!!!

You have two options:

• Figure out how to downgrade the berkeley-db from 5 to 4. (I couldn't figure it out).

Or

• Export your private keys using this command on all of the addresses you own: ./namecoind dumpprivkey <address>

If you go with the second option, you will need to delete all your database stuff and re-download. You can use my node on okturtles.com to "quickly" get up to speed (I have bandwidth to spare):

./namecoind -gen=0 -server -addnode=192.184.93.146

Then after it boots up add re-import your private keys that you exported:

./namecoind importprivkey 23rjkjf823jf98jsfj8932jfj98fw982jf982jf9fj8

(Obviously, the key above isn't a real one, just an example.)

[taoeffect]

Also, regarding build-osx.txt, instructions are simple now:

1. brew install openssl berkeley-db4 boost miniupnpc
2. cd src && make -f Makefile.osx -j $(sysctl -n hw.ncpu) all

If you know your CPU count, you can just use it: make -f Makefile.osx -j 8 all

[taoeffect]

Actually, I would have preferred to rebase (git rebase instead of git merge) your commits on top of the current branch, so as to not have a "merge" commit in the history. But it doesn't matter for now - just remember this for the future. ;)

@domob1812 I decided to not do a rebase because I had already pushed the changes publicly. You aren't supposed to rebase if you've made your stuff public (as that will cause history conflicts).

[domob1812]

@taoeffect Considering that this branch on your repo is just a "work in progress", I see no problem with rebasing it. This is also what Bitcoin devs usually want, and it makes the official repo's history cleaner. I think this counts more than following some "supposed to be" Git guidelines to-the-letter.

[luke-jr]

Won't this break Linux builds?

[taoeffect]

Um, I don't know. Is __WXMSW__ Windows only? Then maybe there should be a separate ifdef that defines it for OS X?

[luke-jr]

Ideally, autotools should detect if the platform has MSG_NOSIGNAL and define it if not.

[taoeffect]

Erm, can't that be done with a simple #ifdef...?

[phelixbtc]

#if defined(__WXMSW__) || defined(__clang__)
#define MSG_NOSIGNAL        0
#endif

I don't know about a direct define for this. Maybe using 'MAC_OSX' instead of '__ clang __' would be cleaner.

[luke-jr]

Yes, that's probably safe too -

#ifndef MSG_NOSIGNAL
#define MSG_NOSIGNAL 0
#endif

[crosser]

On 09/19/2014 10:23 AM, phelixbtc wrote:

In src/util.h:

@@ -109,12 +109,13 @@ T* alignup(T* p)
return u.ptr;
}

+#define MSG_NOSIGNAL 0

|#if defined(WXMSW) || defined(clang)
#define MSG_NOSIGNAL 0
#endif

This will break build with clang on linux, won't it?

#ifndef MSG_NOSIGNAL

define MSG_NOSIGNAL 0

#endif

is probably safer, albeit not bullet-proof. On Linux, MSG_NOSIGNAL is a enum
value, but it is accompanied with a #define, making it workable (hopefully).

[taoeffect]

OK, I actually replaced __clang__ with __APPLE__ because ironically enough that broke compilation with clang under linux. Now I've verified it compiles on OS X and Linux using clang. I wasn't able to test gcc on either platform however (OS X doesn't have it, and on linux my VPS kills the process for probably using too much RAM).

[phelixbtc]

Compiles and runs on Windows. ACK. Linux GCC?

[JeremyRand]

IMO someone should test on Linux GCC before this is merged. Maybe someone can ask pmc to look at it?

[luke-jr]

Just because something compiles does not make it correct. With consensus systems, this is even more important of a point. I advise figuring out the actual issue and fixing it specifically. Or at the very least, leave unaffected platforms unchanged.

[JeremyRand]

Luke raises a good point. Is there any reason to believe one way or the other regarding whether this will affect consensus?

[phelixbtc]

@luke-jr Are you saying this affects any platform other than mac?

[pmconrad]

https://github.com/okTurtles/namecoin/archive/1f4a78a2ab1e973b96fa6edcff4d2dbc04b10ce0.tar.gz compiles with linux/gcc + starts syncing... I don't see why non-OSX builds should be affected by the patch.

[luke-jr]

This change effectively removes the first insert method from the class on non-Apple platforms.

[taoeffect]

On OS X, the compiler errors because of multiple definitions. I can put the guard around the parameter definition if you'd prefer (as that's the only difference) and delete the duplicate body. Maybe there's a way to define the parameters correctly for all platforms, but again, I have no interest in figuring that out, I just made a few small changes to get it to compile on OS X.

[Midar]

There is no need for a guard. Just remove the correct duplicate and it should work with any compiler on any OS.

Maybe there's a way to define the parameters correctly for all platforms, but again, I have no interest in figuring that out, I just made a few small changes to get it to compile on OS X.

Then why bother at all if you only want a quick and dirty hack?!

[taoeffect]

Alternatively, it might also work to simply show the second definition on all platforms except __APPLE__, but so far this seems to work just fine (indicating only one definition is needed... per platform).

[taoeffect]

Just remove the correct duplicate and it should work with any compiler on any OS.

Removing one will break compilation on one of the platforms (regardless of which you remove), if I remember correctly.

Feel free to send your own PR on top of this one if you'd like.

Then why bother at all if you only want a quick and dirty hack?!

...I wanted "minimal changes" to get it to work on OS X. I got them.

[Midar]

It really saddens me a lot to see how much time people spend to place one hack over another. Things like these #ifdef __clang__ were not necessary with my PR, it solved everything cleanly. The amount of time you all spent here to just add a few hacks would have been enough to review my whole PR and do it the clean way and get the GUI to work on OS X. With this, there's still no GUI on OS X! It really saddens me to see how resources are wasted for a half-working hack instead of doing it cleanly - and yet people are arguing about what is correct and what not (hint: all that's proposed is not the clean way to do it), even though this is just a collection of hacks and a clean solution has been proposed and done already.

[taoeffect]

@Midar, I agree with your sentiment with the exception that I believe C++ and autoconf and all that jazz are already hacks by their very nature.

[Midar]

@taoconf autoconf is not really a hack IMHO. It's the correct way to write portable software: Instead of having checks like "Is this Linux? Is this OSX?", it actually checks if the things it needs are available. That greatly increases portability, as this means it even works on OSes about which the software does not know. The only way where I'd say this is a hack is in that there's nothing integrated with C/C++ that could do these checks, not even a #try_include which would already make a lot of checks unnecessary.

As for C++, yes, you could call it a collection of hacks. For each shortcoming of C or an old C++, a new feature was added. But this is how incremental designs work. Not everything is fully planned ahead and then never extended. In fact, almost nothing is. That's still different from adding even more hacks than necessary if there's a proper solution, though ;). Those hacks in C++ are because either a.) backwards compatibility or b.) there's no better way to do it given the constraints.

[taoeffect]

(hint: all that's proposed is not the clean way to do it), even though this is just a collection of hacks and a clean solution has been proposed and done already.

IMO, your PR is an even less clean solution (no offense). It's thousands of additional lines that so far no one has taken the time to review. A C++ node is an ugly, dirty, and potentially dangerous hack. There's no getting around that. This is what I tell everyone when they think it's a bright idea to start new projects in C++ these days. They don't listen. Then this crap happens and we spend all this time and effort on getting crappy, ugly, insecure code to run on multiple platforms. Where's the puke emoticon when you need it?

The "clean" solution would be to write this in a non-hack language.

[Midar]

The problem here is not C++, but the lack of a build system that allows to properly check for features. @luke-jr rightfully complained about those #ifdefs you added, as they are OS checks instead of feature checks. Not even do they limit portability to platforms which are known by the source, this is also a guarantee that things will break in an update of the OS. Your #define for example will break if OS X decides it's a good thing to have it and also adds it.

There are a lot of use cases where C++ is the right tool. Granted, there are better languages for a cryptocurrency, but C++ really isn't the biggest problem here. A way bigger problem is the mix of Boost and Qt, mixing two giant frameworks. Or that there's a lot of #ifdef __someos__.

Those additional lines suddenly get down to a really low number if you just compare those files that are taken from autotools with the upstream ones. It gets down to a few hundred lines. Really now, if you don't trust autotools from upstream, you can't trust your system at all. Heck, your compiler uses them, so why even trust your compiler? Anyway, the amount of time spent on this PR is already magnitudes larger than the amount of time that would have been required to review those parts of my PR that are not just copied from elsewhere (and those you can easily diff), while the gain is significantly smaller.

[phelixbtc]

@Midar Namecoin will be rebased on current Bitcoin / Libcoin. There is a consensus that we will not add a build system at this point. Please take further discussions about this to the forum or make a PR for contrib/

I appreciate all of you staying polite and it may be a bit late as we hopefully are about done here, but still: Everybody please stay on topic. Off topic posts will be deleted from now on.

@taoeffect This is just what we needed. Thank you!

[indolering]

@luke-jr @Midar We really appreciate both of your efforts on this patch, we don't want to impact non-OS X systems! Remember that we are just trying to make the minimum changes required to get this building on OS X.

[taoeffect]

@luke-jr @Midar We really appreciate both of your efforts on this patch, we don't want to impact non-OS X systems! Remember that we are just trying to make the minimum changes required to get this building on OS X.

Ditto. 👍

[Midar]

@indolering Sometimes, minimal effort equals to the most dirty solution. While my patch does touch other OSes, it does not break any. In fact, it even increases portability for other OSes.