Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

LLVM version should be 3.2, not 3.2svn #17034

Closed
jvoorhis opened this issue Jan 12, 2013 · 19 comments
Closed

LLVM version should be 3.2, not 3.2svn #17034

jvoorhis opened this issue Jan 12, 2013 · 19 comments

Comments

@jvoorhis
Copy link
Contributor

llvm-config reports that the installed version is 3.2svn and on OS X 10.8 the shared library is named libLLVM-3.2svn.dylib. This causes builds that expect a 3.2 dylib (i.e. ruby-llvm) to fail.

Steps to reproduce:

  • brew install llvm --shared
  • llvm-config --version should return 3.2, but returns 3.2svn
@jacknagel
Copy link
Contributor

We don't do anything to set the version, shouldn't this be addressed upstream?

@jvoorhis
Copy link
Contributor Author

It is a problem upstream, where they broke the convention for the version number. Other packagers like Ubuntu, however, are shipping with the version as 3.2.

@Eszet
Copy link

Eszet commented Jan 13, 2013

I am still getting a SHA1 mismatch when attempting to brew install llvm --shared (source URL looking OK to me http://llvm.org/releases/3.2/llvm-3.2-src.tar.gz).

@thoughtpolice
Copy link
Contributor

I have a patch in progress that fixes both:

  • The version number being wrong
  • The sha1 being wrong

With these, I think most of the problems will be fixed (there are still others; the formula should really bundle compiler-rt for example, and the bottles will need to be rebuilt.)

@thoughtpolice
Copy link
Contributor

And the bug for this currently is:

http://llvm.org/bugs/show_bug.cgi?id=14715

But it's unresolved.

@thoughtpolice
Copy link
Contributor

Fixed in my pull request:

$ /usr/local/Cellar/llvm/3.2/bin/llvm-config --version
3.2
$ ls /usr/local/Cellar/llvm/3.2/lib | grep dylib
BugpointPasses.dylib
LLVMHello.dylib
libLLVM-3.2.dylib
libLTO.dylib
libclang.dylib
libprofile_rt.dylib
$

@adamv
Copy link
Contributor

adamv commented Jan 30, 2013

Can we get the compiler-rt and version number as separate pull requests?

@adamv
Copy link
Contributor

adamv commented Jan 31, 2013

Passing on this since Apple also uses the "bad" version number. Don't want to carry a large patch for the sake of three letters in a version number.

@adamv adamv closed this as completed Jan 31, 2013
@thoughtpolice
Copy link
Contributor

Apple always uses 'bad' version numbers in their releases, but that's not relevant here. They always cut their releases from a working branch of LLVM and apply in-house modifications. There has never been a clang shipped by Apple, inside XCode, that has been based on a released, non-SVN variant of LLVM, to my knowledge.

The more relevant issue is that things like library detection can break. Apple does not ship things like llvm-config with XCode - or any LLVM libraries at all. They are outside of the scope and necessary bits required for the toolchain. So having an odd version number is far less of a problem.

But the version number from configure.ac does affect llvm-config --version output, as well as library path names as we can see here. This has obviously broken packages already.

Considering every other upstream distro will likely fix this for similar reasons, I don't think that Apple shenanigans are a good basis for closing this as WONTFIX.

@thoughtpolice
Copy link
Contributor

And BTW, thanks for the compiler-rt merge, but I think this should still be fixed obviously. I'm open to making another PR with your requested comment change discussing the issue, that is up to date with Homebrew's HEAD. I've just been busy the past few days.

@MikeMcQuaid
Copy link
Member

I agree here, would rather not patch it just for this. Nag upstream.

@Sharpie
Copy link
Contributor

Sharpie commented Feb 1, 2013

Agreed as well. If this was a simple matter of a misnamed tarball that we could fix by adding a version to the Formula, then that would be fine. But when we have to patch the source code to "fix" the version number... then upstream screwed up.

@thoughtpolice
Copy link
Contributor

The reality is nagging upstream will not help. They - almost entirely without exception in the history of the project to my knowledge, modulo build system instability - have never done post-release bugfixing or maintenance cycles. They have even completely left out entire functionality from the backend before (as in, it existed before and vanished out of the next release,) and not fixed it in any way for an entire release (effectively marginalizing a large amount of ARM users at the time during release cycles for Ubuntu/Fedora, etc.) Needless to say, I am not hopeful it will be fixed ever in the lifetime of 3.2. I submitted it because, as that whole reality thing would have it - things will probably break due to it.

But I digress and will not try to push the issue further. I have my patched copy and I'll use it (and I won't have to be the one dealing with any possible bug reports, in any case.)

@Sharpie
Copy link
Contributor

Sharpie commented Feb 1, 2013

The reality is nagging upstream will not help.

Well, the only call for a fix is from the reporter of the upstream issue. It's hard to say "nagging will not help" unless a bunch of people pile into that issue and demand a fix---and that effort does not produce a result.

From our end, whenever a project makes a change like this and the "reasonable response" proposed is that:

  • Debian generates and maintains a patch.
  • Fedora generates and maintains a patch.
  • MacPorts generates and maintains a patch.
  • etc.
  • etc.

