libclang 6.0.1 (new formula) #30229

albertosottile · 2018-07-18T13:07:04Z

Provides a way to install just libclang without downloading and installing the whole llvm bottle.
It reduces build times and bottle sizes (270 MB vs 410 MB on Sierra).
Being keg-only it does not interfere with system or with full llvm installations.
It avoids potential library conflicts when only libclang.dylib and clang headers are required (see #29699).
Useful for building (and maybe using) qt and pyside without downloading llvm (see #29415).

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Provides a way to install libclang without installing the whole llvm bottle. It reduces build times and bottle sizes (~280 MB vs 1.3 GB). It avoids potential library conflicts when only libclang.dylib is required (see Homebrew#29699). Useful for building qt without downloading llvm (see Homebrew#29415).

albertosottile · 2018-07-18T13:09:16Z

The test code is taken from this gist. If possible, I would like @quarnster to authorize the use of his code in this repository and its potential release under the BSD-2-Clause license.

Otherwise, we can stick with more traditional llvm-config based tests.

albertosottile · 2018-07-18T13:11:43Z

P.S. I stripped the head section to pass brew audit --new-formula, but of course it is totally possible to put it back.

albertosottile · 2018-07-20T14:49:35Z

Given that we did not get an answer from the gist author, I changed the test block with a new code heavily adapted by myself and inspired by a code released under the MIT license. This eliminates any potential license issues.

shawwn · 2018-07-28T01:27:25Z

This is pretty amazing, thank you. I have limited HD space. I hope this gets merged! :)

albertosottile · 2018-08-01T13:10:57Z

Some formulas that can be built and used with libclang instead of llvm (tested on 10.13.6):

castxml
creduce
nvc
pyside
rtags
woboc_codebrowser

DomT4 · 2018-08-08T20:09:31Z

@BrewTestBot test this please

sjackman · 2018-08-08T20:27:51Z

Is it possible for llvm to depend on libclang, so that both can be installed and linked?

DomT4 · 2018-08-08T20:28:01Z

I'm leaning towards a 👍 here but I'll give it another day or two to let other maintainers discuss/comment as they wish to. The idea that 5/7 of the formulae that currently mandate a runtime llvm dependency could be switched over to use this much slimmer dependency is compelling.

DomT4 · 2018-08-08T20:30:08Z

Is it possible for llvm to depend on libclang, so that both can be installed and linked?

IIRC llvm contains libclang already, alongside a whole bunch of other stuff. I think OP is essentially saying that most of what other formulae need is in this formula, which could be used as a drop-in replacement where the full llvm package isn't necessary, given the full llvm package is a gigabyte heavier.

fxcoudert · 2018-08-08T20:27:48Z

Formula/libclang.rb

+  fails_with :gcc
+  ("4.3".."4.6").each do |n|
+    fails_with :gcc => n
+  end


All those versions of GCC are very old and not carried by recent macOS and recent Homebrew anymore. Remove the whole block.

fxcoudert · 2018-08-08T20:29:18Z

Formula/libclang.rb

+      -DLLVM_INCLUDE_DOCS=OFF
+      -DLLVM_TARGETS_TO_BUILD=all
+    ]
+    args << "-DLIBOMP_ARCH=x86_64"


Inline all arguments below, no temporary args variable. (I am surprised -DLIBOMP_ARCH=x86_64 is not auto-detected.)

fxcoudert · 2018-08-08T20:31:14Z

Formula/libclang.rb

+        clang_disposeIndex(index);
+        return 0;
+      }
+    EOS


Can we maybe trim down that example C source a bit?

fxcoudert · 2018-08-08T20:32:08Z

Formula/libclang.rb

+                           "libclangtest.cpp", "-o", "libclangtest"
+    testresult = shell_output("./libclangtest sample.cpp")
+
+    sorted_testresult = testresult.split("\n").sort.join("\n")


Why split, sort, and join if the result is a single line?

fxcoudert · 2018-08-08T20:32:58Z

My only concern is that the name is misleading… we’re not only installing libclang, but also an actual clang binary, and more

DomT4 · 2018-08-08T20:41:24Z

My only concern is that the name is misleading… we’re not only installing libclang, but also an actual clang binary, and more

Aye, fair comment 👍. Open to suggestions if anyone has anything. clang-only or llvm-lite or llvm-clang or something hopefully less slightly silly sounding than any of those.

RandomDSdevel · 2018-08-08T21:24:28Z

@DomT4:

The name 'llvm-clang' seems a little too close to the Homebrew's internal 'llvm_clang' compiler specification from line 19 of Library/Homebrew/compilers.rb, as specifiable using the HOMEBREW_CC environment variable.

