Welcome to the 77th edition of Git Rev News, a digest of all things Git. For our goals, the archives, the way we work, and how to contribute or to subscribe, see the Git Rev News page on git.github.io.
This edition covers what happened during the month of June 2021.
[PATCH] builtins + test helpers: use return instead of exit() in cmd_*
Ævar Arnfjörð Bjarmason sent a patch to the mailing list that
changed some cmd_*() functions so that they use a return statement
instead of exit(). He further said that it is legitimate for the
SunCC compiler on Solaris to complain about the exit() calls, as
they would skip any cleanup made after them.
The cmd_*() functions are important in the architecture of Git, as
there is one such function for each Git “builtin” command, and the
function is called by run_builtin() in git.c to perform the
command. For example when git log is launched, the cmd_log()
function is called by run_builtin().
Felipe Contreras reviewed the patch and found it obviously correct.
Peff, alias Jeff King, also said that it looked like simple and
obvious conversions, but he wondered what was SunCC complaining
about, especially if it didn’t know about NORETURN, and would
complain about many other exit() calls.
NORETURN is a special statement to tell the compiler that a
function doesn’t return, but instead uses a function like exit()
to stop the current process.
Phillip Wood also wondered if SunCC would complain about die()
calls, which use exit() underneath.
Ævar then sent
a version 2
of his patch, with no code change but explaining that SunCC actually
complains when there’s no NORETURN while we declare a cmd_*()
function to return an int. He replied to Peff with the same
explanation and added that around half of SunCC warnings are
legitimate, and that he had already been sending miscellaneous fixes
for 15-20 of them.
Junio Hamano, the Git maintainer, replied to the version 2 patch.
He especially had an issue with the part in the commit message that
said that directly exit()-ing would skip the cleanups git.c would
otherwise do, like closing file descriptors and issuing errors if it
failed. He considered that it was “not a crime” for the functions to
exit themselves as file descriptors are closed when we exit and “if
we do have clean-ups that are truly important, we would have
arranged them to happen in the atexit() handler”.
Junio anyway thought that the patch was still “a good idea because it encourages good code hygiene”.
Ævar replied to Junio that file descriptors are indeed closed when we
exit, but the errors we could get when closing them would not be
reported. He pointed to previous commits that had been merged back
in 2007 to make sure IO errors were properly reported after the
cmd_*() functions return, and said that “the atexit() handlers
cannot modify the exit code (both per the C standard, and POSIX)”.
He also discussed a bit how glibc allows atexit() handlers to
munge the exit code though it’s not portable behavior.
Junio replied that Ævar was right and that “we leave a final clean-up
for normal returns (i.e. when cmd_foo() intends to return or exit
with 0) to be done by the caller”.
The patch was later merged into the master branch and the next version of Git will better signal IO errors, thanks to SunCC and people running it to compile Git on Solaris machines.
Various
Light reading
Git tools and sites
diffsitter creates semantically meaningful
diffs that ignore formatting differences like spacing. It does so by computing a diff
on the AST (abstract syntax tree) of a file rather than computing the diff on the text
contents of the file. diffsitter uses the parsers from
the tree-sitter project
to parse source code. Written in Rust, it is very much a work in progress.
GitLive is a Visual Studio Code extension to see which branch your teammates are on in VS Code.
This edition of Git Rev News was curated by Christian Couder <christian.couder@gmail.com>, Jakub Narębski <jnareb@gmail.com>, Markus Jansen <mja@jansen-preisler.de> and Kaartic Sivaraam <kaartic.sivaraam@gmail.com> with help from Elijah Newren.