Differences between revisions 35 and 36
Revision 35 as of 2019-06-27 21:02:48
Size: 14595
Editor: DmitryLevin
Comment: fix formatting
Revision 36 as of 2019-10-10 20:37:57
Size: 14613
Deletions are marked like this. Additions are marked like this.
Line 36: Line 36:
<<TableOfContents(2)>> == Consensus topics ==
Line 40: Line 40:
== Trivial Bug-Fix Changes == === Trivial Bug-Fix Changes ===
Line 65: Line 65:
== Machine Changes == === Machine Changes ===
Line 70: Line 70:
== WIP: Kernel syscalls wrappers == === WIP: Kernel syscalls wrappers ===
Line 93: Line 93:
== Backports to release branches == === Backports to release branches ===
Line 97: Line 97:
== Locales == === Locales ===
Line 101: Line 101:
== Other Changes == === Other Changes ===
Line 112: Line 112:
== Bad Changes == === Bad Changes ===
Line 121: Line 121:
== Best Practice for Larger Changes == === Best Practice for Larger Changes ===
Line 129: Line 129:
== Standards we use == === Standards we use ===

Community Consensus

The GNU C Library is part of the GNU Project, and key developers have agreed to be responsible for the project direction (GNU package maintainers), and for adhering to GNU policy, those people are listed here.

The GNU C Library project uses a consensus-based community-driven development model.

Consensus: General agreement, characterized by the absence of sustained opposition to substantial issues by an important part of the concerned interests and by a process that involves seeking to take into account the views of all parties concerned and to reconcile any conflicting arguments. Consensus need not imply unanimity. Although it is necessary for developer work to progress speedily, sufficient time is required for the discussion, negotiation and resolution of significant technical disagreements. Developers need to ensure discipline with respect to release schedules in order to avoid long review times. Similarly, to avoid re-discussion, developers have the responsibility of ensuring that their contribution takes into account all interests concerned, and that this standpoint is made clear at an early stage of the work rather than, for example, in a final patch or commit. (Language based loosely on the ISO definition).

1. How do I build consensus?

You build consensus in 5 very difficult steps:

  • Define or reiterate success criteria.
  • Identify a technical problem.
  • Look for resolution on a singular technical problem, summarize participant positions, and eventually move it to a "resolved" state. Keep in mind consensus need not be unanimous and record complaints and concerns for future use.
  • Summarize the "resolved" list, and summarize the remaining identified technical problems.
  • Repeat.

The first step is to define success. Defining when you stop is important because continuing a technical discussion takes time and effort, and without a success criteria it has no end. Examples of success criteria include:

  • A decision to take ownership of the malloc algorithm and implementation.
  • A decision that the TLS block should not be taken from the thread stack.
  • Lazy TLS allocation cannot be done safely from signal handlers and should happen upfront in dlopen.
  • The tunables framework should use one environment variable to express all library tunables.

Do not start a discussion if you don't have a well written success criteria.

Identifying, resolving and summarizing are the next key parts of reaching consensus. There is no easy way to describe what should be done at each step, but it should be noted that recording participant positions and summarizing are key aspects of moving the technical discussion forward. The conversation should move forward, avoiding discussion of previous points, and should be focused on the next technical problem to discuss.

Ultimately the health of the project is the responsibility of the project stewards (official GNU maintainers for the project), and these people should be counted upon to assist in all aspects of consensus building.

2. Consensus topics

The following is a nonexhaustive list of items which have community consensus.