MikeMcQuaid · 2018-08-09T13:31:22Z

Personally I'm 👎 on this. It's annoying to have the extra disk space used but I don't think we should be optimising package management around disk space usage. Additionally, if you need to have something that depends on llvm and something that depends on libclang it will use more disk space after this rather than less.

RandomDSdevel · 2018-08-09T18:37:02Z

@MikeMcQuaid:

This new libclang formula (or whatever it ends up being named) might, in all probability, be needed as a dependency of a PR I'm planning to submit that will restore the qt formula's --with-docs option (or resurrect something similar, such as a --with-llvm/--with-libclang/whatever option, in its place,) as @ilovezfs removed said pre-existing option in #29469 in response to my earlier report of it being broken in #29415. Whether this PR gets merged or not, neither either of us nor @apjanke (who has also participated in discussion concerning that issue and PR linked just previously) have yet managed to make Qt's documentation build properly with Homebrew's full LLVM keg. There may still be a way to fix this (perhaps those two upstream patches I'm going to need anyway in addition to this PR might be enough to make qdoc's build work well enough on their own, along with manual specification of the location of Homebrew's LLVM if that actually turns out to be necessary,) but I haven't looked into doing that just yet.

albertosottile · 2018-08-10T14:02:01Z

@fxcoudert
I just pushed a commit according to your review, I hope I addressed all your concerns. I deleted 12 lines from the test code, I do not think it can be shrunk further without excessively compromising its legibility. (EDIT: I realized later that a function was not used anymore by the test and I deleted it).

@MikeMcQuaid

Additionally, if you need to have something that depends on llvm and something that depends on libclang it will use more disk space after this rather than less.

To be honest, I do not know if any of the existing formulas actually need llvm. I am on vacation at the moment and I only have my MacBook Air with me, so I tested only the formulas that I posted before. It might be that other formulas can be built and used with libclang instead of llvm, and there is a strong chance that none of the current formulas actually need full llvm. In any case, reducing the number of formulas that use llvm reduces also the number of users that need to install it, also reducing the probability that someone needs to have both libclang and llvm in their disk.

It's annoying to have the extra disk space used but I don't think we should be optimising package management around disk space usage.

It's not just about disk space but also bandwidth and data usage, especially if llvm becomes a dependency of qt.

MikeMcQuaid · 2018-08-11T14:06:02Z

have yet managed to make Qt's documentation build properly with Homebrew's full LLVM keg

To me that points to removing the option rather than adding a new, duplicated formula.

also reducing the probability that someone needs to have both libclang and llvm in their disk.

Currently that probability is 0% because there is no libclang formula 😉.

It's not just about disk space but also bandwidth and data usage, especially if llvm becomes a dependency of qt.

The same argument applies. If you have a formula that does need to depend on LLVM and another that depends on this formula: you've just increased the disk, bandwidth and data usage for that user.

javian · 2018-08-11T15:19:17Z

Sorry this might be a stupid question but why is llvm the primary resource here and not a build dependency ?

albertosottile · 2018-08-11T15:44:30Z

The same argument applies. If you have a formula that does need to depend on LLVM and another that depends on this formula: you've just increased the disk, bandwidth and data usage for that user.

That's true, but the vast majority of users will save disk space, bandwidth, and data usage. Also, does anyone know if there is any Formula that currently needs the full llvm keg?

RandomDSdevel · 2018-08-11T16:21:13Z

…have yet managed to make Qt's documentation build properly with Homebrew's full LLVM keg…

To me that points to removing the option rather than adding a new, duplicated formula.

⋮

I already mentioned #29469, where @ilovezfs did just that for now. Regardless, this involves/affects not just Qt's documentation, but qdoc as well, if you'll recall. To quote earlier discussion between @ilovezfs and @apjanke in #29415 (comment) #29415 (comment):

I think we need to distinguish two goals:

building the docs

installing qdoc itself

⋮

