Venkatesh Srinivas (OP)
Newbie
 Offline
Activity: 18
Merit: 0
|
 |
July 12, 2011, 12:44:13 AM |
|
Hi,
I have a small series of patches that allow bitcoind to build on DragonFly BSD; what would be an acceptable way to submit these patches upstream? It looks like I should make a github fork of the bitcoin/bitcoin repository, push the patches on the master branch of the clone, and submit a pull request.
Some patches are specific to DragonFly BSD; others correct minor problems that are more generic; should I submit them as one patch with one pull request? Multiple patches with one pull request? Multiple patches with multiple pull requests?
Thanks,
-- vs
|
|
|
|
|
|
|
|
|
|
|
|
The grue lurks in the darkest places of the earth. Its favorite diet is adventurers, but its insatiable appetite is tempered by its fear of light. No grue has ever been seen by the light of day, and few have survived its fearsome jaws to tell the tale.
|
|
|
Advertised sites are not endorsed by the Bitcoin Forum. They may be unsafe, untrustworthy, or illegal in your jurisdiction.
|
|
|
|
|
|
|
ghotir
Newbie
Offline
Activity: 16
Merit: 0
|
|
July 14, 2011, 09:37:36 AM |
|
I'd be interested in those patches for my DragonFlyBSD install.
|
|
|
|
|
|
Vladimir
|
|
July 14, 2011, 09:42:23 AM |
|
you could simply make/change freebsd port
|
-
|
|
|
ghotir
Newbie
Offline
Activity: 16
Merit: 0
|
|
July 14, 2011, 01:02:37 PM |
|
you could simply make/change freebsd port
DragonFlyBSD uses the NetBSD pkgsrc tree, so if the OP has already done some ground work, why reinvent the wheel? On that note- OP should contact the pkgsrc-wip project about getting it submitted: https://lists.sourceforge.net/lists/listinfo/pkgsrc-wip-discuss |
|
|
|
|
marino
Newbie
Offline
Activity: 1
Merit: 0
|
|
July 14, 2011, 02:56:36 PM |
|
Vladimir,
I think vsrinivas is trying update the code base such that the change only has to be made once. Your suggestion is to add the patches to the pkgsrc package (rather than the FreeBSD port) and leave the code base alone, and risk that the patches eventually stop working.
If it's not possible to fix the code base, fine, your suggestion would work as a last resort, but it's better to attempt to fix it properly.
|
|
|
|
|
|
Vladimir
|
|
July 14, 2011, 03:32:45 PM |
|
fair points, thank you.
|
-
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
July 14, 2011, 05:25:33 PM |
|
Some of the patches are more general than just DragonFly support:
1) The bitcoind make target in makefile.unix defines USE_UPNP to 0; however net.cpp among others uses '#ifdef' as the test for miniupnp, which means the upnp headers are always required.
2) main.cpp has:
-char pchMessageStart[4] = { 0xf9, 0xbe, 0xb4, 0xd9 };
g++ 4.4.5 on DragonFly disapproves; the constants are being narrowed from int -> char inside an array initializer. I think the error is correct, though harsh.
3) g++ on DragonFly wants -std=c++0x or -std=gnu++0x; I agree with it -- typed enumerations are used in the source. I don't know why the same g++ version on Linux doesn't require it, though.
4) in db.cpp, make_tuple is meant to resolve to boost::make_tuple; at least in our configuration, it resolves to std::make_tuple, which is not correct. I don't know enough to say what correct behavior is when a template is defined in more than one namespace (std and boost) and both are 'using namespace <x>'-ed. But qualifying it helps.
DragonFly specific stuff:
1) Currently net.cpp, there is an assumption that all BSD systems support SO_NOSIGPIPE; DragonFly doesn't. The test should probably be for SO_NOSIGPIPE.
|
|
|
|
|
ghotir
Newbie
Offline
Activity: 16
Merit: 0
|
|
July 20, 2011, 11:31:09 AM |
|
Some of the patches are more general than just DragonFly support:
1) The bitcoind make target in makefile.unix defines USE_UPNP to 0; however net.cpp among others uses '#ifdef' as the test for miniupnp, which means the upnp headers are always required.
2) main.cpp has:
-char pchMessageStart[4] = { 0xf9, 0xbe, 0xb4, 0xd9 };
g++ 4.4.5 on DragonFly disapproves; the constants are being narrowed from int -> char inside an array initializer. I think the error is correct, though harsh.
3) g++ on DragonFly wants -std=c++0x or -std=gnu++0x; I agree with it -- typed enumerations are used in the source. I don't know why the same g++ version on Linux doesn't require it, though.
4) in db.cpp, make_tuple is meant to resolve to boost::make_tuple; at least in our configuration, it resolves to std::make_tuple, which is not correct. I don't know enough to say what correct behavior is when a template is defined in more than one namespace (std and boost) and both are 'using namespace <x>'-ed. But qualifying it helps.
DragonFly specific stuff:
1) Currently net.cpp, there is an assumption that all BSD systems support SO_NOSIGPIPE; DragonFly doesn't. The test should probably be for SO_NOSIGPIPE.
I knew about the SO_NOSIGPIPE issue ( supported in FreeBSD >5 only. Being that DragonFly is based on the 4.x series i can understand why it's not; but also read somewhere that Open and Net don't either ) The db.cpp boost issue has been driving me absolutely nuts (usually I can figure such things out with a little poking around make files, etc.). Anyway, if it's working for you - compiled, all that, I'd be willing to be a 2nd to apply the patches and build on my dragonflybsd system.  |
|
|
|
|
Gavin Andresen
Legendary
Offline
Activity: 1652
Merit: 2216
Chief Scientist
|
|
August 07, 2011, 03:08:34 PM |
|
Two pull requests seem appropriate: one for the generic issues (talk with TheBlueMatt about the upnp #define, I believe it is working as designed), and one for DragonFly-specific stuff.
Frankly, DragonFlyBSD-specific stuff is unlikely to get pulled; there just aren't enough DragonFly-BSD systems to justify the work of maintaining support for it (according to bsdstats.org it isn't a very popular BSD variant).
|
How often do you get the chance to work on a potentially world-changing project?
|
|
|
JoelKatz
Legendary
Offline
Activity: 1596
Merit: 1012
Democracy is vulnerable to a 51% attack.
|
|
August 07, 2011, 03:36:46 PM |
|
The bitcoind make target in makefile.unix defines USE_UPNP to 0; however net.cpp among others uses '#ifdef' as the test for miniupnp, which means the upnp headers are always required. I believe this is intentional. USE_UPNP=0 means that UPNP is not used by default but can be enabled. It's a three-way switch. 2) main.cpp has:
-char pchMessageStart[4] = { 0xf9, 0xbe, 0xb4, 0xd9 };
g++ 4.4.5 on DragonFly disapproves; the constants are being narrowed from int -> char inside an array initializer. I think the error is correct, though harsh. Really? That's kind of a nutty error. Are you sure you aren't using non-standard warning flags? GCC will issue crazy warnings if you enable all possible warnings, even "unsigned char j[10], k[10]; j[ i ]^=k[ i ];" will trigger an int->char warning. |
I am an employee of Ripple. Follow me on Twitter @JoelKatz
1Joe1Katzci1rFcsr9HH7SLuHVnDy2aihZ BM-NBM3FRExVJSJJamV9ccgyWvQfratUHgN
|
|
|
|
wumpus
|
|
August 07, 2011, 03:47:12 PM |
|
-char pchMessageStart[4] = { 0xf9, 0xbe, 0xb4, 0xd9 };
g++ 4.4.5 on DragonFly disapproves; the constants are being narrowed from int -> char inside an array initializer. AFAIK the problem is not the narrowing from int to char (which is perfectly allowed if it fits). The problem is that plain "char" has undefined signedness in C (signed in most compilers these days). So values larger than >0x80 throw an overflow warning. The correct thing would be to use "unsigned char" or uint8_t. |
Bitcoin Core developer [PGP] Warning: For most, coin loss is a larger risk than coin theft. A disk can die any time. Regularly back up your wallet through File → Backup Wallet to an external storage or the (encrypted!) cloud. Use a separate offline wallet for storing larger amounts. |
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 07, 2011, 04:06:25 PM |
|
>> char pchMessageStart[4] = { 0xf9, 0xbe, 0xb4, 0xd9 };
>> g++ 4.4.5 on DragonFly disapproves; the constants are being narrowed from int -> char inside an array initializer.
>AFAIK the problem is not the narrowing from int to char (which is perfectly allowed if it fits). The problem is that plain "char" has undefined signedness in C (signed in most compilers these days). So values larger than >0x80 throw an overflow warning. The correct thing would be to use "unsigned char" or uint8_t.
You're right; I misunderstood the error message:
main.cpp:1773: error: narrowing conversion of '249' from 'int' to 'char' inside { }
unsigned char would be correct.
It also turns out that the SO_NOSIGPIPE issue affects NetBSD and OpenBSD, both of which don't have SO_NOSIGPIPE, but do define BSD. So none of the issues are DragonFly specific.
|
|
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 11, 2011, 05:20:04 AM |
|
Okay, looks like all of the patches have been merged into bitcoind!
Thanks,
|
|
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 11, 2011, 04:23:11 PM |
|
To build bitcoind on DragonFly BSD (or NetBSD), you can now grab a copy of the git tree for bitcoin. From pkgsrc, you'll need to install Berkeley DB (db4); I used version 4.8, but 4.6 (also in pkgsrc, as db46) should work. You'll also need boost; I installed: boost-1.46.1, boost-libs-1.46.1, and boost-headers-1.46.
You'll need to make a few changes to makefile.unix before building:
To LIBS, I added -L/usr/pkg/lib ; replace with wherever your pkgsrc libraries are located. In LIBS, I also replaced -ldb_cxx with -ldb4_cxx.
To DEFS, I added -std=gnu++0x and -I/usr/pkg/include and -I/usr/pkg/include/db4 ; these changes don't really belong in DEFS, but its an easy place to add them.
After that, use GNU make with makefile.unix; bitcoind works and is able to download the blockchain and make transactions. This has been tested on DragonFly 2.10 / i386; the same steps should apply to NetBSD, but I haven't had a chance to try it.
Thanks!
|
|
|
|
|
phma
Newbie
Offline
Activity: 11
Merit: 0
|
|
August 19, 2011, 12:37:23 AM |
|
I'm getting an error; there are two files called serialize.h, and apparently it's getting the wrong one. The error is: # gmake -f ./makefile.unix bitcoind
g++ -c -O2 -Wno-invalid-offsetof -Wformat -g -D__WXDEBUG__ -DNOPCH -DFOURWAYSSE2 -DUSE_SSL -std=gnu++0x -I/usr/pkg/include -I/usr/pkg/include/db46 -DUSE_UPNP=0 -o obj/nogui/db.o db.cpp
In file included from headers.h:91,
from db.cpp:5:
serialize.h: In function 'void Serialize(Stream&, const T&, long int, int) [with Stream = CDataStream, T = std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>]':
serialize.h:1081: instantiated from 'CDataStream& CDataStream::operator<<(const T&) [with T = std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>]'
db.cpp:643: instantiated from here
serialize.h:392: error: 'const class std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>' has no member named 'Serialize'
gmake: *** [obj/nogui/db.o] Error 1 There are /usr/include/sys/serialize.h and bitcoin-0.3.24/src/src/serialize.h. I'm using DragonFly 2.11. I modified makefile.unix as follows: CXX=g++
WXINCLUDEPATHS=$(shell wx-config --cxxflags)
WXLIBS=$(shell wx-config --libs)
USE_UPNP:=0
DEFS=-DNOPCH -DFOURWAYSSE2 -DUSE_SSL -std=gnu++0x -I/usr/pkg/include -I/usr/pkg/include/db46
# for boost 1.37, add -mt to the boost libraries
LIBS= \
-Wl,-Bstatic -L/usr/pkg/lib \
-l boost_system \
-l boost_filesystem \
-l boost_program_options \
-l boost_thread \
-l db46_cxx \
-l ssl \
-l crypto
ifdef USE_UPNP
LIBS += -l miniupnpc
DEFS += -DUSE_UPNP=$(USE_UPNP)
endif
LIBS+= \
-Wl,-Bdynamic \
-l gthread-2.0 \
-l z \
-l dl \
-l pthread There's also a db5 package. I tried it and got an error, switched to db46, and still got an error. |
|
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 19, 2011, 06:06:55 AM |
|
Let me just post my Makefile here: # Copyright (c) 2009-2010 Satoshi Nakamoto # Distributed under the MIT/X11 software license, see the accompanying # file license.txt or http://www.opensource.org/licenses/mit-license.php. CXX=g++ WXINCLUDEPATHS=$(shell wx-config --cxxflags) WXLIBS=$(shell wx-config --libs) USE_UPNP:=0 DEFS=-std=gnu++0x -DNOPCH -DUSE_SSL # for boost 1.37, add -mt to the boost libraries LIBS= \ -L/usr/pkg/lib \ -Wl,-Bstatic \ -l boost_system \ -l boost_filesystem \ -l boost_program_options \ -l boost_thread \ -l db4_cxx \ -l ssl \ -l crypto DEFS += -UUSE_UPNP -I/usr/pkg/include -I/usr/pkg/include/db4 LIBS+= \ -Wl,-Bdynamic \ -l gthread-2.0 \ -l z \ -l pthread DEBUGFLAGS=-g -D__WXDEBUG__ CXXFLAGS=-O2 -Wno-invalid-offsetof -Wformat $(DEBUGFLAGS) $(DEFS) HEADERS=headers.h strlcpy.h serialize.h uint256.h util.h key.h bignum.h base58.h \ script.h db.h net.h irc.h keystore.h main.h wallet.h rpc.h uibase.h ui.h noui.h \ init.h crypter.h OBJS= \ obj/util.o \ obj/script.o \ obj/db.o \ obj/net.o \ obj/irc.o \ obj/keystore.o \ obj/main.o \ obj/wallet.o \ obj/rpc.o \ obj/init.o \ obj/crypter.o \ cryptopp/obj/sha.o \ cryptopp/obj/cpu.o all: bitcoin obj/%.o: %.cpp $(HEADERS) $(CXX) -c $(CXXFLAGS) $(WXINCLUDEPATHS) -DGUI -o $@ $< cryptopp/obj/%.o: cryptopp/%.cpp $(CXX) -c $(CXXFLAGS) -O3 -o $@ $< bitcoin: $(OBJS) obj/ui.o obj/uibase.o $(CXX) $(CXXFLAGS) -o $@ $^ $(WXLIBS) $(LIBS) obj/nogui/%.o: %.cpp $(HEADERS) $(CXX) -c $(CXXFLAGS) -o $@ $< bitcoind: $(OBJS:obj/%=obj/nogui/%) $(CXX) $(CXXFLAGS) -o $@ $^ $(LIBS) obj/test/%.o: test/%.cpp $(HEADERS) $(CXX) -c $(CFLAGS) -o $@ $< test_bitcoin: obj/test/test_bitcoin.o $(CXX) $(CFLAGS) -o $@ $(LIBPATHS) $^ $(LIBS) -lboost_unit_test_framework clean: -rm -f bitcoin bitcoind test_bitcoin -rm -f obj/*.o -rm -f obj/nogui/*.o -rm -f obj/test/*.o -rm -f cryptopp/obj/*.o -rm -f headers.h.gch |
|
|
|
|
phma
Newbie
Offline
Activity: 11
Merit: 0
|
|
August 26, 2011, 03:58:39 AM |
|
I had to remove crypter from the Makefile (there's no such file in the source) and change "db4" to "db46". I still get an error relating to serialize: # gmake -f ./makefile.dfly bitcoind
g++ -c -O2 -Wno-invalid-offsetof -Wformat -g -D__WXDEBUG__ -std=gnu++0x -DNOPCH -DUSE_SSL -UUSE_UPNP -I/usr/pkg/include -I/usr/pkg/include/db46 -o obj/nogui/util.o util.cpp
g++ -c -O2 -Wno-invalid-offsetof -Wformat -g -D__WXDEBUG__ -std=gnu++0x -DNOPCH -DUSE_SSL -UUSE_UPNP -I/usr/pkg/include -I/usr/pkg/include/db46 -o obj/nogui/script.o script.cpp
g++ -c -O2 -Wno-invalid-offsetof -Wformat -g -D__WXDEBUG__ -std=gnu++0x -DNOPCH -DUSE_SSL -UUSE_UPNP -I/usr/pkg/include -I/usr/pkg/include/db46 -o obj/nogui/db.o db.cpp
In file included from headers.h:91,
from db.cpp:5:
serialize.h: In function 'void Serialize(Stream&, const T&, long int, int) [with Stream = CDataStream, T = std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>]':
serialize.h:1081: instantiated from 'CDataStream& CDataStream::operator<<(const T&) [with T = std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>]'
db.cpp:643: instantiated from here
serialize.h:392: error: 'const class std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, long long unsigned int>' has no member named 'Serialize'
gmake: *** [obj/nogui/db.o] Error 1
|
|
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 26, 2011, 04:55:13 AM |
|
Which version of bitcoin are you using? crypter is present in git master.
Also, what Berkeley DB package do you have? /usr/pkg/include/db4 was provided by db4-4.8.30 from pkgsrc.
|
|
|
|
|
phma
Newbie
Offline
Activity: 11
Merit: 0
|
|
August 26, 2011, 08:16:47 PM |
|
db46-4.6.21
bitcoin-0.3.24
From the build-unix file:
You need Berkeley DB 4.7. Don't use 4.8, the database/log0000* files
are incompatible.
How hard would it be to make the build process use cmake or the like so that this sort of mess wouldn't happen?
|
|
|
|
|
Venkatesh Srinivas (OP)
Newbie
Offline
Activity: 18
Merit: 0
|
|
August 26, 2011, 09:05:18 PM |
|
Incompatible with what? If you download the blockchain from within the client and don't need to share the db with precompiled clients, 4.8 works fine.
|
|
|
|
|
|
