Git Mailing List Archive mirror
 help / color / mirror / Atom feed

* [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW
@ 2023-05-16 16:58 rsbecker
  2023-05-16 17:06 ` Taylor Blau
  0 siblings, 1 reply; 15+ messages in thread
From: rsbecker @ 2023-05-16 16:58 UTC (permalink / raw)
  To: git

Hi Git Team,

I am getting a compile error on rc0:

  	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,
git_atexit_hdlrs.alloc);
  	^
"/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c
", line 1103: error(114): identifier "ALLOC_GROW" is undefined

Any help o nthis?

Thanks,
Randall

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.




^ permalink raw reply	[flat|nested] 15+ messages in thread

• * Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW
    2023-05-16 16:58 [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW rsbecker
  @ 2023-05-16 17:06 ` Taylor Blau
    2023-05-16 17:19   ` Junio C Hamano
    2023-05-16 17:56   ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau
    0 siblings, 2 replies; 15+ messages in thread
  From: Taylor Blau @ 2023-05-16 17:06 UTC (permalink / raw)
    To: rsbecker; +Cc: git, Elijah Newren
  
  On Tue, May 16, 2023 at 12:58:15PM -0400, rsbecker@nexbridge.com wrote:
  > Hi Git Team,
  >
  > I am getting a compile error on rc0:
  >
  >   	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,
  > git_atexit_hdlrs.alloc);
  >   	^
  > "/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c
  > ", line 1103: error(114): identifier "ALLOC_GROW" is undefined
  >
  > Any help o nthis?
  
  None of the code in run-command.c has changed much recently, but this
  bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
  cache.h, 2023-02-24).
  
  Note that it only affects when building with NO_PTHREADS=1 (which I
  suspect is the case on NonStop). Here's the bisection:
  
      $ git bisect start
      $ git bisect bad
      $ git bisect good v2.40.0
      $ git bisect run sh -c 'make DEVELOPER=1 NO_PTHREADS=1 run-command.o'
  
  I'll attach a patch in a separate message.
  
  Thanks,
  Taylor
  
  ^ permalink raw reply	[flat|nested] 15+ messages in thread

• • * Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW
      2023-05-16 17:06 ` Taylor Blau
    @ 2023-05-16 17:19   ` Junio C Hamano
      2023-05-16 17:24     ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano
      2023-05-16 17:56   ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau
      1 sibling, 1 reply; 15+ messages in thread
    From: Junio C Hamano @ 2023-05-16 17:19 UTC (permalink / raw)
      To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren
    
    Taylor Blau <me@ttaylorr.com> writes:
    
    Ah, alloc.h was introduced and everybody who took ALLOC_GROW() from
    git-compat-util.h (or was it cache.h?) now includes it, but
    apparently Elijah forgot  run-command.c and nobody caught this.
    
    I wonder if there are other leftover ones that we haven't caught?
    
        $ git grep -l '[^_]ALLOC_GROW(' \*.c | sort >/var/tmp/1
        $ git grep -l 'alloc\.h' \*.c | sort >/var/tmp/2
        $ comm -23 /var/tmp/[12]
        compat/simple-ipc/ipc-unix-socket.c
        run-command.c
    
    But the former one is only in a comment, so it probably is OK.
    
    
    ^ permalink raw reply	[flat|nested] 15+ messages in thread

  • • * [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
        2023-05-16 17:19   ` Junio C Hamano
      @ 2023-05-16 17:24     ` Junio C Hamano
        2023-05-16 17:33       ` rsbecker
        2023-05-16 17:57       ` Taylor Blau
        0 siblings, 2 replies; 15+ messages in thread
      From: Junio C Hamano @ 2023-05-16 17:24 UTC (permalink / raw)
        To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren
      
      Recent header file shuffling missed this old user of ALLOC_GROW()
      that was inside "#ifdef NO_PTHREADS' section and forgot to include
      the new file, alloc.h, that defines the macro.
      
      Signed-off-by: Junio C Hamano <gitster@pobox.com>
      
      ---
       run-command.c | 1 +
       1 file changed, 1 insertion(+)
      
      diff --git c/run-command.c w/run-command.c
      index d4247d5fcc..1affea48af 100644
      --- c/run-command.c
      +++ w/run-command.c
      @@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code)
       }
       
       #else
      +#include <alloc.h>
       
       static struct {
       	void (**handlers)(void);
      
      ^ permalink raw reply related	[flat|nested] 15+ messages in thread

    • • * RE: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
          2023-05-16 17:24     ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano
        @ 2023-05-16 17:33       ` rsbecker
          2023-05-16 17:57       ` Taylor Blau
          1 sibling, 0 replies; 15+ messages in thread
        From: rsbecker @ 2023-05-16 17:33 UTC (permalink / raw)
          To: 'Junio C Hamano', 'Taylor Blau'
          Cc: git, 'Elijah Newren'
        
        On Tuesday, May 16, 2023 1:24 PM, Junio C Hamano wrote:
        >Recent header file shuffling missed this old user of ALLOC_GROW() that was
        inside
        >"#ifdef NO_PTHREADS' section and forgot to include the new file, alloc.h,
        that defines
        >the macro.
        >
        >Signed-off-by: Junio C Hamano <gitster@pobox.com>
        >
        >---
        > run-command.c | 1 +
        > 1 file changed, 1 insertion(+)
        >
        >diff --git c/run-command.c w/run-command.c index d4247d5fcc..1affea48af
        100644
        >--- c/run-command.c
        >+++ w/run-command.c
        >@@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code)  }
        >
        > #else
        >+#include <alloc.h>
        >
        > static struct {
        > 	void (**handlers)(void);
        
        Thanks gents.
        
        Regards,
        Randall
        
        
        ^ permalink raw reply	[flat|nested] 15+ messages in thread

      • * Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
          2023-05-16 17:24     ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano
          2023-05-16 17:33       ` rsbecker
        @ 2023-05-16 17:57       ` Taylor Blau
          2023-05-16 18:01         ` Taylor Blau
          1 sibling, 1 reply; 15+ messages in thread
        From: Taylor Blau @ 2023-05-16 17:57 UTC (permalink / raw)
          To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren
        
        On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote:
        > Recent header file shuffling missed this old user of ALLOC_GROW()
        > that was inside "#ifdef NO_PTHREADS' section and forgot to include
        > the new file, alloc.h, that defines the macro.
        
        Heh. I wrote an identical patch before lunch (which I just came back
        from). Feel free to queue either, I honestly do not care which as long
        as the bug is fixed :-).
        
          https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/
        
        Thanks,
        Taylor
        
        ^ permalink raw reply	[flat|nested] 15+ messages in thread

      • • * Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
            2023-05-16 17:57       ` Taylor Blau
          @ 2023-05-16 18:01         ` Taylor Blau
            2023-05-16 18:44           ` Junio C Hamano
            0 siblings, 1 reply; 15+ messages in thread
          From: Taylor Blau @ 2023-05-16 18:01 UTC (permalink / raw)
            To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren
          
          On Tue, May 16, 2023 at 01:57:50PM -0400, Taylor Blau wrote:
          > On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote:
          > > Recent header file shuffling missed this old user of ALLOC_GROW()
          > > that was inside "#ifdef NO_PTHREADS' section and forgot to include
          > > the new file, alloc.h, that defines the macro.
          >
          > Heh. I wrote an identical patch before lunch (which I just came back
          > from). Feel free to queue either, I honestly do not care which as long
          > as the bug is fixed :-).
          >
          >   https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/
          
          OK, having now read both completely, I would say I have a vague
          preference for my version since it keeps the include at the top and
          unconditional, and has slightly more information in the patch message.
          
          But I do not prefer it so much over yours that I would be sad if you had
          already queued yours and didn't want to bother shuffling it around.
          
          Thanks,
          Taylor
          
          ^ permalink raw reply	[flat|nested] 15+ messages in thread

        • • * Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
              2023-05-16 18:01         ` Taylor Blau
            @ 2023-05-16 18:44           ` Junio C Hamano
              2023-05-16 18:47             ` Taylor Blau
              0 siblings, 1 reply; 15+ messages in thread
            From: Junio C Hamano @ 2023-05-16 18:44 UTC (permalink / raw)
              To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren
            
            Taylor Blau <me@ttaylorr.com> writes:
            
            > OK, having now read both completely, I would say I have a vague
            > preference for my version since it keeps the include at the top and
            > unconditional, and has slightly more information in the patch message.
            >
            > But I do not prefer it so much over yours that I would be sad if you had
            > already queued yours and didn't want to bother shuffling it around.
            
            I do not have much preference between the two, either.  Both lack
            one important description that we are reasonably confident that this
            breakage is limited to run-command.c and no other files.
            
            Thanks.
            
            ^ permalink raw reply	[flat|nested] 15+ messages in thread

          • • * Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
                2023-05-16 18:44           ` Junio C Hamano
              @ 2023-05-16 18:47             ` Taylor Blau
                2023-05-16 18:53               ` Junio C Hamano
                0 siblings, 1 reply; 15+ messages in thread
              From: Taylor Blau @ 2023-05-16 18:47 UTC (permalink / raw)
                To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren
              
              On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote:
              > Taylor Blau <me@ttaylorr.com> writes:
              >
              > > OK, having now read both completely, I would say I have a vague
              > > preference for my version since it keeps the include at the top and
              > > unconditional, and has slightly more information in the patch message.
              > >
              > > But I do not prefer it so much over yours that I would be sad if you had
              > > already queued yours and didn't want to bother shuffling it around.
              >
              > I do not have much preference between the two, either.  Both lack
              > one important description that we are reasonably confident that this
              > breakage is limited to run-command.c and no other files.
              
              I believe that mine does:
              
                (Everything else compiles fine when NO_PTHREADS is defined, so this is
                the only spot that needs fixing).
              
              Thanks,
              Taylor
              
              ^ permalink raw reply	[flat|nested] 15+ messages in thread

            • • * Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation
                  2023-05-16 18:47             ` Taylor Blau
                @ 2023-05-16 18:53               ` Junio C Hamano
                  0 siblings, 0 replies; 15+ messages in thread
                From: Junio C Hamano @ 2023-05-16 18:53 UTC (permalink / raw)
                  To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren
                
                Taylor Blau <me@ttaylorr.com> writes:
                
                > On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote:
                >> Taylor Blau <me@ttaylorr.com> writes:
                >>
                >> > OK, having now read both completely, I would say I have a vague
                >> > preference for my version since it keeps the include at the top and
                >> > unconditional, and has slightly more information in the patch message.
                >> >
                >> > But I do not prefer it so much over yours that I would be sad if you had
                >> > already queued yours and didn't want to bother shuffling it around.
                >>
                >> I do not have much preference between the two, either.  Both lack
                >> one important description that we are reasonably confident that this
                >> breakage is limited to run-command.c and no other files.
                >
                > I believe that mine does:
                >
                >   (Everything else compiles fine when NO_PTHREADS is defined, so this is
                >   the only spot that needs fixing).
                
                Not quite.  Who says NO_PTHREADS is the only conditional that may
                hide use of ALLOC_GROW()?
                
                ^ permalink raw reply	[flat|nested] 15+ messages in thread

  • * [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
      2023-05-16 17:06 ` Taylor Blau
      2023-05-16 17:19   ` Junio C Hamano
    @ 2023-05-16 17:56   ` Taylor Blau
      2023-05-16 19:12     ` Junio C Hamano
      1 sibling, 1 reply; 15+ messages in thread
    From: Taylor Blau @ 2023-05-16 17:56 UTC (permalink / raw)
      To: git; +Cc: Junio C Hamano, Randall S. Becker, Elijah Newren
    
    When building git with `NO_PTHREADS=YesPlease`, we fail to build
    run-command.o since we don't have a definition for ALLOC_GROW:
    
        $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o
        GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty
            CC run-command.o
        run-command.c: In function ‘git_atexit’:
        run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]
         1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
              |         ^~~~~~~~~~
        cc1: all warnings being treated as errors
        make: *** [Makefile:2715: run-command.o] Error 1
    
    This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
    cache.h, 2023-02-24), which replaced includes of "cache.h" with
    "alloc.h", which is the new home of `ALLOC_GROW()` (and
    `ALLOC_GROW_BY()`).
    
    run-command.c compiles fine when `NO_PTHREADS` is undefined, since its
    use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything
    else compiles fine when NO_PTHREADS is defined, so this is the only spot
    that needs fixing).
    
    We could fix this by conditionally including "alloc.h" when
    `NO_PTHREADS` is defined.  But we have relatively few examples of
    conditional includes throughout the tree[^1].
    
    Instead, include "alloc.h" unconditionally in run-command.c to allow it
    to successfully compile even when NO_PTHREADS is defined.
    
    [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.
    
    Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
    Signed-off-by: Taylor Blau <me@ttaylorr.com>
    ---
     run-command.c | 1 +
     1 file changed, 1 insertion(+)
    
    diff --git a/run-command.c b/run-command.c
    index d4247d5fcc..60c9419866 100644
    --- a/run-command.c
    +++ b/run-command.c
    @@ -16,6 +16,7 @@
     #include "packfile.h"
     #include "hook.h"
     #include "compat/nonblock.h"
    +#include "alloc.h"
     
     void child_process_init(struct child_process *child)
     {
    -- 
    2.41.0.rc0.dirty
    
    ^ permalink raw reply related	[flat|nested] 15+ messages in thread

  • • * Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
        2023-05-16 17:56   ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau
      @ 2023-05-16 19:12     ` Junio C Hamano
        2023-05-16 19:13       ` rsbecker
        2023-05-16 21:33       ` Taylor Blau
        0 siblings, 2 replies; 15+ messages in thread
      From: Junio C Hamano @ 2023-05-16 19:12 UTC (permalink / raw)
        To: Taylor Blau; +Cc: git, Randall S. Becker, Elijah Newren
      
      Taylor Blau <me@ttaylorr.com> writes:
      
      > When building git with `NO_PTHREADS=YesPlease`, we fail to build
      > run-command.o since we don't have a definition for ALLOC_GROW:
      >
      >     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o
      >     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty
      >         CC run-command.o
      >     run-command.c: In function ‘git_atexit’:
      >     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]
      >      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
      >           |         ^~~~~~~~~~
      >     cc1: all warnings being treated as errors
      >     make: *** [Makefile:2715: run-command.o] Error 1
      
      I am OK to give a reproduction recipe, i.e. the "$ make" command
      line, to make it easy for people, who cannot remember how to define
      make variables from the command line, to try out themselves, but I
      hate the "build transcript" in the log message, when a few lines of
      prose suffices.
      
      How about this?
      
          Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS
          defined, i.e.
      
            $ make NO_PTHREADS=1 run-command.o
      
          as ALLOC_GROW() macro is used in the atexit() emulation but the
          macro definition is not available.
      
      > This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
      > cache.h, 2023-02-24), which replaced includes of "cache.h" with
      > "alloc.h", which is the new home of `ALLOC_GROW()` (and
      > `ALLOC_GROW_BY()`).
      
      Good.
      Insert something like:
      
          We can see that run-command.c is the only one that try to use
          these macros without including <alloc.h>.
      
            $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1
            $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2
            $ comm -23 /tmp/[12]
            compat/simple-ipc/ipc-unix-socket.c
            run-command.c
      
          The "compat/" file only talks about the macro in the comment,
          and is not broken.
      
      here.  What I am aiming for is to tell readers what they do not have
      to spend their time on.  As we encourage people to make sure to look
      for other errors that come from the same root cause when making a
      fix, avoiding duplicated work by telling what they do not have to
      look at is rather important.
      
      > run-command.c compiles fine when `NO_PTHREADS` is undefined, since its
      > use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything
      > else compiles fine when NO_PTHREADS is defined, so this is the only spot
      > that needs fixing).
      
      I think we can say that, but is probably coered by the three-line
      summary I gave above.
      
      > We could fix this by conditionally including "alloc.h" when
      > `NO_PTHREADS` is defined.  But we have relatively few examples of
      > conditional includes throughout the tree[^1].
      >
      > Instead, include "alloc.h" unconditionally in run-command.c to allow it
      > to successfully compile even when NO_PTHREADS is defined.
      
      Good.
      
      > [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.
      
      Good.
      
      ^ permalink raw reply	[flat|nested] 15+ messages in thread

    • • * RE: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
          2023-05-16 19:12     ` Junio C Hamano
        @ 2023-05-16 19:13       ` rsbecker
          2023-05-16 21:33       ` Taylor Blau
          1 sibling, 0 replies; 15+ messages in thread
        From: rsbecker @ 2023-05-16 19:13 UTC (permalink / raw)
          To: 'Junio C Hamano', 'Taylor Blau'
          Cc: git, 'Randall S. Becker', 'Elijah Newren'
        
        On Tuesday, May 16, 2023 3:12 PM, Junio C Hamano wrote:
        >Taylor Blau <me@ttaylorr.com> writes:
        >
        >> When building git with `NO_PTHREADS=YesPlease`, we fail to build
        >> run-command.o since we don't have a definition for ALLOC_GROW:
        >>
        >>     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o
        >>     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty
        >>         CC run-command.o
        >>     run-command.c: In function ‘git_atexit’:
        >>     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-
        >Werror=implicit-function-declaration]
        >>      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,
        >git_atexit_hdlrs.alloc);
        >>           |         ^~~~~~~~~~
        >>     cc1: all warnings being treated as errors
        >>     make: *** [Makefile:2715: run-command.o] Error 1
        >
        >I am OK to give a reproduction recipe, i.e. the "$ make" command line, to make it
        >easy for people, who cannot remember how to define make variables from the
        >command line, to try out themselves, but I hate the "build transcript" in the log
        >message, when a few lines of prose suffices.
        >
        >How about this?
        >
        >    Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS
        >    defined, i.e.
        >
        >      $ make NO_PTHREADS=1 run-command.o
        >
        >    as ALLOC_GROW() macro is used in the atexit() emulation but the
        >    macro definition is not available.
        >
        >> This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
        >> cache.h, 2023-02-24), which replaced includes of "cache.h" with
        >> "alloc.h", which is the new home of `ALLOC_GROW()` (and
        >> `ALLOC_GROW_BY()`).
        >
        >Good.
        >Insert something like:
        >
        >    We can see that run-command.c is the only one that try to use
        >    these macros without including <alloc.h>.
        >
        >      $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1
        >      $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2
        >      $ comm -23 /tmp/[12]
        >      compat/simple-ipc/ipc-unix-socket.c
        >      run-command.c
        >
        >    The "compat/" file only talks about the macro in the comment,
        >    and is not broken.
        >
        >here.  What I am aiming for is to tell readers what they do not have to spend their
        >time on.  As we encourage people to make sure to look for other errors that come
        >from the same root cause when making a fix, avoiding duplicated work by telling
        >what they do not have to look at is rather important.
        >
        >> run-command.c compiles fine when `NO_PTHREADS` is undefined, since its
        >> use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything
        >> else compiles fine when NO_PTHREADS is defined, so this is the only
        >> spot that needs fixing).
        >
        >I think we can say that, but is probably coered by the three-line summary I gave
        >above.
        >
        >> We could fix this by conditionally including "alloc.h" when
        >> `NO_PTHREADS` is defined.  But we have relatively few examples of
        >> conditional includes throughout the tree[^1].
        >>
        >> Instead, include "alloc.h" unconditionally in run-command.c to allow
        >> it to successfully compile even when NO_PTHREADS is defined.
        >
        >Good.
        >
        >> [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.
        >
        >Good.
        
        This all makes sense to me.
        
        
        ^ permalink raw reply	[flat|nested] 15+ messages in thread

      • * Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
          2023-05-16 19:12     ` Junio C Hamano
          2023-05-16 19:13       ` rsbecker
        @ 2023-05-16 21:33       ` Taylor Blau
          2023-05-18  4:16         ` Elijah Newren
          1 sibling, 1 reply; 15+ messages in thread
        From: Taylor Blau @ 2023-05-16 21:33 UTC (permalink / raw)
          To: Junio C Hamano; +Cc: git, Randall S. Becker, Elijah Newren
        
        On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote:
        > Taylor Blau <me@ttaylorr.com> writes:
        >
        > > When building git with `NO_PTHREADS=YesPlease`, we fail to build
        > > run-command.o since we don't have a definition for ALLOC_GROW:
        > >
        > >     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o
        > >     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty
        > >         CC run-command.o
        > >     run-command.c: In function ‘git_atexit’:
        > >     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]
        > >      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
        > >           |         ^~~~~~~~~~
        > >     cc1: all warnings being treated as errors
        > >     make: *** [Makefile:2715: run-command.o] Error 1
        >
        > I am OK to give a reproduction recipe, i.e. the "$ make" command
        > line, to make it easy for people, who cannot remember how to define
        > make variables from the command line, to try out themselves, but I
        > hate the "build transcript" in the log message, when a few lines of
        > prose suffices.
        
        Much appreciated. Here's a version with your changes include that is
        suitable for queueing:
        
        --- 8< ---
        
        Subject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
        
        Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined,
        i.e.
        
          $ make NO_PTHREADS=1 run-command.o
        
        as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the
        macro definition is not available.
        
        This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
        cache.h, 2023-02-24), which replaced includes of <cache.h> with
        <alloc.h>, which is the new home of `ALLOC_GROW()` (and
        `ALLOC_GROW_BY()`).
        
        We can see that run-command.c is the only one that try to use these
        macros without including <alloc.h>.
        
          $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1
          $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2
          $ comm -23 /tmp/[12]
          compat/simple-ipc/ipc-unix-socket.c
          run-command.c
        
        The "compat/" file only talks about the macro in the comment,
        and is not broken.
        
        We could fix this by conditionally including "alloc.h" when
        `NO_PTHREADS` is defined.  But we have relatively few examples of
        conditional includes throughout the tree[^1].
        
        Instead, include "alloc.h" unconditionally in run-command.c to allow it
        to successfully compile even when NO_PTHREADS is defined.
        
        [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.
        
        Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
        Co-authored-by: Junio C Hamano <gitster@pobox.com>
        Signed-off-by: Junio C Hamano <gitster@pobox.com>
        Signed-off-by: Taylor Blau <me@ttaylorr.com>
        ---
         run-command.c | 1 +
         1 file changed, 1 insertion(+)
        
        diff --git a/run-command.c b/run-command.c
        index d4247d5fcc..60c9419866 100644
        --- a/run-command.c
        +++ b/run-command.c
        @@ -16,6 +16,7 @@
         #include "packfile.h"
         #include "hook.h"
         #include "compat/nonblock.h"
        +#include "alloc.h"
        
         void child_process_init(struct child_process *child)
         {
        --
        2.41.0.rc0.dirty
        
        
        ^ permalink raw reply related	[flat|nested] 15+ messages in thread

      • • * Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
            2023-05-16 21:33       ` Taylor Blau
          @ 2023-05-18  4:16         ` Elijah Newren
            0 siblings, 0 replies; 15+ messages in thread
          From: Elijah Newren @ 2023-05-18  4:16 UTC (permalink / raw)
            To: Taylor Blau; +Cc: Junio C Hamano, git, Randall S. Becker
          
          On Tue, May 16, 2023 at 2:33 PM Taylor Blau <me@ttaylorr.com> wrote:
          >
          > On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote:
          > > Taylor Blau <me@ttaylorr.com> writes:
          > >
          > > > When building git with `NO_PTHREADS=YesPlease`, we fail to build
          > > > run-command.o since we don't have a definition for ALLOC_GROW:
          > > >
          > > >     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o
          > > >     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty
          > > >         CC run-command.o
          > > >     run-command.c: In function ‘git_atexit’:
          > > >     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]
          > > >      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
          > > >           |         ^~~~~~~~~~
          > > >     cc1: all warnings being treated as errors
          > > >     make: *** [Makefile:2715: run-command.o] Error 1
          > >
          > > I am OK to give a reproduction recipe, i.e. the "$ make" command
          > > line, to make it easy for people, who cannot remember how to define
          > > make variables from the command line, to try out themselves, but I
          > > hate the "build transcript" in the log message, when a few lines of
          > > prose suffices.
          >
          > Much appreciated. Here's a version with your changes include that is
          > suitable for queueing:
          >
          > --- 8< ---
          >
          > Subject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
          >
          > Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined,
          > i.e.
          >
          >   $ make NO_PTHREADS=1 run-command.o
          >
          > as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the
          > macro definition is not available.
          >
          > This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
          > cache.h, 2023-02-24), which replaced includes of <cache.h> with
          > <alloc.h>, which is the new home of `ALLOC_GROW()` (and
          > `ALLOC_GROW_BY()`).
          >
          > We can see that run-command.c is the only one that try to use these
          > macros without including <alloc.h>.
          >
          >   $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1
          >   $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2
          >   $ comm -23 /tmp/[12]
          >   compat/simple-ipc/ipc-unix-socket.c
          >   run-command.c
          >
          > The "compat/" file only talks about the macro in the comment,
          > and is not broken.
          >
          > We could fix this by conditionally including "alloc.h" when
          > `NO_PTHREADS` is defined.  But we have relatively few examples of
          > conditional includes throughout the tree[^1].
          >
          > Instead, include "alloc.h" unconditionally in run-command.c to allow it
          > to successfully compile even when NO_PTHREADS is defined.
          >
          > [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.
          >
          > Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
          > Co-authored-by: Junio C Hamano <gitster@pobox.com>
          > Signed-off-by: Junio C Hamano <gitster@pobox.com>
          > Signed-off-by: Taylor Blau <me@ttaylorr.com>
          > ---
          >  run-command.c | 1 +
          >  1 file changed, 1 insertion(+)
          >
          > diff --git a/run-command.c b/run-command.c
          > index d4247d5fcc..60c9419866 100644
          > --- a/run-command.c
          > +++ b/run-command.c
          > @@ -16,6 +16,7 @@
          >  #include "packfile.h"
          >  #include "hook.h"
          >  #include "compat/nonblock.h"
          > +#include "alloc.h"
          >
          >  void child_process_init(struct child_process *child)
          >  {
          > --
          > 2.41.0.rc0.dirty
          
          Thanks everyone for fixing up my issue, and my other issue in the
          imap-send/strbuf.h thread.  Sorry for any headaches.
          
          ^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-05-18  4:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-16 16:58 [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW rsbecker
2023-05-16 17:06 ` Taylor Blau
2023-05-16 17:19   ` Junio C Hamano
2023-05-16 17:24     ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano
2023-05-16 17:33       ` rsbecker
2023-05-16 17:57       ` Taylor Blau
2023-05-16 18:01         ` Taylor Blau
2023-05-16 18:44           ` Junio C Hamano
2023-05-16 18:47             ` Taylor Blau
2023-05-16 18:53               ` Junio C Hamano
2023-05-16 17:56   ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau
2023-05-16 19:12     ` Junio C Hamano
2023-05-16 19:13       ` rsbecker
2023-05-16 21:33       ` Taylor Blau
2023-05-18  4:16         ` Elijah Newren

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