I agree with this distinction. But I lean more towards doing what it takes to build qdoc, because (if I'm reading this right), qdoc was previously available by default in our qt formula up through 5.10, so its absence now is a regression. And we have no information on how many users and child formulae are actually using it.

Maybe llvm could be a default/mandatory :build time only dependency? That would keep Homebrew producing a maximal, back-compatible qt bottle, but not require users to install the... oh, wow, 1.3 GB llvm formula.

I get the desire to have a lot of users clamoring for qdoc, but of the users who might be affected, how many know to come here and ask for it, and how many would just give up?

⋮

Besides, @albertosottile has provided other examples for which a minimal libclang formula could come in handy. I'll report back if I can finally manage to get qdoc and Qt's documentation to build using the full LLVM formula, though.

Off-Topic Note Addition:

Y'know, @sjackman and @DomT4 discussing the following…:

Is it possible for llvm to depend on libclang, so that both can be installed and linked?

IIRC llvm contains libclang already, alongside a whole bunch of other stuff. …

…reminds me of an old item on my to-do list that I still haven't gotten around to: experimenting with modularizing all of the LLVM suite's components into their own individual formulas in a personal tap. That can wait until after I've closed all my currently open Homebrew issues, though…

albertosottile · 2018-08-27T14:33:04Z

Update on other formulas compatibility with libclang:
zig can be built with libclang and the included test passes. Now, of course, zig is a full language and it is not guaranteed that a single test covers all its requirements, but consider that zig itself provides pre-built "llvm+clang" library for Windows for their users here, in a 275 MB tar (wink wink).

emacs-clang-complete-async is the last formula that depends on llvm, but unfortunately, it does not have a test block and I am not an emacs user so I cannot test it privately. Nevertheless, build runs just fine and on their website only libclang is stated as a requirement.

There are two other formulas with an optional llvm dependency: doxygen and rust, if you want I can also test those. In addition, there are other formulas that depend on older llvm versions, I am not sure what to do with those honestly. Besides optional dependencies and old versions, it seems that the proposed libclang formula supports all the existing formulas that currently depend on llvm.

About the name, I understand that libclang could appear confusing because a clang binary is included, even though I saw this name used by other parties for the same package (Qt in the first place). I like the proposed alternatives and I suggest also clang-lib for consideration.

albertosottile · 2018-08-28T10:37:10Z

I apologize, I looked more carefully at sizes now that I am back at work and, apparently, I was comparing apples to oranges. While there is indeed a size reduction, is not as dramatic as I thought.
Specifically, on Sierra (in mega binary bytes):

Formula	Bottled	Uncompressed
`libclang`	270 MB	920 MB
`llvm`	410 MB	1375 MB

Looking only at those numbers, I have to agree with @MikeMcQuaid and concede that the size benefits alone do not justify the added hassle of maintaining two similar formulas. However, as mentioned before, libclang could still be useful when linking conflicts between system and llvm libraries arise (e.g. in qt and pyside formulas).

In addition, I personally find quite puzzling that apparently none of the existing formulas need all the resources currently embedded in the llvm bottle. Maybe an alternative easier solution could be to make this extra stuff optional in the existing llvm formula (specifically, clang-tools-extra, compiler-rt, libcxx, libunwind, lld, openmp, polly). This would achieve both the results sought by this PR without adding a partially overlapping formula.

MikeMcQuaid · 2018-08-28T12:42:28Z

Thanks for doing the research and being so open about this @albertosottile. I'm afraid as a result I definitely think it's worth closing this as-is.

libclang could still be useful when linking conflicts between system and llvm libraries arise (e.g. in qt and pyside formulas).

Generally if this only applies to a few formulae we consider vendoring (e.g. using a resource block) in the relevant formula as a last resort.

In addition, I personally find quite puzzling that apparently none of the existing formulas need all the resources currently embedded in the llvm bottle. Maybe an alternative easier solution could be to make this extra stuff optional in the existing llvm formula (specifically, clang-tools-extra, compiler-rt, libcxx, libunwind, lld, openmp, polly). This would achieve both the results sought by this PR without adding a partially overlapping formula.

I'm afraid we're moving in exactly the opposite direction here:
#31510. The desire is to have formulae with no options that provide all the necessary functionality over having multiple, overlapping formulae or formulae with options.

MikeMcQuaid · 2018-08-28T12:53:05Z

I think the best bet here is for you to create your own tap in which you maintain a stripped down/option heavy LLVM. Sorry!

albertosottile · 2018-08-28T13:03:35Z

To be honest, I do not need a stripped down LLVM, personally. With this PR, I was trying to solve the issue experienced by qt --with-docs and the linking conflict we found in pyside. The reduction in size was an added benefit that could have interested the community, or so I thought. Given your comments and #31510, I feel that you do not perceive that as a benefit at all. No hard feelings, though and, from my side, I apologize for not having checked sooner the uncompressed install size of this formula, but only its bottle size.

MikeMcQuaid · 2018-08-28T13:12:03Z

There's absolutely no need to apologise @albertosottile; that you've done the research you have and it's all publicly documented is a genuinely useful contribution for future reference ❤️

DomT4 · 2018-08-28T16:10:23Z

Thanks for the hard work here @albertosottile. I appreciate the time you put into this, despite the end result.

albertosottile mentioned this pull request Jul 18, 2018

qt --with-docs build fails #29415

Closed

6 tasks

ilovezfs added the new formula PR adds a new formula to Homebrew/homebrew-core label Jul 19, 2018

libclang: change test code

d2deaf0

libclang: change test code

fd7e4c9

This was referenced Jul 29, 2018

pyside 5.11.0 (new formula) #29699

Closed

oclgrind 18.3 (new formula) #30539

Closed

fxcoudert requested changes Aug 8, 2018

View reviewed changes

libclang: review

bc68386

DomT4 added in progress Stale bot should stay away maintainer feedback Additional maintainers' opinions may be needed labels Aug 10, 2018

libclang: reduce test code length

ac1d685

MikeMcQuaid closed this Aug 28, 2018

DomT4 removed in progress Stale bot should stay away maintainer feedback Additional maintainers' opinions may be needed labels Aug 28, 2018

DomT4 mentioned this pull request Sep 9, 2018

docs: add formal governance system Homebrew/brew#4864

Closed

3 tasks

lock bot added the outdated PR was locked due to age label Sep 27, 2018

lock bot locked as resolved and limited conversation to collaborators Sep 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libclang 6.0.1 (new formula) #30229

libclang 6.0.1 (new formula) #30229

albertosottile commented Jul 18, 2018 •

edited

Loading

albertosottile commented Jul 18, 2018 •

edited

Loading

albertosottile commented Jul 18, 2018

albertosottile commented Jul 20, 2018 •

edited

Loading

shawwn commented Jul 28, 2018

albertosottile commented Aug 1, 2018

DomT4 commented Aug 8, 2018

sjackman commented Aug 8, 2018

DomT4 commented Aug 8, 2018

DomT4 commented Aug 8, 2018

fxcoudert Aug 8, 2018

fxcoudert Aug 8, 2018

fxcoudert Aug 8, 2018

albertosottile Aug 11, 2018

fxcoudert Aug 8, 2018

fxcoudert commented Aug 8, 2018

DomT4 commented Aug 8, 2018 •

edited

Loading

RandomDSdevel commented Aug 8, 2018

MikeMcQuaid commented Aug 9, 2018

RandomDSdevel commented Aug 9, 2018 •

edited

Loading

albertosottile commented Aug 10, 2018 •

edited

Loading

MikeMcQuaid commented Aug 11, 2018

javian commented Aug 11, 2018

albertosottile commented Aug 11, 2018

RandomDSdevel commented Aug 11, 2018 •

edited

Loading

albertosottile commented Aug 27, 2018

albertosottile commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

albertosottile commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

DomT4 commented Aug 28, 2018

libclang 6.0.1 (new formula) #30229

libclang 6.0.1 (new formula) #30229

Conversation

albertosottile commented Jul 18, 2018 • edited Loading

albertosottile commented Jul 18, 2018 • edited Loading

albertosottile commented Jul 18, 2018

albertosottile commented Jul 20, 2018 • edited Loading

shawwn commented Jul 28, 2018

albertosottile commented Aug 1, 2018

DomT4 commented Aug 8, 2018

sjackman commented Aug 8, 2018

DomT4 commented Aug 8, 2018

DomT4 commented Aug 8, 2018

fxcoudert Aug 8, 2018

Choose a reason for hiding this comment

fxcoudert Aug 8, 2018

Choose a reason for hiding this comment

fxcoudert Aug 8, 2018

Choose a reason for hiding this comment

albertosottile Aug 11, 2018

Choose a reason for hiding this comment

fxcoudert Aug 8, 2018

Choose a reason for hiding this comment

fxcoudert commented Aug 8, 2018

DomT4 commented Aug 8, 2018 • edited Loading

RandomDSdevel commented Aug 8, 2018

MikeMcQuaid commented Aug 9, 2018

RandomDSdevel commented Aug 9, 2018 • edited Loading

albertosottile commented Aug 10, 2018 • edited Loading

MikeMcQuaid commented Aug 11, 2018

javian commented Aug 11, 2018

albertosottile commented Aug 11, 2018

RandomDSdevel commented Aug 11, 2018 • edited Loading

albertosottile commented Aug 27, 2018

albertosottile commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

albertosottile commented Aug 28, 2018

MikeMcQuaid commented Aug 28, 2018

DomT4 commented Aug 28, 2018

albertosottile commented Jul 18, 2018 •

edited

Loading

albertosottile commented Jul 18, 2018 •

edited

Loading

albertosottile commented Jul 20, 2018 •

edited

Loading

DomT4 commented Aug 8, 2018 •

edited

Loading

RandomDSdevel commented Aug 9, 2018 •

edited

Loading

albertosottile commented Aug 10, 2018 •

edited

Loading

RandomDSdevel commented Aug 11, 2018 •

edited

Loading