Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: "brian m. carlson" <>
To: Calvin Wan <>
Cc: Git Mailing List <>
Subject: Re: [RFD] Libification proposal: separate internal and external interfaces
Date: Sun, 7 Apr 2024 21:33:43 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

  reply	other threads:[~2024-04-07 21:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-02 14:18 [RFD] Libification proposal: separate internal and external interfaces Calvin Wan
2024-04-07 21:33 ` brian m. carlson [this message]
2024-04-07 21:48   ` rsbecker
2024-04-08  1:09     ` brian m. carlson
2024-04-08 11:07       ` rsbecker
2024-04-08 21:29       ` Junio C Hamano
2024-04-09  0:35         ` brian m. carlson
2024-04-09 17:26           ` Calvin Wan
2024-04-09  9:40         ` Phillip Wood
2024-04-09 17:30           ` Calvin Wan
2024-04-22 16:26 ` Calvin Wan
2024-04-22 20:28   ` Junio C Hamano
2024-04-23  9:57   ` phillip.wood123
2024-05-09  1:00   ` Kyle Lippincott
2024-05-10  9:52     ` Phillip Wood
2024-05-10 21:35       ` Kyle Lippincott
2024-05-09 19:45   ` Kyle Lippincott
2024-05-09 20:14     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).