This is an archive of the discontinued LLVM Phabricator instance.

[llvm-config] Fix bug where `--libfiles` and `--names` would produce incorrect output
ClosedPublic

Authored by delcypher on Dec 4 2016, 2:22 AM.

Details

Summary

[llvm-config] Fix bug where --libfiles and --names would produce
incorrect output when LLVM is built with LLVM_BUILD_LLVM_DYLIB.

llvm-config previously produced output like this

$ llvm-config --libfiles
/usr/lib/liblibLLVM-4.0svn.so.so
$ llvm-config --libnames
liblibLLVM-4.0svn.so.so

The library prefix and shared library extension were added to
the library name twice which was wrong.

I wanted to write a test cases for this but it looks like all
llvm-config tests were disabled by r260386 so I'll leave this for
now.

Diff Detail

Repository
rL LLVM

Event Timeline

delcypher updated this revision to Diff 80203.Dec 4 2016, 2:22 AM
delcypher retitled this revision from to [llvm-config] Fix bug where `--libfiles` and `--names` would produce incorrect output.
delcypher updated this object.
delcypher added reviewers: axw, beanz, DiamondLovesYou.
delcypher added a subscriber: tstellarAMD.

@tstellarAMD If you're still doing the point releases a fix for the problem fixed by this patch should be back ported to LLVM 3.9.x if possible because it also has this bug.

axw accepted this revision.Dec 11 2016, 11:23 PM
axw edited edge metadata.

Sorry for the delay.

This revision is now accepted and ready to land.Dec 11 2016, 11:23 PM
beanz edited edge metadata.Dec 12 2016, 10:56 AM

@delcypher this patch LGTM. Thank you for mentioning rL260386. I had no idea that patch (or rL260281) had been committed. Neither of those are remotely acceptable, and both need to be fixed. I'll take a look at that today.

This revision was automatically updated to reflect the committed changes.

@delcypher this patch LGTM. Thank you for mentioning rL260386. I had no idea that patch (or rL260281) had been committed. Neither of those are remotely acceptable, and both need to be fixed. I'll take a look at that today.

@beanz Thanks for approving and please do take a look at those tests. It was rather concerning to me that those tests were disabled in what looked like a short term commit but then were never re-enabled.

beanz added a comment.Dec 12 2016, 3:27 PM

I just committed rL289490, which reverts the commit that disabled the tests. They work fine for me (after fixing one), so let's see what happens on the bots.