Skip to content

Conversation

@chelcassanova
Copy link
Contributor

This commit creates a Python script that fixes up the versioning information in lldb-defines.h. It also moves the build logic for fixing up the lldb headers from being in the framework only to being in the same location that we create the liblldb target.

@llvmbot
Copy link
Member

llvmbot commented May 22, 2025

@llvm/pr-subscribers-lldb

Author: Chelsea Cassanova (chelcassanova)

Changes

This commit creates a Python script that fixes up the versioning information in lldb-defines.h. It also moves the build logic for fixing up the lldb headers from being in the framework only to being in the same location that we create the liblldb target.


Full diff: https://github.com/llvm/llvm-project/pull/141116.diff

4 Files Affected:

  • (added) lldb/scripts/version-header-fix.py (+53)
  • (modified) lldb/source/API/CMakeLists.txt (+34)
  • (added) lldb/test/Shell/Scripts/Inputs/lldb-defines.h (+16)
  • (added) lldb/test/Shell/Scripts/TestVersionFixScript.test (+13)
diff --git a/lldb/scripts/version-header-fix.py b/lldb/scripts/version-header-fix.py
new file mode 100755
index 0000000000000..ddf2c9057fdc3
--- /dev/null
+++ b/lldb/scripts/version-header-fix.py
@@ -0,0 +1,53 @@
+#!/usr/bin/env python3
+"""
+Usage: <path/to/input-header.h> <path/to/output-header.h> LLDB_MAJOR_VERSION LLDB_MINOR_VERSION LLDB_PATCH_VERSION
+
+This script uncomments and populates the versioning information in lldb-defines.h
+"""
+
+import argparse
+import os
+import re
+
+LLDB_VERSION_REGEX = re.compile(r'^//#define LLDB_VERSION$')
+LLDB_REVISION_REGEX = re.compile(r'^//#define LLDB_REVISION$')
+LLDB_VERSION_STRING_REGEX = re.compile(r'^//#define LLDB_VERSION_STRING$')
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("input_path")
+    parser.add_argument("output_path")
+    parser.add_argument("lldb_version_major")
+    parser.add_argument("lldb_version_minor")
+    parser.add_argument("lldb_version_patch")
+    args = parser.parse_args()
+    input_path = str(args.input_path)
+    output_path = str(args.output_path)
+    lldb_version_major = args.lldb_version_major
+    lldb_version_minor = args.lldb_version_minor
+    lldb_version_patch = args.lldb_version_patch
+
+    with open(input_path, "r") as input_file:
+        lines = input_file.readlines()
+
+    with open(output_path, "w") as output_file:
+        for line in lines:
+            version_match = LLDB_VERSION_REGEX.match(line)
+            revision_match = LLDB_REVISION_REGEX.match(line)
+            version_string_match = LLDB_VERSION_STRING_REGEX.match(line)
+
+            # For the defines in lldb-defines.h that define the major, minor and version string
+            # uncomment each define and populate its value using the arguments passed in.
+            # e.g. //#define LLDB_VERSION -> #define LLDB_VERSION <LLDB_MAJOR_VERSION>
+            if version_match:
+                output_file.write(re.sub(LLDB_VERSION_REGEX, r'#define LLDB_VERSION ' + lldb_version_major, line))
+            elif revision_match:
+                output_file.write(re.sub(LLDB_REVISION_REGEX, r'#define LLDB_REVISION ' + lldb_version_minor, line))
+            elif version_string_match:
+                output_file.write(re.sub(LLDB_VERSION_STRING_REGEX, r'#define LLDB_VERSION_STRING "{0}.{1}.{2}"'.format(lldb_version_major, lldb_version_minor, lldb_version_patch), line))
+            else:
+                output_file.write(line)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 3bc569608e458..a1f2d4fee82c8 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -290,6 +290,40 @@ else()
   endif()
 endif()
 
