Compiler requirements for git?

Compiler requirements for git?

[Corey Stup]

Newbie to the group.
I couldn't find a reference to say what the requirements are to
compile git.  Certainly it works with GCC, but GCC allows for all
sorts of non standard syntax.

When trying to compile with a C89 compliant compiler, I'm coming
across a couple issues:
- "inline" use
- trailing comma on the last member of enums

Are these oversights or does git require GCC or a C99 compiler?
Thanks!

Re: Compiler requirements for git?

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Wed, Jan 14, 2009 at 01:32:56PM -0500, Corey Stup <coreystup@gmail.com> wrote:
> When trying to compile with a C89 compliant compiler, I'm coming
> across a couple issues:
> - "inline" use

AFAIK that can be avoided with -Dinline=.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] Autoconf: Disable inline for compilers that don't support it.

[Allan Caffee]

The Autoconf macro AC_C_INLINE will redefine the inline keyword to whatever the
current compiler supports (including possibly nothing).

Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
---
On Wed, 14 Jan 2009, Miklos Vajna wrote:
> On Wed, Jan 14, 2009 at 01:32:56PM -0500, Corey Stup
> <coreystup@gmail.com> wrote:
> > When trying to compile with a C89 compliant compiler, I'm coming
> > across a couple issues:
> > - "inline" use
>
> AFAIK that can be avoided with -Dinline=.

But some compilers support other variations of this like __inline__ or
__inline.  Luckily Autoconf has a builtin method for handling this.

diff --git a/configure.ac b/configure.ac
index 082a03d..69fa25e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,6 +308,9 @@ AC_SUBST(OLD_ICONV)
 ## Checks for typedefs, structures, and compiler characteristics.
 AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])
 #
+# Check for compilers ability to inline functions.
+AC_C_INLINE
+#
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
-- 
1.5.4.3

Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.

[Junio C Hamano]

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

> The Autoconf macro AC_C_INLINE will redefine the inline keyword to whatever the
> current compiler supports (including possibly nothing).
>
> Signed-off-by: Allan Caffee <allan.caffee@gmail.com>

As far as I can tell, this makes scriptlet to set ac_cv_c_inline and then
the result is written to confdefs.h:

    case $ac_cv_c_inline in
      inline | yes) ;;
      *)
        case $ac_cv_c_inline in
          no) ac_val=;;
          *) ac_val=$ac_cv_c_inline;;
        esac
        cat >>confdefs.h <<_ACEOF
    #ifndef __cplusplus
    #define inline $ac_val
    #endif
    _ACEOF
        ;;
    esac

which is used only during the ./configure run but not during the actual
build.

What am I missing?

Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.

[Allan Caffee]

On Sat, 14 Mar 2009, Junio C Hamano wrote:
> Allan Caffee <allan.caffee@gmail.com> writes:
> 
> > The Autoconf macro AC_C_INLINE will redefine the inline keyword to whatever the
> > current compiler supports (including possibly nothing).
> >
> > Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
> 
> As far as I can tell, this makes scriptlet to set ac_cv_c_inline and then
> the result is written to confdefs.h:
> 
>     case $ac_cv_c_inline in
>       inline | yes) ;;
>       *)
>         case $ac_cv_c_inline in
>           no) ac_val=;;
>           *) ac_val=$ac_cv_c_inline;;
>         esac
>         cat >>confdefs.h <<_ACEOF
>     #ifndef __cplusplus
>     #define inline $ac_val
>     #endif
>     _ACEOF
>         ;;
>     esac
> 
> which is used only during the ./configure run but not during the actual
> build.
> 
> What am I missing?

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).  I could send a patch that
would set up a config header, but that would mean adding an #include
directive to all of the source files (or at least those using inline).
OTOH doing so would allow git to make use of some other handy macros
like AC_C_CONST.  Do you think this is worth adding a configuration
header?

Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.

[Junio C Hamano]

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.

Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.

[Allan Caffee]

On Sun, 15 Mar 2009, Junio C Hamano wrote:
> 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.

In addition to these three possibilities we could also use config.h only
in the event that the user decides to run configure.  When using a
config header autoconf appends -DHAVE_CONFIG_H to the CPPFLAGS.  So
the code could conditionally include it.

Although I'm not really sure if this still maintains the degree of user
control you're looking for.