GobySoft / goby

The Goby Underwater Autonomy Project
Other
26 stars 11 forks source link

Out of source build not consistent (was: Undefined symbol when using goby/acomms/protobuf/dccl_option_extensions.proto) #10

Closed berkowski closed 8 years ago

berkowski commented 8 years ago

Merge of pull request #7 introduced the following runtime error in protobuf messages importing from goby/acomms/protobuf/dccl_option_extensions.proto:

./a.out: symbol lookup error: /opt/goby/lib/libgoby_acomms.so.26: undefined symbol: _ZN4goby4util22DynamicProtobufManager5inst_E

You can reproduce the problem using the following script:

# Save as test.sh
cat <<EOF > simple_message.proto 
//simple_message.proto
//Works using this import
//import "dccl/protobuf/option_extensions.proto";

// Using this import creates the problem
import "goby/acomms/protobuf/dccl_option_extensions.proto";

message SomeMessage {
    required int32 some_field=1;
}
EOF

/usr/bin/protoc -I/usr/include -I$PWD --cpp_out=$PWD $PWD/simple_message.proto

echo "int main() { return 0; }" | /usr/bin/c++  simple_message.pb.cc -lgoby_common -lgoby_acomms -lgoby_pb -lgoby_util -ldccl -lprotobuf -x c++ -

./a.out && echo "OK"

The script runs fine using the first protobuf import line import "dccl/protobuf/option_extensions.proto":

$./test.sh
OK

However the 2nd import line produces the following:

$./test.sh
./a.out: symbol lookup error: /opt/goby/lib/libgoby_acomms.so.26: undefined symbol: _ZN4goby4util22DynamicProtobufManager5inst_E

Reverting back to 3cf85e9aea7f56635aa73ee81d13fc084d92546c fixes the problem.

tsaubergine commented 8 years ago

I can't replicate this with the existing script and current head of Goby (276c74e32ee7e0f7851949795f0db5e99f87edd9) and DCCL (GobySoft/dccl@ea7d817c02a6670a6e8fc428551fb36708abd3f2).

Both options (either "goby/acomms/protobuf/dccl_option_extensions.proto" or "dccl/protobuf/option_extensions.proto") compile correctly and report "OK".

Check to ensure that Goby was rebuilt and reinstalled (make; make install)?

That symbol (_ZN4goby4util22DynamicProtobufManager5inst_E) no longer exists in the updated goby_util (post #7) so it appears that the version of libgoby_acomms used in the example above was compiled against old headers.

berkowski commented 8 years ago

Ok, I think I may know what's going on.

Goby tags the actual library filenames with the short-version SHA of the current sources. If you update the repository without cleaning the build directory first you end up with multiple libraries -- it doesn't update the 'old' built version because it actually isn't the same target. Multiple copies then get installed into the system with make install

Furthermore, since Goby places built targets outside of the build directory (e.g. libraries get placed in goby/lib, not goby/build/lib) just nuking the build directory isn't enough to prevent this from messing things up. make clean does remove the targets as desired.

I had been removing my build/ directory before rebuilding, but I hadn't been running make clean. Given that the target names change with each commit I'm not sure make clean would fix it either.

tsaubergine commented 8 years ago

If you're linking against libgoby_acomms.so (-lgoby_acomms) this is a symlink and thus should always point to the latest built actual library (e.g. libgoby_acomms.so.2.1.1+git276c74e). Thus I'm still confused as to why this would cause the problem you saw above.

Either way, I agree that "make clean" should clean all the files in goby/lib, so I'll put a patch for that shortly.

tsaubergine commented 8 years ago

11 should fix the make clean issue.

Should never (in my experience at least) need to remove build/*. If you want to have a clean CMake cache, just remove "CMakeCache.txt"

berkowski commented 8 years ago

I'll remove build/* if I want to make absolutely sure that everything will be reconfigured/rebuilt as I intend. To me that's the whole point of out-of-source builds; everything gets placed under a single directory. The fact that goby/include, goby/bin, etc are all in the .gitignore file made it harder to track down where things actually were getting placed.

I did get lazy by not running make clean; bad assumption on my part there. I'm not sure how the symlinks weren't getting moved to the correct library target either, but making sure you actually start from a clean slate does seem to fix it.

tsaubergine commented 8 years ago

Yes, you have a valid point re: out of source builds. I'd like to maintain backwards compatibility. How does this sound: move builds to: build/lib build/bin build/include

And then add these symlinks (automatically with CMake as part of the build process): lib -> build/lib bin -> build/bin include -> build/include

(where "build" is really ${CMAKE_BINARY_DIR}, which defaults to "build" using "build.sh")

(If it isn't clear, I want a FHS style directory structure somewhere in the Goby build so that I can build dependent projects without having to do a "make install", and to add binaries to the path using goby/bin)

berkowski commented 8 years ago

Works for me! I should be more vigilante about using make clean; rm -r is a dangerous crutch!

tsaubergine commented 8 years ago

I've put up #12 to clean up the build/* business. You're not first to bring this up, but I've finally found the time to fix this (hopefully right).

You'll probably have to remove the goby/bin, goby/lib, goby/share, and goby/include files from your local repository before compiling this change, as otherwise CMake won't be able to create symlinks to replace these directories.

Let me know how this works for you. I'd like to merge this pull request into 2.1 and make release 2.1.2 soon with all the recent changes.

berkowski commented 8 years ago

Alright, I'll give it a try first thing tomorrow

berkowski commented 8 years ago

Working for me! Thanks.

tsaubergine commented 8 years ago

Merged #12