git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Yann Dirson <ydirson@altern.org>,
	git@vger.kernel.org
Subject: Re: [PATCH] compat: add memrchr()
Date: Fri, 15 Oct 2010 15:27:36 -0700	[thread overview]
Message-ID: <7vy69zay7b.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <AANLkTi=YN41n-BnVNo3HsnxNxQNBX=Ev-upmM0N49uOZ@mail.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Fri\, 15 Oct 2010 10\:49\:00 +0000")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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

  reply	other threads:[~2010-10-15 22:27 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-14 23:29 [PATCH v6 0/5] Detection of directory renames Yann Dirson
2010-10-14 23:29 ` [PATCH v6 1/5] Introduce bulk-move detection in diffcore Yann Dirson
2010-10-14 23:29   ` [PATCH v6 2/5] Add testcases for the --detect-bulk-moves diffcore flag Yann Dirson
2010-10-14 23:29     ` [PATCH v6 3/5] [RFC] Handle the simpler case of a subdir invalidating bulk move Yann Dirson
2010-10-14 23:29       ` [PATCH v6 4/5] [RFC] Consider all parents of a file as candidates for bulk rename Yann Dirson
2010-10-14 23:29         ` [PATCH v6 5/5] [WIP] Allow hiding renames of individual files involved in a directory rename Yann Dirson
2010-10-17 19:24         ` [PATCH v6.1] [RFC] Consider all parents of a file as candidates for bulk rename Yann Dirson
2010-10-15  5:17 ` [PATCH] compat: add memrchr() Jonathan Nieder
2010-10-15  5:31   ` Ævar Arnfjörð Bjarmason
2010-10-15  6:06     ` Jonathan Nieder
2010-10-15 10:49       ` Ævar Arnfjörð Bjarmason
2010-10-15 22:27         ` Junio C Hamano [this message]
2010-10-15  6:57     ` Johannes Sixt
2010-10-15  8:56   ` Ludvig Strigeus
2010-10-15 15:26     ` [PATCH v2] " Jonathan Nieder
2010-10-15  8:56   ` [PATCH] " Erik Faye-Lund
2010-10-15  9:35     ` Johannes Sixt

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=7vy69zay7b.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=ydirson@altern.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).