Closed taoeffect closed 10 years ago
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.
Luke raises a good point. Is there any reason to believe one way or the other regarding whether this will affect consensus?
@luke-jr Are you saying this affects any platform other than mac?
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.
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.
@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.
@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.
(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.
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.
@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!
@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.
@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. :+1:
@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.
Install dependencies with Homebrew and then from within src directory:
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.