Skip to content
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

Fix a few issues with the C generator (part 4) #20289

Merged
merged 6 commits into from
Dec 12, 2024

Conversation

eafer
Copy link
Contributor

@eafer eafer commented Dec 11, 2024

Four more patches from the original pull request at #14379. I reordered the patches this time because the last remaining two are somewhat harder to explain.

@wing328 @ityuhui @zhemant @michelealbano

I ran generate-samples.sh as usual, but I see something off with the results for petstore/c/. Not all of the cmake files are getting updated. I retried multiple times but I can't tell what I'm doing wrong.

EDIT: This PR includes the following changes:

  • Add support for binary arguments in the request body
  • Add support for enums in the request path
  • Update the schemas to test both 1 and 2
  • Update the cmake configuration so that headers are installed as well, not just the library
  • Update the cmake configuration so that a C++ compiler is not required for the build

@wing328 wing328 added this to the 7.11.0 milestone Dec 11, 2024
@eafer
Copy link
Contributor Author

eafer commented Dec 11, 2024

So the checks are passing, but I still think something is off because petstore/c is still being built as C++:

-- The C compiler identification is GNU 11.4.0
-- The CXX compiler identification is GNU 11.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped

while the other two samples are being built as plain C:

-- The C compiler identification is GNU 11.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done

I don't know why this is happening. It does get fixed if I delete the whole sample folder and generate it again, for some reason.

@wing328
Copy link
Member

wing328 commented Dec 11, 2024

@eafer in the description, can you please list out (point form) what issues are fixed by this PR?

@eafer
Copy link
Contributor Author

eafer commented Dec 11, 2024

@wing328 I just added the list of changes to the description. Let me know if you need anything else.

@wing328
Copy link
Member

wing328 commented Dec 11, 2024

Looks good 👍

@eafer
Copy link
Contributor Author

eafer commented Dec 11, 2024

I just deleted all the C samples and generated them from scratch. I don't understand enough to tell why the result is different, but it looks better now.

@eafer
Copy link
Contributor Author

eafer commented Dec 11, 2024

I guess that wasn't the right thing to do...

@wing328
Copy link
Member

wing328 commented Dec 12, 2024

We can improve the CI/tests with another PR instead.

@wing328 wing328 merged commit b7c7ed0 into OpenAPITools:master Dec 12, 2024
33 checks passed
@eafer
Copy link
Contributor Author

eafer commented Dec 12, 2024

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants