[libclang/python] Add LIBCLANG_LIBRARY_PATH and LIBCLANG_LIBRARY_FILE

[TApplencourt]

Close #167421

This PR ad the environment variables LIBCLANG_LIBRARY_PATH and LIBCLANG_LIBRARY_FILE, which allow users to specify the directory path and the exact library file that should be used to locate libclang.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Thomas Applencourt (TApplencourt)

Changes

Close #167421

This PR ad the environment variables LIBCLANG_LIBRARY_PATH and LIBCLANG_LIBRARY_FILE, which allow users to specify the directory path and the exact library file that should be used to locate libclang.

---

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

2 Files Affected:

• (modified) clang/bindings/python/clang/cindex.py (+2-2)
• (modified) clang/docs/ReleaseNotes.rst (+3)

diff --git a/clang/bindings/python/clang/cindex.py b/clang/bindings/python/clang/cindex.py
index d352373e85c60..b728a8d8369ad 100644
--- a/clang/bindings/python/clang/cindex.py
+++ b/clang/bindings/python/clang/cindex.py
@@ -4383,8 +4383,8 @@ def register(item: LibFunc) -> None:
 
 
 class Config:
-    library_path = None
-    library_file: str | None = None
+    library_path: str | None = os.environ.get("LIBCLANG_LIBRARY_PATH")
+    library_file: str | None = os.environ.get("LIBCLANG_LIBRARY_FILE")
     compatibility_check = True
     loaded = False
 
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9f8d781c93021..fb812a21f2f31 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -170,6 +170,9 @@ Clang Python Bindings Potentially Breaking Changes
   ElaboratedTypes. The value becomes unused, and all the existing users should
   expect the former underlying type to be reported instead.
 - Remove ``AccessSpecifier.NONE`` kind. No libclang interfaces ever returned this kind.
+- Added the environment variables ``LIBCLANG_LIBRARY_PATH`` and ``LIBCLANG_LIBRARY_FILE``,
+  which allow users to specify the directory path and the exact library file that
+  should be used to locate libclang.
 
 What's New in Clang |release|?
 ==============================

[TApplencourt]

For more rational on the change see the issue #167421.

The PR is currently in the "minimal" stage. I can:

• In the test remove the old support for CLANG_LIBRARY_PATH, and add in the release note that people should migrate to LIBCLANG_LIBRARY_PATH (will remove 3 lines per tests file so simplifying the code base)
• Add tests. The simple one will be to do a negative testing (aka setting LIBCLANG_LIBRARY_PATH to /dev/null and verify that it crash). Other test seem harder (1/ verify that cindex if working 2/ find the libclang.so that he used, 3/ use this path to set LIBCLANG_LIBRARY_PATH, retry)

Curious what do people thinks (previous CLANG_LIBRARY_PATH didn't get any test, but it was used only the tests)

[DeinAlptraum]

add in the release note that people should migrate to LIBCLANG_LIBRARY_PATH

I don't think that's necessary since this is only used by the tests and they are not part of the interface we offer.
Where this should be adapted however is the clang/bindings/python/README.md

Add tests.

Yes please. You can create an additional test file for this, where you set the environment variable from the code, and then import cindex. Then this shouldn't be an issue. I think...
All in favor of removing the old setting CLANG_LIBRARY_PATH at the top of all the test files. Note that the current cmake build flow requires this, so you will need to change the variable name in clang/bindings/python/tests/CMakeLists.txt

Additionally, in get_cindex_library, an error message is returned that refers to the methods to set the library file. This should be adapted to also mention the environment variables.

[DeinAlptraum]

CC @Endilll

[github-actions]

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

[TApplencourt]

Ok, so:

• Added new tests: clang/bindings/python/tests/cindex/test_environment_variable.py
• Removed the old environments notions (update the README and the CINDEX error message)
• Updated the error message

Sorry I failed my git amend so the commit history is ugly, will make the review harder.

---

Erf, I run black I guess darker is not black : ( Will fix!

[DeinAlptraum]

Some minor cleanup, otherwise LGTM!

[DeinAlptraum]

Config should now be unused in most (all?) test files, so you can remove the import as well

[DeinAlptraum]

Suggested change

	def reset_import_and_get_frech_config():
	def reset_import_and_get_fresh_config():