Re: [PATCH] compat: add memrchr()

Ævar Arnfjörð Bjarmason [off-list ref] writes:

Maybe it should be a NO_* flag since that's what we use when we expect
sanity, e.g. we have NO_REGEXP=, NO_CURL and NO_GETTEXT.

Then you just need NO_MEMRCHR=UnfortunatelyYes entries in the big
if/else block for those platforms that don't have it.

Hmmm, perhaps our naming convention have deteriorated over time.

I think one of the oldest one is NO_CURL, which started out as a way to
say "I do not have to go over http and dumb http transport is of no use to
me.  I _might_ have curl library installed on this system, but it does not
matter.  No thanks, please do not build git with curl support".  For some
people, it also meant "Unfortunately I do not have a working curl library
here, so I'll live with a git without http support".

On the other hand, NO_REGEX means "regrettably the platform implementation
is not good enough for our codebase, and we do need to use a replacement
implementation from compat/".

The difference is that it is not an option to build a less capable git
that lacks features that depend on REGEX.  But somehow we ended up using
the same NO_ prefix, conflating the two.  In that sense NO_GETTEXT is
named correctly, I think, as we fall back on implementation (i.e. less
capable git) that does not do i18n/l10n.

But memrchr() is a GNU extension so it should probably be a HAVE_*. I
don't know.

Yes, if we want to express that we positively rely on the existence of an
extension, HAVE_* does sound more correct.  It also is how the world with
autoconf/configure works.

We might want to clean up the names of variables at some point, but from a
quick glance things do not look very good.

Most NO_FOO are misnamed and should be !HAVE_FOO, but because our codebase
tend to use more common features, switching from the way we currently do,
i.e. assuming we have FOO and defining NO_FOO for oddball platforms, to
the other way around of defining HAVE_FOO for platforms where we do have FOO
and omit it for the ones that lack, may make the Makefile more cluttered.

But as I already said, switching to HAVE_FOO has one definite advantage;
it is more conventional and would mesh a lot better with the way
autoconf/configure would do things.

Here is a quick break-down of the current set I found from the 'master'
branch.

* You decline to build, or are unable to build, git with...

  NO_CURL		transport based on libcurl e.g. HTTP.
  NO_ICONV		charset re-encoding support.
  NO_IPV6		IPV6
  NO_NSEC		nanosecond resolution file timestamps
  NO_POSIX_ONLY_PROGRAMS
  NO_PYTHON		anything that depends on Python
  NO_SYMLINK_HEAD	support for original layout where .git/HEAD was a symlink
  NO_SVN_TESTS		test that spends too much time on git-svn


* We work around the lack of the platform feature FOO without loss of
  features from git (i.e. should be !HAVE_FOO):

  NO_C99_FORMAT		SZ_FMT and friends
  NO_D_INO_IN_DIRENT	(struct dirent)d.d_ino
  NO_D_TYPE_IN_DIRENT	(struct dirent)d.d_type
  NO_HSTRERROR
  NO_INET_NTOP
  NO_INET_PTON
  NO_LIBGEN_H
  NO_MEMMEM
  NO_MKDTEMP
  NO_MKSTEMPS
  NO_MMAP
  HAVE_PATHS_H		#include <paths.h>
  NO_PERL_MAKEMAKER
  NO_PREAD
  NO_REGEX
  NO_R_TO_GCC_LINKER
  NO_SETENV
  NO_SOCKADDR_STORAGE
  NO_ST_BLOCKS_IN_STRUCT_STAT
  NO_STRCASESTR
  NO_STRLCPY
  NO_STRTOK_R
  NO_STRTOULL
  NO_STRTOUMAX
  NO_SYS_SELECT_H
  NO_TRUSTABLE_FILEMODE
  NO_UINTMAX_T
  NO_UNSETENV

Unfortunately the world is not so black-and-white.  There is another class

* Depending on platform features, we use different implementation with
  almost no observable difference in behaviour (perhaps except for
  performance)

  NO_PTHREADS
	Use of threads for better machine utilization

  NO_FAST_WORKING_DIRECTORY
	Reading from object database is faster than opening a file in the
	working directory and reading from it on this platform, so we do
	the former when we know the file in the working directory is clean.
        Perhaps !HAVE_FAST_WORKING_DIRECTORY.

And yet another that is completely unrelated:

  NO_SUBDIR