Rack library integration steps

Development
1

Ref: Static analysis issues · Issue #10 · Paul-Dempsey/pachde1 (github.com)

Now that Rack are running CodeQL and cppcheck during integration with the library, it would be great to have them in our build environment/CI, with the same configuration Rack are using, so that we can verify for ourselves that we’ve fixed the issues that will be raised during the process of publishing to the library.

It would be great if anything we need to pass in order to publish were built into the Rack SDK. (Maybe special targets for analysis). At the very least this should be documented what tools to use and how they’re configured for Rack, so that when issues are raised, we can self-verify our fixes before resubmitting to the library. Otherwise, releases can be delayed and it adds to the work of the library maintainers.

For build issues, many of us depend on @qno 's wonderful work: Building and releasing-plugins on github. I hate to ask for more when we’ve already received a wonderful gift.

What do you think?

1 Like
2

CodeQL is running the standard out-of-the-box configuration for language cpp:

codeql database create "${CODEQL_DB_DIR}" --language=cpp --source-root "${SOURCE_REPO}" --command="${build_script}" --overwrite
codeql database analyze "${CODEQL_DB_DIR}" --format=csv --output "${CODEQL_RESULTS_FILE}"

The build_script.sh contains:

make cleandep
make clean
make dep -j                      
make -j

cppcheck is invoked like this:

cppcheck src/ -isrc/dep --std=c++11 -j 8 -q --error-exitcode=1

I’ll document this somewhere at some point. For now, this is good enough, because I am still working through adjusting the process of analysis to be useful rather than a burden due to a million false positives.

Right now, there is not a fixed set of rules that need to be passed for integration. I run the analysis and anything that I determine looks worth fixing, I will raise with the developer. Generally, I look for things that are flagged as major issues or errors.

2 Likes
3

From @cschol

It won’t be integrated in the Rack SDK. It could be integrated in the VCV Rack Plugin Toolchain at some point, but for now I will run it manually during integration. CodeQL is the standard out-of-the-box configuration for for cpp.

cppcheck is invoked like this:

cppcheck src/ -isrc/dep --std=c++11 -j 8 -q --error-exitcode=1

Be careful searching for ccpcheck – I found an insecure site that looks just like what appears to be the official one. I wouldn’t download from that one – could be a spoof (or just a misconfigured server).

Here’s the official one (AFAIKT): Cppcheck - A tool for static C/C++ code analysis (sourceforge.io)

For CodeQL, I’m going to try the VSCode extension, since that’s my IDE.

Since passing these checks are a library requirement, it would be great to see the docs on the VCVRack/library: Database for the VCV Library (github.com) updated to include this info, including trustworthy links to the tools (and also how to cover all platform builds).

4

I think in the previous message he said the opposite. He runs it, but there are no fixed requirements at all.

1 Like
5

Yes, and no. A bug was filed in my repo, and my module was held, so effectively a requirement. But since I installed and ran cppcheck, I can see why there isn’t a fixed requirement, at least not yet. It’s currently being run with default settings, so the voluminous output has to be reviewed manually, and only things that look like real issues to the reviewer are being flagged.

There are some really good diagnostics, and I fixed a bunch of stuff. Great for re-learning C++.

Now I wish that the compiler warnings and errors in the Rack dependencies would get fixed, too. I like clean builds.

I also wish that the plugins I cloned would all build cleanly, to, or at least build. e.g. SquinkyVCVMain is broken for me since I brought down 2.4, so i should probably open an issue for Robert, and Bogaudio has never built successfully for me, but I haven’t taken the time to run down the issue.

6

well, since we are both saying the same thing, I don’t think with have to quibble about it.

Yeah, it sure would be nice if rack would build on all platforms without warnings. And, yeah, when I was making plugins I had no warnings on windows builds, but I’m pretty sure I had some warnings on mac builds. purely laziness on my part, but I suspect no harm done.

btw, I don’t know this cpp check program, but I do know that valgrind and especially address sanitizer will find actual runtime bugs. That said, I never found a bug in my modules with it, but I for sure have in my day job.

1 Like
7

You are generalizing from your specific case to all cases. I ran the analysis, saw a few issues, and logged an issue with your plugin asking you to check them out. I wasn’t particularly concerned about the issues and would have proceeded with integration, but I wanted to give you a chance to look at it in case you wanted to fix them.

If I am concerned about issues that prevent integration, I will reference the issue from the library thread and you will get a notification. Anything logged against your plugin and not referenced is like any other issue report and pure courtesy on my part.

2 Likes
8

Thank you for that! Now I understand. I appreciate the work. I was mostly reacting in surprise, I guess, and still learning the general culture of how things work, and what the expectations are, given that documentation is minimal.

Cppcheck is a good tool to have in the toolbox. It’s really good for (re) learning some of the gotchas in C++ that can lead to problems (calling virtual functions in a constructor, for example). I’m finding it quite useful.

2 Likes