go-curl icon indicating copy to clipboard operation
go-curl copied to clipboard

Fix/library not supporting curl version 8.0.x or higher, Closes #10

Open Laeri opened this issue 2 years ago • 11 comments

Closes #84

Laeri avatar Mar 22 '23 04:03 Laeri

Some of the errors disappeared, but I still have the following ones with my changes.

• Generating bindings: 
  ERROR   
          # github.com/Laeri/go-curl
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:148:34: could not determine kind of name for C.CURLOPT_CLOSEFUNCTION
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:127:34: could not determine kind of name for C.CURLOPT_FTPASCII
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:111:34: could not determine kind of name for C.CURLOPT_HTTPREQUEST
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:130:34: could not determine kind of name for C.CURLOPT_MUTE
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:78:34: could not determine kind of name for C.CURLOPT_NOTHING
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:142:34: could not determine kind of name for C.CURLOPT_PASSWDDATA
          ../../../go/pkg/mod/github.com/!laeri/[email protected]/const_gen.go:141:34: could not determine kind of name for C.CURLOPT_PASSWDFUNCTION

Laeri avatar Mar 22 '23 04:03 Laeri

Ok I found out that these options that result in an error when compiling were removed going from version 7.15.5 to 7.16.0. I am not sure how this works as compatgen should only have checked for minor version 10 or higher.

Do you think that simply requiring a minor version of 16 or higher is enough to finish this pull request?

Laeri avatar Mar 22 '23 05:03 Laeri

I got help on #go-nuts in Libera Chat to make this patch work. I was building a program that uses go-curl as a dependency (cameradar) and it used constants that this patch removes. I worked around the problem by adding them back into const_gen.go like so:

OPT_RTSP_STREAM_URI           = C.CURLOPT_RTSP_STREAM_URI
OPT_RTSP_REQUEST              = C.CURLOPT_RTSP_REQUEST
OPT_RTSP_TRANSPORT            = C.CURLOPT_RTSP_TRANSPORT
OPT_TIMEOUT_MS                = C.CURLOPT_TIMEOUT_MS

But the fundamental issue diagnosis I got on #go-nuts was:

It's looking specifically for any lines with a #define CURLOPT_ prefix, but looking at current curl sources, those options have been changed to be enum constants now.

So codegen.py would have to be updated to support the new format.

ilmari-lauhakangas avatar Apr 06 '23 09:04 ilmari-lauhakangas

@ilmari-lauhakangas Thanks for having a look at it. I am currently working on other things so I won't be able to do your suggested changes for your info. Hopefully you or someone else can implement the missing changes.

Laeri avatar Apr 18 '23 10:04 Laeri

Hi, If it can help in the meantime, I've fixed my go-curl usage with this trivial change: https://github.com/andelf/go-curl/compare/master...batmac:go-curl:batmac

batmac avatar Apr 18 '23 19:04 batmac

Hello @andelf. Thank you for go-curl. Would you be able to review some pending PRs which fix blockers? Thank you.

sprive avatar Jul 03 '24 14:07 sprive

@sprive Apologies, it's been quite some time since I've worked with Go. Since Go uses a repo-URL based package management system, I haven't given much attention to maintaining this particular repository.

I noticed this PR is still a working-in-progress. Could you provide some hints on which PR I should merge?

This project starts at the year 2011. It was the first Go code that implemented a bidirectional callback at that time.

andelf avatar Jul 03 '24 16:07 andelf

Hello @andelf Definitely do not apologize (this looks great and I see you are exceptionally busy on other projects/languages). I am grateful you put this together as I have some use cases that are best handled by libcurl. :-)

So... I had the same error as @Laeri - building go-curl on a system with libcurl 8 or higher. I solved my problem more simply by adding LIBCURL_VERSION_MAJOR == 7:

compa:t.h::

#if (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR == 10 && LIBCURL_VERSION_PATCH < 1) || (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR < 10)
#error your version is TOOOOOOOO low

^^ fixed my build failure. :party:

What I could do (please advise): option a) I pr only the above? option b) I extend the LIBCURL_VERSION_MAJOR == 7 (blindly) to every single check in compat.h? This type of change (and done without testing) gives me pause, if I don't a +1 first.

I would create a standalone PR for whichever you choose.

Unfortunately I'm not into libcurl (or C) enough to validate what all the different defines are for. But since these rules were written pre-curl 8, it seems to me to be OK in assuming they all mean LIBCURL_VERSION_MAJOR == 7.

sprive avatar Jul 05 '24 12:07 sprive

@sprive Feel free to add a PR.😁 I remember that compat.h is generated using a Python script, filling all missing symbols.

andelf avatar Jul 05 '24 19:07 andelf

Related: https://github.com/andelf/go-curl/pull/88

PR#88 has some of the cleanup contained in here (but does not try to address the main curl8 issue, yet)

sprive avatar Jul 06 '24 05:07 sprive

Related: #89

PR#89 contains the remainder of things not in the first PR above. (if 89 is accepted, then "this" PR can be closed. Or I will remind to do so afterwards)

sprive avatar Jul 16 '24 04:07 sprive