Page MenuHomePhabricator

[Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
ClosedPublic

Authored by bryanpkc on Nov 16 2015, 1:31 PM.

Details

Summary

Some GCC 5 installations store the libstdc++ includes and GCC-specific files in paths without
the minor part of the version number, such as

/usr/include/c++/5
/usr/lib64/gcc/x86_64-suse-linux/5

Diff Detail

Event Timeline

thiagomacieira retitled this revision from to [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations.
thiagomacieira updated this object.
thiagomacieira added a reviewer: cfe-commits.

v2: fix accidental breakage of GCC 4

testcase?

Gladly, but where are the tests for GCCVersion located?

hHave a look at the svn-blame for this file, then find other commits which changed this part of the file, and see where those commits added tests.

hHave a look at the svn-blame for this file, then find other commits which changed this part of the file, and see where those commits added tests.

Thanks, looks like 189212 has a way to do it.

v3: updated to add unit testing

tinti added a subscriber: tinti.Nov 17 2015, 6:47 AM

What happens now?

thiagomacieira accepted this revision.Nov 27 2015, 9:41 AM
thiagomacieira added a reviewer: thiagomacieira.
This revision is now accepted and ready to land.Nov 27 2015, 9:41 AM
jroelofs requested changes to this revision.Dec 1 2015, 3:26 AM
jroelofs added a reviewer: jroelofs.

I'm getting a couple of test failures from this patch:

Clang :: Driver/linux-header-search.cpp
Clang :: Driver/solaris-header-search.cpp

For the latter of the two, the driver picks:

test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/.8/include/c++/4.8.2/sparc-sun-solaris2.11

as the -internal-isystem directory, which doesn't exist. It should be finding:

test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8.2/include/c++/4.8.2/sparc-sun-solaris2.11

I think it might be because GoodVersion.MajorStr is no longer set.

This revision now requires changes to proceed.Dec 1 2015, 3:26 AM

Ah, I see a comment. Sorry about that. I'll see if I can fix it, but I had no failures when I tried...

bryanpkc commandeered this revision.May 26 2016, 5:03 PM
bryanpkc edited edge metadata.
bryanpkc updated this revision to Diff 58722.May 26 2016, 5:06 PM
bryanpkc edited edge metadata.

Fixed the code to set GoodVersion.MajorStr before returning, and removed an unnecessary file (test/Driver/Inputs/gcc_version_parsing5/lib/gcc/i386-unknown-linux/4.9.2/crtbegin.o).

Fixed the code to set GoodVersion.MajorStr before returning, and removed an unnecessary file (test/Driver/Inputs/gcc_version_parsing5/lib/gcc/i386-unknown-linux/4.9.2/crtbegin.o).

That file is not unnecessary... The test is to make sure that if there are multiple versions installed in the same spot, that the latest one is picked up.

bryanpkc updated this revision to Diff 58723.May 26 2016, 5:22 PM
bryanpkc edited edge metadata.

Re-added test/Driver/Inputs/gcc_version_parsing5/lib/gcc/i386-unknown-linux/4.9.2/crtbegin.o.

jroelofs accepted this revision.Jun 17 2016, 9:04 AM
jroelofs edited edge metadata.

LGTM. Do you want me to commit it for you?

This revision is now accepted and ready to land.Jun 17 2016, 9:04 AM
bryanpkc closed this revision.Jun 17 2016, 9:53 AM

Thanks for the review! I have committed the patch.