Skip to content

C++: Generate flow summaries for curl/curl #19596

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MathiasVP
Copy link
Contributor

Similar to what we did in #19492, but for curl/curl.

I've used the database coming from DCA to generate the models.

@Copilot Copilot AI review requested due to automatic review settings May 27, 2025 11:54
@MathiasVP MathiasVP requested a review from a team as a code owner May 27, 2025 11:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@github-actions github-actions bot added the C++ label May 27, 2025
@jketema
Copy link
Contributor

jketema commented May 27, 2025

I assume you generated these based on the slug we have in DCA? If so, did you check that the autobuild configuration we use there actually makes sort-of sense, and is not a configuration that disables a lot of things?

@MathiasVP
Copy link
Contributor Author

MathiasVP commented May 27, 2025

I assume you generated these based on the slug we have in DCA? If so, did you check that the autobuild configuration we use there actually makes sort-of sense, and is not a configuration that disables a lot of things?

Yeah, I used the slug from DCA. I briefly checked that the (arguably few) summaries I expected to be there were present, but I didn't investigate whether the autobuild configuration made sense. I'm happy to spend some time investigating this before we merge this PR

@jketema
Copy link
Contributor

jketema commented May 27, 2025

I'm happy to spend some time investigating this before we merge this PR

I think that would make sense.

I also see you have some test failures.

@MathiasVP MathiasVP marked this pull request as draft May 27, 2025 12:19
@MathiasVP
Copy link
Contributor Author

Thanks. (Luckily those files are simply changes that needs to be accepted!)

I see that the autobuilder is trying to run:

-DCMAKE_VERBOSE_MAKEFILE=ON -DBUILD_DOCS=OFF -DCATKIN_ENABLE_TESTING=OFF -DBUILD_DOCUMENTATION=OFF -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS= -DCMAKE_C_FLAGS=

This looks fairly sensible. In particular, it doesn't disable any non-documentation/non-testing features.

What I did notice was that curl (obviously) depends on a lot of other more low-level libraries that we may want to add models for first. The ones I recognized were:

  • google/brotli
  • madler/zlib
  • nghttp2/nghttp2
  • libidn/libidn2
  • libssh2/libssh2
  • libuv/libuv

I think it's high time that I reorganize the DCA repositories relevant for MaD generation slightly better so that we can get all of these dependencies modeled first. So I think I'll work on that.

@jketema
Copy link
Contributor

jketema commented May 27, 2025

Thanks for looking at the DCA configuration. I see you moved this back into draft. Is your intention to add models for the used libraries first, before continuing with this?

@MathiasVP
Copy link
Contributor Author

Thanks for looking at the DCA configuration. I see you moved this back into draft. Is your intention to add models for the used libraries first, before continuing with this?

Yeah, I think I want to do that to ensure that we get the proper models

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