Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add libc++ (fixes #6) #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add libc++ (fixes #6) #10

wants to merge 3 commits into from

Conversation

hesiod
Copy link
Contributor

@hesiod hesiod commented Feb 23, 2017

Add libc++-svn to the PKGBUILD, adapting some lines from the libc++ PKGBUILD,
to be found at https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libc%2B%2B.

I still have to test this a bit more, but it should generally work.

@brunocodutra
Copy link

Awesome!

Add libc++-svn to the PKGBUILD, adapting some lines from the libc++ PKGBUILD,
to be found at https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libc%2B%2B.
An incorrect path slipped through
@kerberizer
Copy link
Member

kerberizer commented Feb 26, 2017

@hesiod Thank you very much for the pull request. I know libc++ is important for some people, but unfortunately I've never been able to invest more time in acquainting myself better with it.

Here are some points that we'll probably need to take care of (starting from the least important ones):

  • Could you please set the indentation in the two new package functions to 4 spaces?
  • I think it'll be best if we leave those {LICENSE,CREDITS}.TXT files as they are. BTW, thanks for noticing the CREDITS one in the first place, as it seems to be needed at least also for compiler-rt. I'll actually change _install_license() to install both files (or whichever of them exists) and from the correct directory, so we may use it directly in the two new package functions as well.
  • I see that some unit tests are failing. More on this little further below.

My biggest concern so far are the instructions suggesting that Clang is the preferred compiler. I wonder if we shouldn't indeed make the additional effort to use it instead of GCC. The obvious problem is which Clang should we use. There are several possible approaches (unless I'm missing something of course):

  • Just use GCC. This obviously is the easiest thing, but I'm worried if we wouldn't get into some compatibility problems. Indeed, when the library is built with GCC, four of the libcxx unit tests fail. These problems actually worry me more than if the compilation was failing right away, as they can lead to some difficult to debug user problems.
  • Add Clang to makedepends. This would pull Clang from extra on a clean system (including the chroots) or use the clang-svn that may be already installed (we may also explicitly ask for clang-svn, but this means forcing the users to use the binary repo or to build in two stages). With the Clang from extra only one unit test fails, so this is kind of an improvement. I've actually tested building everything with Clang, although it might be possible to build only libc++/abi with it.
  • Use the freshly built Clang. This probably is the best approach overall, but also the hardest to achieve. Essentially, we should do the two-stage bootstrap build, which would also bring the additional advantage of having Clang bootstrapped in a more traditional way (if we really want to go hardcore, we could even do the three-stage bootstrap). I'm not sure however whether libc++ would fit naturally into this workflow or it'll require some hacking.

Being a perfectionist, I'm leaning towards the last variant, but again, it might require some non-trivial effort, and so it might be better to leave it as a todo. I'll need to do more testing with different setups, but so far my feelings are that we should avoid GCC if possible and pulling Clang might be the most balanced short-term approach. And we definitely need to have all unit tests passing, or at least know why they fail and that it's not a serious concern (like what's happening with those AMDGPU tests in #12).

Once again, really appreciate your help and will be glad to hear your opinion.

Addendum: the unit tests that fail.

When built with Clang

Failing Tests (1):
    libc++ :: std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp

When built with GCC

Failing Tests (4):
    libc++ :: std/depr/depr.c.headers/math_h.pass.cpp
    libc++ :: std/numerics/c.math/cmath.pass.cpp
    libc++ :: std/numerics/rand/rand.util/rand.util.canonical/generate_canonical.pass.cpp
    libc++ :: std/utilities/time/time.duration/time.duration.nonmember/op_divide_duration.pass.cpp

kerberizer added a commit that referenced this pull request Mar 4, 2017
So far we've simply installed the LICENSE.TXT file located in the LLVM
root as the sole license file. However, as seen in #10, there are other
license files too. In fact, there are also license files in some of the
subdirectories, which is also indicated in the LICENSE.TXT itself. The
new `_install_licenses()` tries to be a tiny bit smart about the issue
by `find`-ing all appropriate license files.

Please note that the new function does require a path as an argument.
kerberizer added a commit that referenced this pull request Mar 4, 2017
So far we've simply installed the LICENSE.TXT file located in the LLVM
root as the sole license file. However, as seen in #10, there are other
license files too. In fact, there are also license files in some of the
subdirectories, which is also indicated in the LICENSE.TXT itself. The
new `_install_licenses()` tries to be a tiny bit smart about the issue
by `find`-ing all appropriate license files.

Please note that the new function does require a path as an argument.
@kerberizer
Copy link
Member

I've updated the patch, but the web editor produced a merged commit that'll probably make the history a bit hard to follow. I'll look into this again when we decide to merge back to master: might simply cherry-pick or custom rebase the needed commits, a bit like I did with #9. Let's keep it the way it is for reference for now.

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.

None yet

3 participants