Then it is pretty obvious that upstream screwed up and they should generate and maintain one patch rather than forcing all the downstream packagers to waste a bunch of effort duplicating the work.

Needless to say, I am not hopeful it will be fixed ever in the lifetime of 3.2.

Yes, but if a proper fix gets generated upstream it is quite easy and reasonable for us to backport that to the current release. But, this never happens unless the people who use LLVM bust out the torches and march on the LLVM bugtracker.

@MikeMcQuaid
Copy link
Member

Well, not just that 'upstream screwed up' but at some point you need to decide whether you want to be resposibile for fixing bugs in software other than Homebrew or just shipping stuff as-is. Personally I'm against shipping any patch that isn't going into a future upstream release (and preferably only patch once upstream has confirmed the sanity).

I know this isn't the same thing but it's worth reading about the Debian OpenSSL patching debacle to understand why downstream patching stuff they don't understand and distributing it to huge numbers of users (who just expect things to be identical to upstream) is a bad idea.

@thoughtpolice
Copy link
Contributor

@Sharpie

Well, the only call for a fix is from the reporter of the upstream issue. It's hard to say "nagging will not help" unless a bunch of people pile into that issue and demand a fix---and that effort does not produce a result.

It has less to do with 'nagging not ever helping,' and more just the fact I would rather not waste my breath and effort on an 0.1% possibility when 99.99% of the time absolutely nothing is done (even when the situation is literally the unintended removal of entire backend support for a platform!) This isn't a damnation of those developers; they are limited in their resources, as you all are (and I) and ultimately projects are defined by how they decide to allocate those resources, not just their technical design or merit. They merely do not do bug-fix releases or long-term maintenance in any sense of the phrase 'long-term' or even 'short-term.'

Then it is pretty obvious that upstream screwed up and they should generate and maintain one patch rather than forcing all the downstream packagers to waste a bunch of effort duplicating the work.

I completely understand this. I don't expect anything to be done about it in this case though, to be honest, so I'm not interested in playing blame games or caring about who's at fault. (again, Debian/Fedora/Ubuntu et al often have to deal with this anyway. When ARM support vanished in the 3.1 release cycle, all of them either applied the patch out of tree, or did not upgrade. Neither is a pretty option. Being a maintainer sucks and I've been there.)

(To be clear, I'm not blaming anyone or insinuating anything about anyone, here. Or trying to tell you how to maintain your project. You guys and package maintainers everywhere put up with tremendous shit from asshole developers like me and deserve a lot of credit you never get, for sure.)

I know this isn't the same thing but it's worth reading about the Debian OpenSSL patching debacle to understand why downstream patching stuff they don't understand and distributing it to huge numbers of users (who just expect things to be identical to upstream) is a bad idea.

I'm intimately familiar with the OpenSSL fiasco in Debian considering it's not only my job to do security research, but my boss was the one who exposed it to the world. :) And unfortunately, I don't think it's anywhere near the same ballpark as this ticket (despite what people like Linus Torvalds believe and/or would say to the contrary, not all bugs or patches are created equal unfortunately. I totally see what argument you're trying to make, but if you're going to compare fixing a version number in a tarball, with a massive unintended flaw in a world-renowned cryptographic library affecting millions of users over years - a flaw that could potentially even have put privacy, liberty or lives at risk - I'm not sure what else to say.)

Regardless as I said this ticket is clearly decided, so we best just move onto greener pastures.

@MikeMcQuaid
Copy link
Member

Thanks for the recognition that we put up with shit. Genuinely appreciate it and I don't think you're an idiot or anything. My point isn't that this patch is going to cause a security disaster but just more the principle of patching stuff without really understanding the effects of the patch (which I'll argue basically only upstream does unless it's literally a e.g. '/usr' to '/usr/local' change). Good points though, I'll take them on board. Thanks!

@thoughtpolice
Copy link
Contributor

Thanks. Hope you get your deserved recognition. :)

@Sharpie
Copy link
Contributor

Sharpie commented Feb 1, 2013

Thanks for the feedback @thoughtpolice, and you are nowhere near an "asshole developer" in my book.

I understand that this is a shitty situation. I'm just pointing out that beyond being an awesome short-term band-aid, this is a shitty fix. There are already projects, such as ruby-llvm/ruby-llvm#4 that are pondering patches to accept svn as a valid component of the LLVM version number.

It is indeed difficult to walk the line between fixing something that needs to be fixed and falling into a blame game where nothing gets done. But, in this case we are talking about a version number which is something so basic and canonical that it absolutely has to be decided by one central authority. If this decision gets left up to umpteen different downstream packagers, then we'll have an even more broken ecosystem going forward where some projects think svn is a component of the LLVM version number and some projects don't.

The only people who can really fix this is upstream because they are the only people who can unilaterally dictate what the version number is. A short term fix is great---but sometimes you have to push back if there isn't a ball rolling upstream towards a long term solution.

Otherwise the shitty situation just gets worse.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants