fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
3.95k stars 820 forks source link

building libgit2 for Fritzing limitations #3646

Open mMerlin opened 4 years ago

mMerlin commented 4 years ago

Current Behaviour

While setting up the Qt Creator environment to build/run/debug Fritzing, I was reviewing the build information for libgit2. I noticed that it was using the bundled code for the http parser, even though http-parse was one of the dependencies included for libgit2-devel. So I tried installing http-parser-devel. That got the libgit2 build to show that it was no longer using the bundled code, BUT, when later attempting to build the phoenix project, I got a bunch or error messages just before the build failed.

/usr/bin/ld: /home/phil/development/fritzing-app/../libgit2/build/libgit2.a(http.c.o): in function `clear_parser_state':
/home/phil/development/libgit2/src/transports/http.c:607: undefined reference to `http_parser_init'
/usr/bin/ld: /home/phil/development/fritzing-app/../libgit2/build/libgit2.a(http.c.o): in function `proxy_connect':
/home/phil/development/libgit2/src/transports/http.c:882: undefined reference to `http_parser_execute'
/usr/bin/ld: /home/phil/development/libgit2/src/transports/http.c:904: undefined reference to `http_errno_description'
/usr/bin/ld: /home/phil/development/fritzing-app/../libgit2/build/libgit2.a(http.c.o): in function `http_connect':
/home/phil/development/libgit2/src/transports/http.c:937: undefined reference to `http_should_keep_alive'
/usr/bin/ld: /home/phil/development/fritzing-app/../libgit2/build/libgit2.a(http.c.o): in function `http_stream_read':
/home/phil/development/libgit2/src/transports/http.c:1106: undefined reference to `http_parser_execute'
/usr/bin/ld: /home/phil/development/libgit2/src/transports/http.c:1132: undefined reference to `http_errno_description'
/usr/bin/ld: /home/phil/development/fritzing-app/../libgit2/build/libgit2.a(netops.c.o): in function `gitno_extract_url_parts':
/home/phil/development/libgit2/src/netops.c:236: undefined reference to `http_parser_parse_url'
collect2: error: ld returned 1 exit status

It appears that the libgit build references the external http-parser library, but that the Fritzing build does not notice that, does not include the library in its build. Some research on the Internet found the same case for another project -- cargo build fails with undefined references to http-parser functions -- from 5+ years ago. Looking at the commits linked to that did not tell me what the real fix was, since that seems to be just updating versions for dependencies. Have to dig further down the chain to see what was needed. I expect it just needs an extra library passed to the linker, but need to also know when that is needed.

Operating System:

Fedora 31 workstation boost_1_72_0 libgit2 v0.28.4 Qt 5.12.7

Steps to reproduce:

cd $HOME/development
tar xvfz v0.28.4.tar.gz
ln -sT $HOME/development/libgit2-0.28.4 $HOME/development/libgit2
sudo dnf install make cmake openssl-devel zlib-devel http-parser-devel
cd $HOME/development/libgit2
mkdir build
cd build
cmake -DBUILD_SHARED_LIBS=OFF ..
cmake --build .

Expected Behaviour

The Fritzing build should work whether libgit2 is built with the bundled or external http-parser functions.

Workaround

For now, just DONT have http-parser-devel installed when building libgit2

KjellMorgenstern commented 4 years ago

I think the workaround is the only way to go. It is good to be aware of this, however, and maybe it is possible to add a check and a warning in the .pri file.

Why-not-now commented 3 years ago

Is this the same as this problem? image

janclod commented 3 years ago

@mMerlin I have being trying to build on Linux and faced similar issues :tired_face: l can build the static version of libgit2, Fritzing finds the lib, but the build still fails. This happens because static libraries (in our case libgit2) need to be manually linked to external dependencies (e.g., http_parser) when building the Qt project. I will file a pull request for closing this issue. The trick is to pass the external libraries (e.g., http_parser, ssh) during the qmake phase by adding those libraries to the LIBS variables.

@KjellMorgenstern Fixing this issue should also make it easy for people to build on Linux by simply following the wiki instructions and hopefully save some struggling and frustrating moments :wink:

Another approach is to build using dynamic library on Linux, see here.

@Why-not-now this may be a similar issue, are you building on Win or Linux?

mMerlin commented 3 years ago

@janclod does yout patch handle using the installed parser, when it exists, and falling back to the bundled version when it does not? I think the extra library needs to be conditionally included. Not always.

janclod commented 3 years ago

@mMerlin well spotted! I was a bit too quick :sweat_smile: my patch allows static libgit2 to find the http_parser when libgit2 is built in the presence of http_parser. When libgit2 is built on a machine in the absence of http_parser, the build indeed fails. If it is possible, I will try to add conditional addition of http_parser and update the pull request.

janclod commented 3 years ago

@mMerlin after some more testing I have to admit that I was definitely too naive when thinking this issue would be easy to fix :sweat: I tried to build my code inside the Docker container fritzing/build:bionic... the build breaks :weary: My two cents:

janclod commented 3 years ago

In the meantime I did update the code to allow for building on different distributions (e.g., bionic, focal) and different setups (e.g. w/ or w/o http_parser installed in the system).

I tested this on fritzing/build:bionic and my own focal distro. The build works :partying_face:

Perhaps one more test should be to build inside the fritzing/build:fedora?

janclod commented 3 years ago

The pull request should allow for building fritzing both in the presence and absence of http_parser lib.

The code successfully builds on: