This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi] Teach CMake better ways to find the libc++ source directory (and misc cleanup).
ClosedPublic

Authored by EricWF on Jan 22 2015, 10:04 AM.

Details

Summary

The main section of this patch teaches CMake a new option LIBCXXABI_LIBCXX_PATH that specifies the path to the libcxx source root. This information is passed to lit so that it can better find libc++'s python module. LIBCXXABI_LIBCXX_PATH is also used to help find the libc++ headers.

The rest of this patch is misc cleanup, mostly to make pep8 and pylint happy.
I've also copied libc++'s .gitignore into libc++abi.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 18621.Jan 22 2015, 10:04 AM
EricWF retitled this revision from to [libcxxabi] Teach CMake better ways to find the libc++ source directory (and misc cleanup)..
EricWF updated this object.
EricWF edited the test plan for this revision. (Show Details)
EricWF added reviewers: danalbert, jroelofs.
EricWF added a subscriber: Unknown Object (MLST).
jroelofs added inline comments.Jan 22 2015, 10:08 AM
CMakeLists.txt
149

shouldn't this be an error?

test/lit.cfg
29

may as well s/_source_/_src_/ while you're here.

51

oops, good catch.

Address @jroelofs comments.

CMakeLists.txt
149

Probably not. Currently we only need the libc++ source directory if we run the tests. Otherwise finding the headers is enough to just build it. If we make this an error we lose the ability to build libc++abi without the libc++ sources.

EricWF updated this revision to Diff 18628.Jan 22 2015, 10:54 AM
  • Rename libcxx_source_root in lit.cfg.
  • Update standalone build documentation.
  • Change lit.cfg to use libcxx.test.config.loadSiteConfig()
  • Update how config.test_exec_root is set when no site config is found. Required for ShTest.
danalbert accepted this revision.Jan 22 2015, 11:42 AM
danalbert edited edge metadata.

LGTM with at least the doc fix and the change to sys.path.insert (either change I mentioned is fine).

test/lit.cfg
33

os.path.isfile(os.path.join(libcxx_test_src_root, 'libcxx', '__init__.py')) is slightly more correct.

34

Apparently should use 1 rather than 0: http://stackoverflow.com/a/10097543/632035

Though in this case there's really no reason you can't just append it (or use site.addsitedir).

www/index.html
93

Make a link to the instructions on the libcxx page.

This revision is now accepted and ready to land.Jan 22 2015, 11:42 AM
EricWF updated this revision to Diff 18632.Jan 22 2015, 11:58 AM
EricWF edited edge metadata.

Address @danalbert's comments.

EricWF closed this revision.Jan 22 2015, 12:01 PM

LGTM.

www/index.html
94

We should start adding id tags to our sections so we can link them directly (not this patch, obviously, just worth mentioning).