+# Stage all headers in the include directory in the build dir.
+file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
+set(lldb_header_staging ${CMAKE_BINARY_DIR}/include/lldb)
+file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
+list(REMOVE_ITEM root_public_headers ${root_private_headers})
+foreach(header
+    ${public_headers}
+    ${generated_public_headers}
+    ${root_public_headers})
+  get_filename_component(basename ${header} NAME)
+  set(staged_header ${lldb_header_staging}/${basename})
+
+  if(unifdef_EXECUTABLE)
+    # unifdef returns 0 when the file is unchanged and 1 if something was changed.
+    # That means if we successfully remove SWIG code, the build system believes
+    # that the command has failed and stops. This is undesirable.
+    set(copy_command ${unifdef_EXECUTABLE} -USWIG -o ${staged_header} ${header} || (exit 0))
+  else()
+    set(copy_command ${CMAKE_COMMAND} -E copy ${header} ${staged_header})
+  endif()
+
+  add_custom_command(
+    DEPENDS ${header} OUTPUT ${staged_header}
+    COMMAND ${copy_command}
+    COMMENT "LLDB headers: stage LLDB headers in include directory")
+
+  list(APPEND lldb_staged_headers ${staged_header})
+endforeach()
+
+add_custom_command(TARGET liblldb POST_BUILD
+  COMMAND ${LLDB_SOURCE_DIR}/scripts/version-header-fix.py ${LLDB_SOURCE_DIR}/include/lldb/lldb-defines.h ${lldb_header_staging}/lldb-defines.h ${LLDB_VERSION_MAJOR} ${LLDB_VERSION_MINOR} ${LLDB_VERSION_PATCH}
+)
+
 if(LLDB_BUILD_FRAMEWORK)
   include(LLDBFramework)
 
diff --git a/lldb/test/Shell/Scripts/Inputs/lldb-defines.h b/lldb/test/Shell/Scripts/Inputs/lldb-defines.h
new file mode 100644
index 0000000000000..eed1b57d35ad3
--- /dev/null
+++ b/lldb/test/Shell/Scripts/Inputs/lldb-defines.h
@@ -0,0 +1,16 @@
+//===-- lldb-defines.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// LLDB version
+//
+// This is a truncated version of lldb-defines.h used to test the script
+// that fixes up its versioning info.
+
+// The script needs to uncomment these lines and populate the info for versioning.
+//#define LLDB_VERSION
+//#define LLDB_REVISION
+//#define LLDB_VERSION_STRING
diff --git a/lldb/test/Shell/Scripts/TestVersionFixScript.test b/lldb/test/Shell/Scripts/TestVersionFixScript.test
new file mode 100644
index 0000000000000..6acecd172f052
--- /dev/null
+++ b/lldb/test/Shell/Scripts/TestVersionFixScript.test
@@ -0,0 +1,13 @@
+// Run the convert script on it, then run the framework include fix on it. The framework version fix script
+// expects that all lldb references have been renamed to lldb-rpc in order for it to modify the includes
+// to go into the framework.
+# RUN: mkdir -p %t/Outputs
+# RUN: %python %p/../../../scripts/version-header-fix.py %p/Inputs/lldb-defines.h %t/Outputs/lldb-defines.h 21 0 0
+
+// Check the output
+# RUN: cat %t/Outputs/lldb-defines.h | FileCheck %s
+
+// The LLDB version defines must be uncommented and filled in with the values passed into the script.
+# CHECK: {{^}}#define LLDB_VERSION 21
+# CHECK: {{^}}#define LLDB_REVISION 0
+# CHECK: {{^}}#define LLDB_VERSION_STRING "21.0.0"

@chelcassanova chelcassanova force-pushed the create-python-version-fix-script branch from 8e33443 to 6b43888 Compare May 22, 2025 18:05
@github-actions
Copy link

github-actions bot commented May 22, 2025

✅ With the latest revision this PR passed the Python code formatter.

@chelcassanova chelcassanova force-pushed the create-python-version-fix-script branch from 6b43888 to 70c3d43 Compare May 22, 2025 19:56
Copy link
Member

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is functionality in lldb/scripts/framework-header-fix.sh that does the same thing. Can you remove this functionality from that script too?

@chelcassanova
Copy link
Contributor Author

