threads / discuss / 17174

Compiler requirements for git?

Subject: Compiler requirements for git?

## tl;dr

7 messages between Jan 14, 2009 and Mar 16, 2009.

replies: 6people: 4as markdown or json

Corey Stup· Jan 14, 2009, 18:32 UTC · lore

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!

Miklos Vajna· Jan 14, 2009, 22:38 UTC · re: Corey Stup · lore

Re: Compiler requirements for git?

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=.
Allan Caffee· Mar 14, 2009, 01:04 UTC · re: Miklos Vajna · lore

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

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:
Show 7 quoted lines
> 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
Junio C Hamano· Mar 14, 2009, 20:46 UTC · re: Allan Caffee · lore

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

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?
Allan Caffee· Mar 15, 2009, 15:21 UTC · re: Junio C Hamano · lore

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

On Sat, 14 Mar 2009, Junio C Hamano wrote:
Show 29 quoted lines
> 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?

Junio C Hamano· Mar 15, 2009, 19:52 UTC · re: Allan Caffee · lore

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

Allan Caffee <allan.caffee@gmail.com> writes:
Show 5 quoted lines
> 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.
Allan Caffee· Mar 16, 2009, 22:31 UTC · re: Junio C Hamano · lore

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

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

← back to recent threads