Skip to contentSkip to conversation

Compiler requirements for git?

PATCH In review Corey Stup 4 participants 7 messages Jun 15, 2016

lore
Share & tools
lore

Corey Stup

author
#0 lore raw

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!

#1 lore raw
Quoted 3 lines · Corey Stup

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

#2 lore raw

PATCH 1/1 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).

---

Quoted 7 lines · Miklos Vajna
Quoted 3 lines · Corey Stup

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.

configure.ac +3 -0
index 082a03d..69fa25e 100644
@@ -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=],
Signature · 1 line
1.5.4.3
#3 lore raw

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

Quoted 4 lines · Allan Caffee

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

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?

#4 lore raw

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

Quoted 25 lines · Junio C Hamano
Quoted 4 lines · Allan Caffee

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

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
Quoted 4 lines

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?

#5 lore raw

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

Quoted 5 lines · Allan Caffee

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.

#6 lore raw

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

Quoted 43 lines · Junio C Hamano
Quoted 5 lines · Allan Caffee

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
Quoted 5 lines

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