Libraries are no longer de-duped? #83

New Issue

ariadne · 2015-09-18T10:28:02Z

tonytheodore commented

2015-09-18 10:28:02 +00:00

(Migrated from github.com)

Hi,

I'm having the opposite problem to #78 while trying to build qt. Some part of it's build system de-dupes libs and will drop dependencies at the end. A suitable test for this is already written but the current result is:

'-lprivate -lfoo -lbaz -lzee -lbar -lfoo'

the ideal result would be either of (probably the first):

'-lprivate -lbaz -lzee -lbar -lfoo'
'-lprivate -lbar -lbaz -lzee -lfoo'

So that the final -lfoo isn't stripped. I'm not sure what pkg-config does in this case but the de-duping behaviour is preferable.

Hi, I'm having the opposite problem to #78 while trying to build qt. Some part of it's build system de-dupes libs and will drop dependencies at the end. A [suitable test for this is already written](https://github.com/pkgconf/pkgconf/blob/master/tests/run.sh.in#L119) but the current result is: ``` '-lprivate -lfoo -lbaz -lzee -lbar -lfoo' ``` the ideal result would be either of (probably the first): ``` '-lprivate -lbaz -lzee -lbar -lfoo' '-lprivate -lbar -lbaz -lzee -lfoo' ``` So that the final `-lfoo` isn't stripped. I'm not sure what `pkg-config` does in this case but the de-duping behaviour is preferable.

ariadne commented

2015-09-19 03:15:10 +00:00

I think -lprivate -lfoo -baz -lzee -lbar -lfoo is correct. Can you explain why it wouldn't be?

Keep in mind static linking, which may require the dependency to be declared twice.

I think `-lprivate -lfoo -baz -lzee -lbar -lfoo` is correct. Can you explain why it wouldn't be? Keep in mind static linking, which may require the dependency to be declared twice.

tonytheodore commented

2015-09-19 06:02:43 +00:00

(Migrated from github.com)

This is for static linking (cross-compiling actually), I'm updating pkgconf in mxe.cc from a pre 0.9.3 (https://github.com/pkgconf/pkgconf/commit/da179fd) to 0.9.12. The old version produces nice output like:

-lssl -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lz

where the new version does this:

-lssl -lws2_32 -lgdi32 -lcrypt32 -lz -lws2_32 -lgdi32 -lcrypt32 -lz -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lz

which looks like something pkg-config would produce. There's a lot of redundancy there and it's very hard to read - it's not incorrect, but also not in the spirit of pkgconf.

...which may require the dependency to be declared twice.

Isn't it only the last appearance of a lib that matters to the linker? That's been my experience but I'll browse though the issues - do you have an example that comes to mind?

This is for static linking (cross-compiling actually), I'm updating `pkgconf` in [mxe.cc](http://mxe.cc/) from a pre 0.9.3 (https://github.com/pkgconf/pkgconf/commit/da179fd) to 0.9.12. The old version produces nice output like: ``` -lssl -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lz ``` where the new version does this: ``` -lssl -lws2_32 -lgdi32 -lcrypt32 -lz -lws2_32 -lgdi32 -lcrypt32 -lz -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lz ``` which looks like something `pkg-config` would produce. There's a lot of redundancy there and it's very hard to read - it's not incorrect, but also not in the spirit of `pkgconf`. > ...which may require the dependency to be declared twice. Isn't it only the last appearance of a lib that matters to the linker? That's been my experience but I'll browse though the issues - do you have an example that comes to mind?

tonytheodore commented

2015-09-19 07:27:58 +00:00

(Migrated from github.com)

So a younger version of myself didn't understand the correct behaviour! Your comment:

What is happening here is that pkgconf is deduplicating the extra flags provided by Libs.private. Beyond this, the ordering of the libs is the same in the pkg-config output, after binutils does a merge-back phase on the linker flags anyway.

pkgconf does a merge-back on the linker flags so that libraries are ordered by number of dependencies beginning from the root, outward. This ensures that all linker dependencies are resolved in proper order and prevents overlinking.

Is the right behaviour.

Oh, I see now. --static should disable merge-back.

Now I think the merge-back should be enabled for static but I'll do some more reading. We've actually been using a merge-back enabled version of pkgconf in MXE for years without ordering issues.

So a younger version of myself didn't understand the correct behaviour! [Your comment](https://github.com/pkgconf/pkgconf/issues/51#issuecomment-23140558): > What is happening here is that pkgconf is deduplicating the extra flags provided by Libs.private. Beyond this, the ordering of the libs is the same in the pkg-config output, after binutils does a merge-back phase on the linker flags anyway. > > pkgconf does a merge-back on the linker flags so that libraries are ordered by number of dependencies beginning from the root, outward. This ensures that all linker dependencies are resolved in proper order and prevents overlinking. Is the right behaviour. > Oh, I see now. --static should disable merge-back. Now I think the merge-back should be enabled for static but I'll do some more reading. We've actually been using a merge-back enabled version of `pkgconf` in MXE for years without ordering issues.

ariadne commented

2015-09-19 16:56:52 +00:00

With shared linking, it does not matter and merge-back is safe always.

With static linking, some optimizations are unsafe. For example, if -lws2_32 depends on -lcrypt32, then -lcrypt32 must come first. Obviously we try to handle that in all cases.

It may be a bug. I'm still not convinced though. Static linking is always going to be ugly, and I think the fact that it happened to just work for you before was likely a bug...

With shared linking, it does not matter and merge-back is safe always. With static linking, some optimizations are unsafe. For example, if `-lws2_32` depends on `-lcrypt32`, then `-lcrypt32` must come first. Obviously we try to handle that in all cases. It may be a bug. I'm still not convinced though. Static linking is always going to be ugly, and I think the fact that it happened to just work for you before was likely a bug...

ariadne commented

2015-09-19 17:01:53 +00:00

Please attach all involved .pc files so we can check their correctness. The deduplication features need a full view of the dependency graph in order to safely optimize. Most likely Qt (or a dependency) is not properly declaring the dependency graph.

ariadne commented

2015-09-19 17:04:42 +00:00

Also, if you really want the unsafe optimizations we can probably add an option for them to enable them even in --static mode, but there we should be conservative by default.

Also, if you really want the unsafe optimizations we can probably add an option for them to enable them even in `--static` mode, but there we should be conservative by default.

tonytheodore commented

2015-09-20 16:09:47 +00:00

(Migrated from github.com)

With static linking, some optimizations are unsafe. For example, if -lws2_32 depends on -lcrypt32, then -lcrypt32 must come first.

I'm confused, did you mean last? Neither the old nor new behaviour puts -lcrypt32 first.

Static linking is always going to be ugly, and I think the fact that it happened to just work for you before was likely a bug...

I think a large part of the ugliness comes from people adding libs until something works. The only really ugly workaround we've had with the stricter merge-back is a single case of truly circular dependencies (harfbuzz and freetype from freedesktop.org - maybe they think such things are clever).

Yesterday, I stumbled across the grouping options in ld:

-( archives -)
--start-group archives --end-group

The archives should be a list of archive files. They may be either explicit file names, or `-l' options.

The specified archives are searched repeatedly until no new undefined references are created. Normally, an archive is searched only once in the order that it is specified on the command line. If a symbol in that archive is needed to resolve an undefined symbol referred to by an object in an archive that appears later on the command line, the linker would not be able to resolve that reference. By grouping the archives, they all be searched repeatedly until all possible references are resolved.

Using this option has a significant performance cost. It is best to use it only when there are unavoidable circular references between two or more archives.

This sounds like a very nice solution to such problems, though I haven't tried it yet and aren't sure how it will be parsed if I put it in Libs.private.

Most likely Qt (or a dependency) is not properly declaring the dependency graph.

Qt is a bit of a red-herring, it's doing it's own (naive) de-duplication with qmake's "append unique" operator LIBS *= .... I can simply replace that * with a + and the build will succeed.

Also, if you really want the unsafe optimizations we can probably add an option for them to enable them even in --static mode, but there we should be conservative by default.

That would be ideal! Some sort of strict/pedantic mode with the ability to parse (or simply pass-thru) cycle annotations would likely make the optimisations both safe and readable. (I'll brush up on my graph theory to state that more clearly)

The unsafe optimisations not only reduce maintenance and support requests, but make it much easier to debug problems. Since we started bundling pkgconf there's almost no linking questions that don't amount to "did you use the wrapper script?".

What I'm actually trying to do is improve the out-of-the-box MXE experience for cmake users with static Qt builds (or static builds in general). These issues go back to at least 2006 and there's no current solution from either upstream project. The only workable solution is to get cmake to use pkgconf, it already has built in support for pkg-config and it works well apart from some -L merge-backs that seem to have been fixed recently.

I guess what I'm really trying to do is integrate command-line pkgconf with ld, cmake, and qmake. I know libpkgconf is around the corner, but I doubt I'll be able to integrate it at a library level. As with lib interfaces, the more strict/pedantic/deterministic pkgconf can be, the better.

> With static linking, some optimizations are unsafe. For example, if -lws2_32 depends on -lcrypt32, then -lcrypt32 must come first. I'm confused, did you mean last? Neither the old nor new behaviour puts `-lcrypt32` first. > Static linking is always going to be ugly, and I think the fact that it happened to just work for you before was likely a bug... I think a large part of the ugliness comes from people adding libs until something works. The only really ugly workaround we've had with the stricter merge-back is a single case of truly circular dependencies (harfbuzz and freetype from freedesktop.org - maybe they think such things are clever). Yesterday, I stumbled across the [grouping options in `ld`](https://sourceware.org/binutils/docs/ld/Options.html#Options): > -( archives -) > --start-group archives --end-group > > The archives should be a list of archive files. They may be either explicit file names, or `-l' options. > > The specified archives are searched repeatedly until no new undefined references are created. Normally, an archive is searched only once in the order that it is specified on the command line. If a symbol in that archive is needed to resolve an undefined symbol referred to by an object in an archive that appears later on the command line, the linker would not be able to resolve that reference. By grouping the archives, they all be searched repeatedly until all possible references are resolved. > > Using this option has a significant performance cost. It is best to use it only when there are unavoidable circular references between two or more archives. This sounds like a very nice solution to such problems, though I haven't tried it yet and aren't sure how it will be parsed if I put it in `Libs.private`. > Most likely Qt (or a dependency) is not properly declaring the dependency graph. Qt is a bit of a red-herring, it's doing it's own (naive) de-duplication with qmake's "append unique" operator `LIBS *= ...`. I can simply replace that `*` with a `+` and the build will succeed. > Also, if you really want the unsafe optimizations we can probably add an option for them to enable them even in --static mode, but there we should be conservative by default. That would be ideal! Some sort of strict/pedantic mode with the ability to parse (or simply pass-thru) cycle annotations would likely make the optimisations both safe and readable. (I'll brush up on my graph theory to state that more clearly) The unsafe optimisations not only reduce maintenance and support requests, but make it much easier to debug problems. Since we started bundling `pkgconf` there's almost no linking questions that don't amount to "did you use the wrapper script?". What I'm actually trying to do is improve the out-of-the-box MXE experience for `cmake` users with static Qt builds (or static builds in general). These issues go back to at least 2006 and there's no current solution from either upstream project. The only workable solution is to get `cmake` to use `pkgconf`, it already has built in support for `pkg-config` and it works well apart from some `-L` merge-backs that seem to have been fixed recently. I guess what I'm really trying to do is integrate command-line `pkgconf` with `ld`, `cmake`, and `qmake`. I know `libpkgconf` is around the corner, but I doubt I'll be able to integrate it at a library level. As with lib interfaces, the more strict/pedantic/deterministic `pkgconf` can be, the better.

ariadne commented

2016-03-18 22:41:11 +00:00

Would a flag, say, --pure which has the old behaviour be interesting for you?

Would a flag, say, `--pure` which has the old behaviour be interesting for you?

tonytheodore commented

2016-03-18 23:31:01 +00:00

(Migrated from github.com)

Yes! That would be great.

ariadne commented

2016-11-16 04:04:30 +00:00

@tonytheodore with this you can use pkgconf --static --pure to build a static dep graph and then treat it as if it were a normal dep graph. should give you what you want.

@tonytheodore with this you can use `pkgconf --static --pure` to build a static dep graph and then treat it as if it were a normal dep graph. should give you what you want.

Sign in to join this conversation.