git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Allan Caffee <allan.caffee@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.
Date: Sun, 15 Mar 2009 12:52:54 -0700	[thread overview]
Message-ID: <7vab7m7ecp.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20090315152127.GA7988@linux.vnet> (Allan Caffee's message of "Sun, 15 Mar 2009 11:21:27 -0400")

Allan Caffee <allan.caffee@gmail.com> writes:

> My mistake; it looks like this macro will only work the way I described
> when using a config.h, which I see git is not currently doing.  I
> assumed that it would also provide a -D flag to the precompiler if a
> configuration header isn't used but this doesn't appear to be case (from
> a cursory glance at the macros definition).

The design of our Makefile is such that it will default to some reasonable
values for the make variables depending on the environment, and people who
do not want to use the configure script can override them by creating
custom entries in config.mak manually, which is included by the Makefile.

OPTIONALLY configure can be used to produce config.mak.autogen that is
included just before config.mak is included (so that misdetection by
configure script can be overridden away by config.mak), so the same kind
of overriding happens.

I suspect addition of config.h, unless done carefully, will close the door
to the people who do not use configure to get certain customizations, and
when the same carefulness is applied, we probably do no need to introduce
config.h.

For example, for -Dinline=__inline__, I think you can:

 (1) Add something like this near the beginning of the Makefile:

     # Define USE_THIS_INLINE=__inline__ if your compiler does not
     # understand "inline", but does understand __inline__.
     #
     # Define NO_INLINE=UnfortunatelyYes if your compiler does not
     # understand "inline" at all.

 (2) Add something like this after include "config.mak" happens in the
     Makefile:

     ifdef USE_THIS_INLINE
         BASIC_CFLAGS += -Dinline=$(USE_THIS_INLINE)
     else
         ifdef NO_INLINE
             BASIC_CFLAGS += -Dinline=""
	 endif
     endif

 (3) Add your new logic to configure.ac, _and_ arrange it to substitute
     USE_THIS_INLINE if ac_cv_c_inline is not "inline", and set NO_INLINE
     if it detected that the compiler does not understand inline in any
     shape or form.  You would need two new entries in config.mak.in, I
     think.

  reply	other threads:[~2009-03-15 19:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-14 18:32 Compiler requirements for git? Corey Stup
2009-01-14 22:38 ` Miklos Vajna
2009-03-14  1:04   ` [PATCH] Autoconf: Disable inline for compilers that don't support it Allan Caffee
2009-03-14 20:46     ` Junio C Hamano
2009-03-15 15:21       ` Allan Caffee
2009-03-15 19:52         ` Junio C Hamano [this message]
2009-03-16 22:31           ` Allan Caffee

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

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

  git send-email \
    --in-reply-to=7vab7m7ecp.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=allan.caffee@gmail.com \
    --cc=git@vger.kernel.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.
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 NNTP newsgroup(s).