Build documentation without the compiled extension #11
Conversation
|
Autodoc is working as it should, see e.g. ldap-controls |
|
I'll review this PR later today. It's rather big. |
Modules/constants.c
Outdated
| obj = PyUnicode_FromString(LDAP_CONTROL_ASSERT); | ||
| PyDict_SetItemString( d, "CONTROL_ASSERT", obj ); | ||
| Py_DECREF(obj); | ||
| #define check_result() { \ |
There was a problem hiding this comment.
I don't like this kind of macros. It's confusing whenever a macro picks up values from the outer scope. Please either pass result in the function check(result);or rewrite the few calls to if (PyModule_AddIntConstant(m, "OPT_ON", 1)) return -1;
IIRC some compilers do not like a semicolon after a block {...};. This could be a problem with MSVC.
There was a problem hiding this comment.
OK, I'll write out the error checking. (but I prefer an explicit != 0 if what I'm checking isn't a straightforward Boolean.)
Wrapped the macros in do {...} while (0).
Lib/ldap/constants.py
Outdated
| ) | ||
|
|
||
|
|
||
| def write_header(): |
There was a problem hiding this comment.
It does a bit more than just writing the header :)
There was a problem hiding this comment.
I don't understand the comment. It dumps to stdout in C header format, nothing more :)
I've renamed it to print_header to point out the output is not configurable.
There was a problem hiding this comment.
Ah, you refer to C header. I though that you were referring to file header Generated with: .... Forget what I said. :)
Modules/constants.c
Outdated
|
|
||
| /* Convert an LDAP error into an informative python exception */ | ||
| PyObject* | ||
| LDAPerror( LDAP *l, char *msg ) |
There was a problem hiding this comment.
Is this a straight copy from the other file?
There was a problem hiding this comment.
Yes, except whitespace and a comment.
git diff HEAD^{/Refactor}~:Modules/errors.c HEAD^{/Refactor}:Modules/constants.c
| @@ -0,0 +1,402 @@ | |||
| """Definitions for constants exported by OpenLDAP | |||
There was a problem hiding this comment.
Should I? As a Linux packager, I have a bias against executable files in /usr/lib.
The module is intended to be run as python Lib/ldap/constants.py > Modules/constants_generated.h, and that doesn't need a shebang.
These were created only as:
forward = {}
reverse = {0: None}
The only use was the function LDAPconstant, equivalent to:
def LDAPconstant(i):
return reverse.get(i, i)
It looks like these were prepared for a better future, but python-ldap
has gone for years or decades without needing to actually use them.
Remove both dictionaries and the function.
…te C
Modules/errors.{c,h} are merged into Modules/constants.{c,h}
The function LDAPerror_TypeError is moved to common.{c,h}, as it's not
concerned with LDAPError.
Add a new Python module, ldap.constants, to keep information about
all OpenLDAP constants that we know about, including those that aren't
available in the OpenLDAP used.
Generate a C header file, Modules/constants_generated.h, from this information.
(Checking generated files into Git is better avoided, but it's much
more straightforward than generating the file from setup.py.)
Use proper error checking when adding the constants.
This involves a fake _ldap module and carefully bootstrapping ldap.__init__ (which imports * from _ldap).
The default should be pretty enough.
encukou
force-pushed
the
read-the-docs
branch
from
a96e105 to
5cbbc49
Compare
Codecov Report
@@ Coverage Diff @@
## master #11 +/- ##
==========================================
- Coverage 62.35% 60.99% -1.36%
==========================================
Files 45 46 +1
Lines 4370 4369 -1
Branches 737 749 +12
==========================================
- Hits 2725 2665 -60
- Misses 1337 1390 +53
- Partials 308 314 +6
Continue to review full report at Codecov.
|
|
Awesome, LGTM! 💯 |
This rabbit hole turned out deeper than expected, but I managed to mock out
_ldapso that documentation can be built without a C compiler.All the magic is documented.
C code had the only list of
LDAP_*constants python-ldap used. I turned that into a Python file, and generated a.hfrom it.