There is functionality in lldb/scripts/framework-header-fix.sh that does the same thing. Can you remove this functionality from that script too?

Yeah, I can remove that. The ultimate goal I want here is:

  1. Fixing up the versioning takes place in its own script (the one in this PR).
  2. By fixing up the versioning here, it should remove the need to have a separate script for populating the info on the RPC side.
  3. Fixing up includes for the framework is converted to a Python script, and can handle both the main LLDB headers and the RPC ones in one script (using different invocations).

Comment on lines 34 to 49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that performance really matters here, but any reason to not do the substring replace on the whole buffer and make the regexes slightly less pedantic (e.g. now this script will fail if you have a space before or after the strings we're matching. Unless that's intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless that's intentional?

Not really, this is more of a holdover from how I set up the regex matching for other Python scripts that do this kind of thing. I can try passing in the buffer here instead of going line by line.

Comment on lines 1 to 7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the header in tests.

Suggested change
//===-- lldb-defines.h ------------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

Comment on lines 1 to 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • What's "it"?
  • The comment seems wrong as it's talking about lldb-rpc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed, this comment is referring to lldb-rpc and the "it" would've been from when I was copying a file in from source.

@chelcassanova chelcassanova force-pushed the create-python-version-fix-script branch 5 times, most recently from 23a443b to 81ec17a Compare May 27, 2025 22:26
This commit creates a Python script that fixes up the versioning information in
lldb-defines.h. It also moves the build logic for fixing up the lldb
headers from being in the framework only to being in the same location
that we create the liblldb target.
@chelcassanova chelcassanova force-pushed the create-python-version-fix-script branch from 81ec17a to 40793cd Compare May 28, 2025 19:45
endif()
endif()

# Stage all headers in the include directory in the build dir.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very similar to some logic in LLDBFramework.cmake. Do we need this logic in both places or could it only live here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic of finding all the headers for staging them in a directory should be able to live here alone. The idea is that the framework would then use the staged headers in <build-dir>include/lldb as its own input for when headers have to be fixed up for a framework build (see LLDBFramework.cmake in this patch: https://github.com/llvm/llvm-project/pull/142051/files where the logic for finding headers is removed and instead uses the staged headers from the build dir)

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chelcassanova chelcassanova merged commit d204aa9 into llvm:main Jun 3, 2025
9 of 10 checks passed
# unifdef returns 0 when the file is unchanged and 1 if something was changed.
# That means if we successfully remove SWIG code, the build system believes
# that the command has failed and stops. This is undesirable.
set(copy_command ${unifdef_EXECUTABLE} -USWIG -o ${staged_header} ${header} || (exit 0))
Copy link
Contributor

@HemangGadhavi HemangGadhavi Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chelcassanova
For the unifdef executable, -o option is not available in some of the system like AIX, and its giving warning while building lldb on AIX.
Is there any way to generate the output file without -o? may be using '>' operation ?

[362/3610] LLDB headers: stage LLDB headers in include directory
/usr/bin/unifdef: 1286-201 -o is not a recognized flag.
Usage: /usr/bin/unifdef [-t] [-l] [-c]
        [[-DSymbol] [-USymbol] [-idSymbol] [-iuSymbol]] ... [File]
        At least one parameter from [-D -U -id -iu] is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it works (for me, on a local build at least). I updated this line to use > for output in the patch that relands this.

@JDevlieghere
Copy link
Member

This also appears to be breaking the Xcode build:

19:57:50  CMake Error in source/API/CMakeLists.txt:
19:57:50    The custom command generating
19:57:50  
19:57:50      /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-standalone/lldb-xcode-build/include/lldb/LLDB.h
19:57:50  
19:57:50    is attached to multiple targets:
19:57:50  
19:57:50      liblldb-header-staging
19:57:50      liblldb-resource-headers
19:57:50  
19:57:50    but none of these is a common dependency of the other(s).  This is not
19:57:50    allowed by the Xcode "new build system".

https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/lldb-cmake-standalone/1780/execution/node/93/log/

@JDevlieghere
Copy link
Member

Can you please revert this while we investigate both issues?

chelcassanova added a commit that referenced this pull request Jun 4, 2025
Reverts #141116. It's breaking the Xcode build as well
as the build on AIX.
@chelcassanova
Copy link
Contributor Author

Can you please revert this while we investigate both issues?

Just reverted in #142864

@chelcassanova
Copy link
Contributor Author

For the unifdef executable, -o option is not available in some of the system like AIX, and its giving warning while building lldb on AIX.
Is there any way to generate the output file without -o? may be using '>' operation ?

I think this should be possible, although trying this locally caused the output file to be completely blank. I'll try doing this in the build system and see if it works there.

is attached to multiple targets:
19:57:50
19:57:50 liblldb-header-staging
19:57:50 liblldb-resource-headers
19:57:50
19:57:50 but none of these is a common dependency of the other(s). This is not
19:57:50 allowed by the Xcode "new build system".

So liblldb-header-staging is a target I added for this patch, it's tied to the headers being staged in the build directory. liblldb-resource-headers is from LLDBFramework.cmake. I would have the latter target be dependent on the former, and this is actually something I do in a follow-up patch for fixing up the script we use to modify headers for the LLDB framework https://github.com/llvm/llvm-project/pull/142051/files#diff-71111f357766df0b6e4415772cce9616944eac218eec4e34b3098bcd1a97f556:

# We're taking the header files from where they've been staged in the build directory's include folder,
# so create a dependency on the build step that creates that directory.
add_dependencies(liblldb-resource-headers liblldb-header-staging)

To resolve this build failure, I think the target dependency in LLDBFramework.cmake (the above few lines) can be moved into this patch.

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 4, 2025
…" (#142864)

Reverts llvm/llvm-project#141116. It's breaking the Xcode build as well
as the build on AIX.
@chelcassanova
Copy link
Contributor Author

Patch to reland this patch: #142871

chelcassanova added a commit that referenced this pull request Jun 10, 2025
#142871)

This relands the original commit for the versioning script in LLDB. This
commit uses '>' for output from `unifdef` for platforms that have that
executable but do not have the `-o` option. It also fixes the Xcode
build by adding a dependency between the liblldb-header-staging target
in the source/API/CMakeLists.txt the `liblldb-resource-headers` target
in LLDBFramework.cmake.

Original patch: #141116
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 10, 2025
…" (#142864)" (#142871)

This relands the original commit for the versioning script in LLDB. This
commit uses '>' for output from `unifdef` for platforms that have that
executable but do not have the `-o` option. It also fixes the Xcode
build by adding a dependency between the liblldb-header-staging target
in the source/API/CMakeLists.txt the `liblldb-resource-headers` target
in LLDBFramework.cmake.

Original patch: llvm/llvm-project#141116
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
)" (llvm#142871)

This relands the original commit for the versioning script in LLDB. This
commit uses '>' for output from `unifdef` for platforms that have that
executable but do not have the `-o` option. It also fixes the Xcode
build by adding a dependency between the liblldb-header-staging target
in the source/API/CMakeLists.txt the `liblldb-resource-headers` target
in LLDBFramework.cmake.

Original patch: llvm#141116
chelcassanova added a commit to chelcassanova/llvm-project that referenced this pull request Jun 17, 2025
)" (llvm#142871)

This relands the original commit for the versioning script in LLDB. This
commit uses '>' for output from `unifdef` for platforms that have that
executable but do not have the `-o` option. It also fixes the Xcode
build by adding a dependency between the liblldb-header-staging target
in the source/API/CMakeLists.txt the `liblldb-resource-headers` target
in LLDBFramework.cmake.

Original patch: llvm#141116

(cherry picked from commit eb76d83)
JDevlieghere pushed a commit that referenced this pull request Jun 20, 2025
On Windows, the post build command would open the script in the default
editor, since it doesn't know about shebangs. This effectively adds
`python3` in front of the command.

Amends #142871 /
#141116
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 20, 2025
On Windows, the post build command would open the script in the default
editor, since it doesn't know about shebangs. This effectively adds
`python3` in front of the command.

Amends llvm/llvm-project#142871 /
llvm/llvm-project#141116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants