Skip to content

Finally fix LDFLAGS position.#39

Closed
antiduh wants to merge 1 commit intobareos:masterfrom
antiduh:master
Closed

Finally fix LDFLAGS position.#39
antiduh wants to merge 1 commit intobareos:masterfrom
antiduh:master

Conversation

@antiduh
Copy link

@antiduh antiduh commented Nov 24, 2015

The Makefiles for most binaries had put $(LDFLAGS) /before/ all of the local includes, giving /usr/local/lib more priority over ../libs, etc. This causes the compiler to try to link the programs with libs from the currently installed version, not the version being built.

This is a problem inherited from bacula - see the following bug reports in FreeBSD's ports:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192526
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193641

The Makefiles for most binaries had put $(LDFLAGS) /before/ all of the local includes, giving /usr/local/lib more priority over ../libs, etc. This causes the compiler to try to link the programs with libs from the currently installed version, not the version being built.

This is a problem inherited from bacula - see the following bug reports in FreeBSD's ports:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192526
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193641
@mvwieringen
Copy link
Contributor

Honestly I'm not to sure we really want to do these kind of changes. First of all looking
at the links you moved the LDFLAGS only behind the first set of on libs not after all local libs.
I also fail to see why the include of the /usr/local/lib is done in LDFLAGS as I presume its only
needed for including things like readline etc which have their own set of lib rules in which I think
the -L/usr/local/lib -R/usr/local/lib makes more sense. I'm also not sure what kind of side effects
we get by moving this on other platforms where some of the LDFLAGS may be needed in other ways.

If you really want to fix this for this particular platform I would opt for a patch in the BSD ports system.

@antiduh
Copy link
Author

antiduh commented Dec 17, 2015

As a general rule, Makefiles should put all -L flags to refer to installed software last, and put all -L flags to refer to software in the src directory first.

This is to prevent the software build from linking against itself while being built on a system where it is already installed.

For a concrete example from FreeBSD (pardon any made-up names):
Bareos depends on readline, which is in /usr/local/lib. Bareos also installs libbareos into /usr/local/lib.

Building Bareos from source will rebuild libbarios, and then builds the things that depend on libbarios.

If you have libbarios-15.2.0.so installed in /usr/local/lib, and you put -L /usr/local/lib at the beginning of the linker line, then linking bareos-dir will use /usr/local/lib/libbarios-15.2.0.so instead of the /usr/home/mybarioscheckout/src/lib/libbarios-15.2.0.so file that you just finished compiling. Woops.

This has long been a source of build failures on FreeBSD, and is a fairly well-known Makefile mistake across just about the entirety of OSS; at least... in my own experience.

Moving the position of the flag just tells the linker to prioritize other locations first - which is what you want: ../../lib/cats should always come before /usr/local/lib.

...

There's the further complication that Bareos is typically installed as two packages, especially on FreeBSD - typically as bareos-client and bareos-server, with server depending on client. This means that bareos-client links against libbarios by looking in the src directory, but server will link against libbarios by looking in /usr/local/lib.

If I've missed a few other flags that refer to install-directory-vs-build-directory, my apologies - I'm not intimately familiar with Barios' code; I'm just a user trying to make it compile every darn time I go to rebuild the darn thing.

@mvwieringen
Copy link
Contributor

What you say is exactly the same that I said. Problem is that you should not put the -L in LDFLAGS at all !!! Looking at the problem with readline it means we should move $(CONS_LDFLAGS) and probably
name it CONS_LIBS

@antiduh
Copy link
Author

antiduh commented Dec 17, 2015

Marco, sorry to take more of your time, but I'm awfully ignorant of autoconf; your suggestion is to provide a native explicit means to specify the search path for readline, libbarios, etc, without hijacking LDFLAGS? Would you then manually order their injection into the linker line, so as to properly set directory priority?

I must apologize, but I'm not sure I understand how that would fix anything, because it would seem like it could still allow for -L/usr/local/libs to end up as the highest-priority search directory, causing Barios to fail to build correctly on systems on which it is already installed.

If you think it'll work, by all means; I only care that it compiles correctly.

@mvwieringen
Copy link
Contributor

Yes hijacking as you call it LDFLAGS is certainly the wrong approach. As we link everything dynamically (e.g. static linking is something I will remove next time I have time to do so) If you look how all Makefiles work is that they have

    $(LIBTOOL_LINK) $(CXX) $(WLDFLAGS) $(LDFLAGS) -L../lib -L../cats -L../findlib -o $@ $(SVROBJS) \
          $(NDMP_LIBS) -lbareosfind -lbareossql -lbareoscats -lbareoscfg -lbareos -lm $(DLIB) \
          $(DB_LIBS) $(LIBS) $(WRAPLIBS) $(GETTEXT_LIBS) $(CAP_LIBS) \
          $(OPENSSL_LIBS_NONSHARED) $(GNUTLS_LIBS_NONSHARED)

e.g. they first include libs from the local libs and then when you have external libs from somewhere
else you should have that in the different LIBS variables.

Readline was done in a serious strange way, I have fixed that now and for the filed we had
FDLIBS in the wrong place. I rather fix the actual problem instead of moving LDFLAGS which
is misused anyway. If you use something line --with-readline=/usr/local the right -I/usr/local/include
and -L/usr/local/lib gets added.

The include stuff is also already severely changed in respect to how Bacula use to do it as we only
use the include setting for the files that actually need it instead of including headers globally etc
which really leads to a mess.

@mvwieringen
Copy link
Contributor

Ok I fixed some strange things in the build order of the filed. I would opt for actually fixing the problem
and not some dirty hack. So I wonder what happens on FreeBSD when you remove things from LDFLAGS and use the proper --with-readline=/usr/local etc.

@joergsteffens
Copy link
Member

Have this problem been covered by commit 8cc292d and dcfcce8? Is it solved?

@antiduh
Copy link
Author

antiduh commented Feb 11, 2016

Most of the issues brought up by this pull request have been addressed in a better manner in commits 8cc292d and dcfcce8. I'm still not sure that the original problem is completely solved.

The circumstance seen in the FreeBSD ports version of this software is such that:

  • The software is built as two separate packages, bareos-client and bareos-server
  • The software is installed into /usr/local/
  • Building bareos-server, since it depends on bareos-client, needs a reference to /usr/local to satsify those dependencies using the installed bareos-client.

In this circumstance, rebuilding bareos-server while it is already installed causes build failure, because parts of bareos-server being built in your build directory attempt to link to parts of the old bareos-server in /usr/local. A little surprisingly, the same happens when attempting to build bareos-client while another copy is already installed, but that is likely because the build system is blindly passing the same build flags to both, even though it's not necessary.

I'll have to do some digging, but solving this issue probably requires a different approach than the one taken in this pull request. I agree with Marco, haphazardly moving around the LDFLAGS position as I've done in this pull request is probably an ill-conceived plan. Thus, I'm closing this pull request.

@antiduh antiduh closed this Feb 11, 2016
franku pushed a commit that referenced this pull request Feb 25, 2021
mysql 8.0 support, multiple instance support(--defaults-group-suffix)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants