forked from git-for-windows/git
    
        
        - 
                Notifications
    You must be signed in to change notification settings 
- Fork 106
          Fix osx-gcc CI build failures
          #762
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Merged
      
      
            dscho
  merged 5 commits into
  microsoft:vfs-2.49.0
from
dscho:curl-options-want-long-instead-of-int-msft-git
  
      
      
   
  Jun 6, 2025 
      
    
                
     Merged
            
            
  
    Fix osx-gcc CI build failures
  
  #762
              
                    dscho
  merged 5 commits into
  microsoft:vfs-2.49.0
from
dscho:curl-options-want-long-instead-of-int-msft-git
  
      
      
   
  Jun 6, 2025 
              
            
      
        
          +42
        
        
          −42
        
        
          
        
      
    
  
Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    The curl documentation specifies that curl_easy_setopt() takes either: ...a long, a function pointer, an object pointer or a curl_off_t, depending on what the specific option expects. But when we pass an integer constant like "0", it will by default be a regular non-long int. This has always been wrong, but seemed to work in practice (I didn't dig into curl's implementation to see whether this might actually be triggering undefined behavior, but it seems likely and regardless we should do what the docs say). This is especially important since curl has a type-checking macro that causes building against curl 8.14 to produce many warnings. The specific commit is due to their 79b4e56b3 (typecheck-gcc.h: fix the typechecks, 2025-04-22). Curiously, it does only seem to trigger when compiled with -O2 for me. We can fix it by just marking the constants with a long "L". Backported-from: 6f11c42 (curl: fix integer constant typechecks with curl_easy_setopt(), 2025-06-04) Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
As discussed in the previous commit, we should be passing long integers,
not regular ones, to curl_easy_setopt(), and compiling against curl 8.14
loudly complains if we don't.
That patch fixed integer constants by adding an "L". This one deals with
actual variables.
Arguably these variables could just be declared as "long" in the first
place. But it's actually kind of awkward due to other code which uses
them:
  - port is conceptually a short, and we even call htons() on it (though
    weirdly it is defined as a regular int).
  - ssl_verify is conceptually a bool, and we assign to it from
    git_config_bool().
So I think we could probably switch these out for longs without hurting
anything, but it just feels a bit weird. Doubly so because if you don't
set USE_CURL_FOR_IMAP_SEND set, then the current types are fine!
So let's just cast these to longs in the curl calls, which makes what's
going on obvious. There aren't that many spots to modify (and as you can
see from the context, we already have some similar casts).
Backported-from: 30325e2 (curl: fix integer variable typechecks with curl_easy_setopt(), 2025-06-04)
Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
    As with the previous two commits, we should be passing long integers, not regular ones, to curl_easy_setopt(), and compiling against curl 8.14 loudly complains if we don't. This patch catches the remaining cases, which are ones where we pass curl's own symbolic constants. We'll cast them to long manually in each call. It seems kind of weird to me that curl doesn't define these constants as longs, since the point of them is to pass to curl_easy_setopt(). But in the curl documentation and examples, they clearly show casting them as part of the setopt calls. It may be that there is some reason not to push the type into the macro, like backwards compatibility. I didn't dig, as it doesn't really matter: we have to follow what existing curl versions ask for anyway. Backported-from: 4558c8f (curl: fix symbolic constant typechecks with curl_easy_setopt(), 2025-06-04) Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
As of Homebrew's update to cURL v8.14.0, there are new compile errors to
be observed in the `osx-gcc` job of Git's CI builds:
  In file included from http.h:8,
                   from imap-send.c:36:
  In function 'setup_curl',
      inlined from 'curl_append_msgs_to_imap' at imap-send.c:1460:9,
      inlined from 'cmd_main' at imap-send.c:1581:9:
  /usr/local/Cellar/curl/8.14.0/include/curl/typecheck-gcc.h:50:15: error: call to '_curl_easy_setopt_err_long' declared with attribute warning: curl_easy_setopt expects a long argument [-Werror=attribute-warning]
     50 |               _curl_easy_setopt_err_long();                             \
        |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/local/Cellar/curl/8.14.0/include/curl/curl.h:54:7: note: in definition of macro 'CURL_IGNORE_DEPRECATION'
     54 |       statements \
        |       ^~~~~~~~~~
  imap-send.c:1423:9: note: in expansion of macro 'curl_easy_setopt'
   1423 |         curl_easy_setopt(curl, CURLOPT_PORT, srvc->port);
        |         ^~~~~~~~~~~~~~~~
  [... many more instances of nearly identical warnings...]
See for example this CI workflow run:
https://github.com/git/git/actions/runs/15454602308/job/43504278284#step:4:307
The most likely explanation is the entry "typecheck-gcc.h: fix the
typechecks" in cURL's release notes (https://curl.se/ch/8.14.0.html).
Nearly identical compile errors afflicted recently-updated Debian
setups, which have been addressed by `jk/curl-easy-setopt-typefix`.
However, on macOS Git is built with different build options, which
uncovered more instances of `int` values that need to be cast to
constants, which were not covered by 6f11c42 (curl: fix integer
constant typechecks with curl_easy_setopt(), 2025-06-04). Let's
explicitly convert even those remaining `int` constants in
`curl_easy_setopt()` calls to `long` parameters.
In addition to looking at the compile errors of the `osx-gcc` job, I
verified that there are no other instances of the same issue that need
to be handled in this manner (and that might not be caught by our CI
builds because of yet other build options that might skip those code
parts), I ran the following command and inspected all 23 results
manually to ensure that the fix is now actually complete:
  git grep -n curl_easy_setopt |
  grep -ve ',.*, *[A-Za-z_"&]' \
    -e ',.*, *[-0-9]*L)' \
    -e ',.*,.* (long)'
Signed-off-by: Johannes Schindelin <[email protected]>
    Just like we just did in the backport from my upstream contribution, let's convert the `curl_easy_setopt()` calls in `gvfs-helper.c` that still passed `int` constants to pass `long` instead. Signed-off-by: Johannes Schindelin <[email protected]>
a7c060c    to
    718faf7      
    Compare
  
    
              
                    mjcheetham
  
              
              approved these changes
              
                  
                    Jun 6, 2025 
                  
              
              
            
            
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
The
osx-gccjob fails with compiler errors due to the now-stricter type checks in thecurl_easy_setopt()calls. There is an upstream contribution already to fix this in Git's own main branch, but those patches to not apply cleanly on top ofvfs-2.49.0, therefore I backported them.Range-diff
1: 6f11c42 ! 1: 3b1e099 curl: fix integer constant typechecks with curl_easy_setopt()
@@ Commit message We can fix it by just marking the constants with a long "L". + Backported-from: 6f11c42e8edc (curl: fix integer constant typechecks with curl_easy_setopt(), 2025-06-04) Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> + Signed-off-by: Johannes Schindelin <[email protected]> ## http-push.c ## @@ http-push.c: static char *xml_entities(const char *s) @@ http-push.c: static char *xml_entities(const char *s) curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, fwrite_null); ## http.c ## +@@ http.c: static int has_proxy_cert_password(void) + + static void set_curl_keepalive(CURL *c) + { +- curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1); ++ curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1L); + } + + /* Return 1 if redactions have been made, 0 otherwise. */ @@ http.c: static CURL *get_curl_handle(void) die("curl_easy_init failed"); @@ http.c: static CURL *get_curl_handle(void) if (curl_ssl_try) curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY); -@@ http.c: static CURL *get_curl_handle(void) - } - init_curl_proxy_auth(result); - -- curl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1); -+ curl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1L); - - if (curl_tcp_keepidle > -1) - curl_easy_setopt(result, CURLOPT_TCP_KEEPIDLE, ## remote-curl.c ## @@ remote-curl.c: static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)2: 30325e2 ! 2: ecd7e78 curl: fix integer variable typechecks with curl_easy_setopt()
@@ Commit message going on obvious. There aren't that many spots to modify (and as you can see from the context, we already have some similar casts). + Backported-from: 30325e23ba0d (curl: fix integer variable typechecks with curl_easy_setopt(), 2025-06-04) Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> + Signed-off-by: Johannes Schindelin <[email protected]> ## imap-send.c ## @@ imap-send.c: static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)3: 4558c8f ! 3: a53e8f6 curl: fix symbolic constant typechecks with curl_easy_setopt()
@@ Commit message dig, as it doesn't really matter: we have to follow what existing curl versions ask for anyway. + Backported-from: 4558c8f84b2f (curl: fix symbolic constant typechecks with curl_easy_setopt(), 2025-06-04) Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> + Signed-off-by: Johannes Schindelin <[email protected]> ## http.c ## @@ http.c: static CURL *get_curl_handle(void) - - if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && - !http_schannel_check_revoke) { -- curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); -+ curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, (long)CURLSSLOPT_NO_REVOKE); - } - - if (http_proactive_auth != PROACTIVE_AUTH_NONE) -@@ http.c: static CURL *get_curl_handle(void) } curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20L);4: 80de749 = 4: 560fdc1 curl: pass
longvalues where expected-: ------------ > 5: 718faf7 gvfs-helper: pass
longvalues where expectedObviously, we also needed to address some calls in
gvfs-helper, which means that the tip commit of this PR branch is not a backport.