@tstellar Sorry for the late reply. But I have some thoughts.

There already is a Debuginfod implementation in LLVM by now. Abandoning revision.

Looks like it is already there: https://github.com/llvm/llvm-project/tree/main/llvm/include/llvm/Debuginfod

@iridinite please see these:

Added some ideas. Maybe not all so good.

LGTM except for a more detailed explanation in the comments at one place and except for a small change that slipped in because I requested too many changes ;)

@thieta I hope my comments make some sense.

Works for me.

Working for me.

In D141581#4071617, @thesamesam wrote:

This is currently holding back further testing on our end which is concerning me a bit, especially as we approach the branch point. Could you revert this please if a fix isn't imminent? Thank you!

@tstellar as you suspected, I had the following problem when building clang in standalone mode and your patch solved it for me. Thank you.

• both, build tree and install method support find_package by setting different LLVM_CMAKE_DIR

• Make require_llvm_utility_binary_path usable from a build-tree setup

Thank you @phosek for your review! I really appreciate it. I've addressed all of your comments. Could you give it another review?

• Don't quote output variables
• Use 2 space indention in CMake files
• Make out_var optional for get_llvm_utility_binary_path
• Rename get_llvm_utility_binary_path -> require_llvm_utility_binary_path

This is just a ping. About the status of this patch.

As much as I would like this to be fixed. I vote against this patch because in lld/CMakeLists.txt there's an almost (if not entirely) identical piece of code that screams to be outsourced into a /cmake/Modules/FindLit.cmake (to be created). I'll have a look at this and see if I can come up with a patch for this. Afterall /cmake is the central place to distribute shared CMake code between subprojects, right @phosek (didn't you create /cmake in the first place)?

Squashed commits

• Reorg vars

• Remove redundant vars

In D133476#3829413, @thieta wrote:

[...]
I tried to port my changes over to ghgql - and it seems to work mostly. I filed two issues in the ghgql repo where this one: https://github.com/kwk/ghgql/issues/5 is currently the biggest blocker.

• Make include regex a static member and not a function (@owenpan did this in D134852)
• Run clang-format

I didn't see much difference in what this patch does compare to mine but I saw that it removes the need for instantiating multiple llvm::Regex objects from a single static pattern. But that's something I've just done in a new revision of my own patch: https://reviews.llvm.org/D134733?vs=463205&id=463786#toc . This fast clean up makes it only harder for me to get my patch in but essentially both do the same thing except that I also have a single convenience function for trimming include names from " and <>". Also I have removed the need to duplicate the knowledge of which matching group one has to pick in order to get to the include name. This knowledge is now local to a function.

• Remove ad-hoc instantiated IncludeRegex objects

@MyDeveloperDay please see my potentially uneducated comments.

With respect to the future solution of making the version more centralized (see https://discourse.llvm.org/t/rfc-centralized-location-for-version-information/65295) I think this looks good. Great job!

In D121370#3817753, @MyDeveloperDay wrote:

@kwk have we bottomed out on this issue? its been stale for a bit. If we are not going to work on it further can be "Abandon" the review to get it off our lists?

In D133476#3783529, @thieta wrote:

[...]
The only thing I would make sure we avoid is to make this to-complicated, graphql already seems very over-engineered (to my eyes) and we just need such a small small subset that we shouldn't put to much effort into handling all kinds of queries and edge-cases.

In D133923#3791825, @tschuett wrote:

For Swift I found a PR:
https://github.com/apple/swift/pull/58931/files
The version is in a CMake file, but there were not that many files updated.

For Rust. I found a version file:
https://github.com/rust-lang/rust/blob/master/src/version

Most projects store the current version in ChangeLog files.

In D133923#3791676, @thieta wrote:

In D133923#3791675, @tschuett wrote:

There is no room for a VERSION file at the root of the repository and everybody reads the file if necessary?

I suggested this at some point - but it never got traction. Maybe showing the complexity of this script can be a catalyst of changing this (especially for the bazel build system).

I have used this script for 15.x branch just to make sure I don't forget anything, since it's many files that needs to be updated.

@thieta as much as I like the effort you've made here. I'd like to propose a standalone GraphQL library. You're adding lots of code that is hard to trust without tests. That is why I started to create a library with tests and a release process to pypi. I think much of the things you do here are so generic that we can integrate them into the library at will.

• Fixup python lit installation and run llvm tests
• remove unused defines from lld cmake and move out of directory which later is overwritten by the next git sparse checkout

• Fix installation of python lit and discoverability of the python lit module

Locally this errors out with:

I've found the error and now my worker is running fine since yesterday. I started the worker container in detached mode but quit the SSH session. This eventually caused the container to get a SIGTERM (143).

LGTM.

I don't see serious problems with this change. And my comments are just improvements. Good Work!

• Remove redundant info

LGTM now. Thank you for addressing the comments.

Can you please add links to the phabricator HTTP API documentation that you're using?

Looks good except for what I think is a typo and some suggestion for improving the readability and how things are connected with the workflow. Afterall, this is triggered by a comment, that produces another comment that we want to ignore or skip. Hm, maybe skip is better than ignore?