On 2024-04-02 at 14:18:51, Calvin Wan wrote: > Issues > ------ > - Symbol name collisions: Since C doesn't have namespacing or other > official name mangling mechanisms, all of the symbols inside of the > library that aren't static are going to be at risk of colliding with > symbols in the external project. This is especially a problem for > common symbols like "error()". Yes, I think this is important. We'll want to avoid the mistake that OpenSSL and Perl made by just exporting random symbols or using more than one namespace. We'll also need to consider that libgit2 is currently using `git_` and thus we'll either need to use something different or avoid conflicts. Perhaps `gitlib_` might be useful. I might also suggest that we enable symbol versioning on platforms that support it, which is a nice way to avoid conflicts. > - Header files: This is actually several related problems: > - Git codebase's header files assume that anything that's brought in > via <git-compat-util.h> is available; this includes system header > files, but also macro definitions, including ones that change how > various headers behave. Example: _GNU_SOURCE and > _FILE_OFFSET_BITS=64 cause headers like <unistd.h> to change > behavior; _GNU_SOURCE makes it provide different/additional > functionality, and _FILE_OFFSET_BITS=64 makes types like `off_t` be > 64-bit (on some platforms it might be 32-bit without this define). I should point out that _FILE_OFFSET_BITS=64 is effectively standard these days. Nobody wants their files limited to 2 GiB. > - Tolerant. The header files probably won't be the first/only #include > in the external project's translation unit, and they should still > work. This means not using types like `off_t` or `struct stat` in the > interfaces provided, since their sizes are dependent on the execution > environment (what's been included, #defines, CFLAGS, etc.) Sure. If we need a file size type, it should be something like `int64_t` or `uint64_t` and not `off_t`. > - Limited Platform Compatibility. The external interfaces are able to > assume that <stdint.h> and other C99 (or maybe even C11+) > functionality exists and use it immediately, without weather balloons > or #ifdefs. If some platform requires special handling, that platform > isn't supported, at least initially. I think this is fine. It's 2024. We should assume that C11 (a 13-year-old spec) is available and so is POSIX 1003.1-2008 (except for Windows). We may want to have a nice `#ifdef __STDC__ < 200112L` (and a similar check for POSIX) to just produce an `#error` if the system is too old. We should therefore also assume that threading is available because POSIX 1003.1-2008 requires it as part of the base specification (and Windows obviously also supports it). Because this is a library, we'll want to avoid interfaces that are clearly not thread safe and document the requirements for thread safety (either that the object must be externally synchronized or that it is internally synchronized). > - Non-colliding. Symbol names in these external interface headers should > have a standard, obvious prefix as a manual namespacing solution. > Something like `gitlib_`. (Prior art: cURL uses a `curl_` prefix for > externally-visible symbols, libgit2 uses `git_<module_name>_foo` > names; for internal symbols, they use `Curl_` and `git_<module>__foo` > (with a double underscore), respectively) Yeah, I think we came up with the same idea. I do like the `gitlib_module_foo` approach. Overall, I read through this and I didn't see anything I obviously disagreed with. Everything seemed to be either reasonable or something I didn't have a strong opinion on. -- brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