2.1. Trivial Bug-Fix Changes

  • Anyone can commit a trivial patch to fix a spelling or grammatical error, as well as a typo, in a comment or manual. No developer review required. Post the patch and ChangeLog to libc-alpha/libc-ports with a short message and then push the commit.

  • Anyone can commit a locale related change where a bugzilla issue exists, government sources are cited, common uses are cited, and if there is an original author for the locale, that original author ACKs the change. No developer review required. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit an update to the libc.pot translation file given that the new libc.pot file came from the upstream translation project. No developer review required. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit a change regenerating a file someone previously failed to regenerate, with the same version of the relevant tool such as autoconf. Only a mention of the commit is needed on libc-alpha, not the diffs of the regeneration.

  • Anyone can commit a change fixing obvious coding standards problems in a recently committed patch. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit a change adding a bug number to ChangeLog, NEWS or both; there is no need to post such a patch.

  • Anyone can commit a change adding missing #include directives where it is clear what the right header is for functionality used in a source file. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit a change adding LOCPATH settings for a testcase using locales. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit a change fixing code typos in a commit (e.g. from last-minute untested changes) that break the build or testing, where it is clear what the right fix is. Post the patch and ChangeLog to libc-alpha with a short message and then push the commit.

  • Anyone can commit a change fixing the ordering of the Versions files. These files should be sorted for each version (alphabetically, first internal symbols used by other libraries, then public symbols).
  • Anyone can commit a change to any header to rename __block to __glibc_block or __unused to __glibc_reserved. Multiple versions of the same symbol can be created by appending a number e.g. __glibc_reserved1. Consenus was reached that glibc would use the internal prefix __glibc to avoid name collisions with other tools that comprise the implementation from a standard perspective. See https://sourceware.org/ml/libc-alpha/2012-02/msg00047.html. Note that __unused is used by source from BSD that defines it as the unused attribute for the compiler, while __block is used by Clang's -fblocks extension.

  • Anyone can commit a change to master fixing the date in a ChangeLog entry to match the git commit date. There is no need to post the patch for review, simply make the change and then push the commit. This is only allowed for master where consensus is that dates should be correct. There is still no clear consensus around what the ChangeLog date should be for a cherry-picked change into a non-master branch.

2.2. Machine Changes

  • If you are a maintainer for a machine you are the expert and in a position of leadership. You should post your patches to libc-alpha/libc-ports for general review, but you may immediately commit your patches without the need for review. The community trusts your leadership and experience. If you are unsure about a change seek help and ask specific machine maintainers to review your patches for logical errors.
  • If you are not a maintainer for the machine you're proposing the change for, your patches are subject to consensus like any other patches and while review from a machine maintainer may be ideal, it is not strictly necessary for the patch to be accepted.

2.3. WIP: Kernel syscalls wrappers

On a system with a traditional Unix-style kernel (i.e. we don’t look at Hurd when assessing this) we consider a syscall wrapper to be a function whose primary (preferably sole) purpose it is to provide a minimal C-callable interface to a kernel function that implements substantial portion of the semantics. The syscall wrapper may shuffle its arguments around, it may translate between the glibc ABI and some lower-level ABI, and it may not actually perform a system-call trap (e.g. clock_gettime), but it couldn't do what it does without invoking kernel code, and it doesn’t do any nontrivial work itself.

Traditionally it was the C library’s responsibility to provide minimally mediated C-callable access to all of the functionality of the kernel. At some point in the past the GNU C Library deviated from adding Linux-specific C-callable syscall wrappers on portability (e.g. futex) and/or backward compatibility grounds, but the project recognizes that this was a mistake. It is a mistake because it leads to a profusion of raw syscall wrappers in userspace, and these wrappers are often hard to write, hard to maintain, and glibc has all the infrastructure to handle them already. Adding these C-callable syscall wrappers to the system C library is the right course of action.

It should be noted that syscall wrappers that are OS-specific are subject to weaker compatibility guarantees than those functions which are standards conforming and completely implemented in glibc. When kernel syscalls are deprecated the best that userspace libraries can do is to continue to provide compatiblity symbols to the syscall wrappers for existing applications, but remove them for new static links (normal symbol deprecation process).

The following are the general guidelines for providing syscall wrappers:

  • If a syscall is obsoleted by another syscall (or otherwise considered obsolete), there is no need to add a wrapper to glibc. Existing functions that traditionally were syscall wrappers will be preserved, even if the syscall that does exactly what they do is obsoleted by a newer one with broader capabilities (e.g. open vs openat), if they are specified by ISO C and/or POSIX, or if they are widely used.

  • If a syscall cannot be used behind glibc's back without corrupting internal state (e.g. set_thread_area, set_tid_address, or set_robust_list), and can only be used directly by glibc, there is no need to add a wrapper to glibc.

  • If there's a glibc function that's not quite a direct wrapper of the syscall but provides all the functionality of it that can usefully be used in a program using glibc, there is no need to add a wrapper to glibc.
  • Wrappers to syscalls should be added by default (both for syscalls not used in glibc, and for cases such as futex where the syscall is used in glibc but can usefully be used directly as well) unless excluded by these guidelines.
  • If a wrapper is provided, there should also be a header with corresponding declarations (that either provides any relevant constants / structures or includes appropriate kernel headers providing them).
  • There may be cases where a function is meaningful not just with Linux kernels supporting the syscall, and should be provided with some kind of fallback for older kernels, and possibly made part of the glibc API for systems with other kernels as well.

2.4. Backports to release branches

This follow a different procedure, as described in Release policy documentation.

2.5. Locales

2.6. Other Changes

  • There are no well identified subsystem maintainers for subsystems other than architecture-specific code. Seek the consensus of a minimum of one other senior developer before checking in your changes. If you get a scary feeling around the time you are about to push the commit, stop, and go ask for more review. The reviewer may also ask for other opinions if not fully confident in the patch or in their expertise in the relevant area. Cases likely to need more review and a longer period before pushing a commit include:
    • Changes adding new public functions.
    • Changes involving general design issues.
    • Changes that have previously been controversial (including where it has been disputed whether a particular issue is actually a bug).
    • Changes that seem particularly risky for all or most users of glibc.

2.7. Bad Changes

The source tree should always build and the testsuite should not regress without a clear reason posted to libc-alpha. If the build always works, and the testsuite is in a known good state, then the source is ready for any developers to commit changes. There is never any confusion about who or what patch regressed the testsuite, and bugs can be bisected because the source is always in a buildable state (or as close as possible).

  • Commits that break the build are immediately reverted.
  • Commits that regress the testsuite are immediately reverted.

Note that the libm tests (test-float, test-double, test-ldoubl, test-ifloat, test-idouble, test-ildoubl) test also the accuracy of routines and any new test might fail until the ulp files libm-test-ulps for that architecture is updated (see math/README.libm-tests). Note that unduly large ulps should not get added, these point to problems. So, for these routines, it's expected that the architecture files gets updated (after reviewing the change) as obvious.

2.8. Best Practice for Larger Changes

For doing larger changes, especially if those involve several architectures that the main author cannot test, the following work flow should be done:

  1. Create a branch and push it to the public glibc git repository as a personal branch.
  2. Test it on at least one architecture.
  3. Ask on the libc-alpha mailing list for review and testing by other parties. Give architecture maintainers enough time for this (an explicit deadline with 5 days should be ok)
  4. Merge the branch after getting reviews, additional tests - and no test failures.

2.9. Standards we use

C11 (internally, not exposed via headers):

  • Concurrent code in glibc is free from data races (as defined by C11 and its memory model) by default. A data race is if a possible execution contains two memory accesses, at least one of them is a write, at least one is not an atomic operation, and both are not ordered by happens-before. The transition to data-race freedom will be incremental for existing code, but new code should be data-race-free. If we decide to allow a data race in a certain situation because we reason that it is benign, this must get documented and needs closer inspection, repeatedly (e.g., to check that the generated code is okay). To avoid data races, use locks or atomic operations.

None: Consensus (last edited 2019-12-11 01:31:35 by CarlosODonell)