Add a check-local target that runs our coccinelle parsing problems script

In #31705 (moved), we add a script that detects coccinelle parsing problems. We should run it when we run "make check", if coccinelle is installed.

changed milestone to %Tor: 0.4.3.x-final

added 042-can actualpoints::0.9 component::core tor/tor extra-review milestone::Tor: 0.4.3.x-final points::0.5 priority::medium resolution::fixed reviewer::catalyst reviewer::nickm severity::normal status::closed type::enhancement labels

See my draft pull request:

• master: https://github.com/torproject/tor/pull/1454

TODO:

• merge #31705 (moved) and rebase
• squash the two Travis commits
• add a changes file
• run 'make autostyle-operators' or 'make autostyle', and see if the exceptions file gets smaller

Trac:
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.3.x-final
Actualpoints: N/A to 0.3
Status: new to needs_review
Reviewer: N/A to nickm

Oh, it's based on #31705 (moved), you only want the last 6 commits.

Edit: counting is hard

It looks like we might need to upgrade to the xenial or bionic images to get a recent enough version of spatch. Testing now.

Trac:
Status: needs_review to needs_revision

The minimum required version seems to be ~1.0.4:

• 1.0.0-rc19 does not work
• 1.0.4 works on Debian
• 1.0.6 works on macOS

Quick comment on patch:

I'm not 100% sure about requiring everybody to install coccinelle for them to be able to run the pre-commit hook.

Replying to nickm:

Quick comment on patch:

I'm not 100% sure about requiring everybody to install coccinelle for them to be able to run the pre-commit hook.

If spatch is not installed, the script logs a message and exits without error: https://github.com/torproject/tor/pull/1454/commits/616bc21859c50f75e0f1092cab0dca0de62f47b9#diff-e4d63afb05764cd8a5149d2f09edd87dR21

I'm not entirely happy with the speed of spatch, but let's see how it goes. Maybe there's some nice way to do it in parallel.

I'm making progress on this, just doing performance checks now.

Trac:
Parent: #31705 (moved) to N/A

This was slightly more complicated than I expected, we needed Ubuntu Bionic for a recent spatch, and spatch is so slow that we can only check modified files.

See my PR:

• master: https://github.com/torproject/tor/pull/1462

Trac:
Actualpoints: 0.3 to 0.8
Status: needs_revision to needs_review

We should consider backporting "Travis: Use Ubuntu Bionic, but keep Trusty for chutney" before we merge this ticket.

I'll open a child ticket.

This looks pretty good to me. I'm only so-so on git hooks, though, so I'd like somebody else to take a second look at those changes before we merge.

Trac:
Status: needs_review to merge_ready

I force-pushed the branch to remove the commit and changes entry for "Travis: Use Ubuntu Bionic, but keep Trusty for chutney". I'm backporting that change in #32260 (moved), all the other changes go in master.

Marking as extra-review for the git hooks commits:

• scripts/git: Add check_cocci_parse.sh to the pre-commit hook
• scripts/git: Only check modified files in existing git hook checks
• scripts/git: Make spaces consistent in pre-push.git-hook

Here's how you can check them:

Coccinelle checks for correct C syntax. Here's how you can test it:

• Modify a file to have really bad C syntax
• Run a hook or make check-cocci
• Files in scripts/coccinelle/exceptions.txt are ignored, they have known bad syntax
• Files in src/ext are also ignored

pre-commit hook:

• At pre-push time, the pre-commit hook takes a list of changed files from the pre-push hook. So you can call the pre-commit hook manually with a list of files
• At pre-commit time, the pre-commit hook creates the changed file list itself. So you can call it with no arguments, and a set of staged (git add) files
• Install the hooks, and coccinelle, and try a commit

pre-push hook:

• Install the hooks, and coccinelle, and try a push

Trac:
Keywords: N/A deleted, extra-review added
Reviewer: nickm to N/A

Trac:
Status: merge_ready to needs_review

I manually tested the pre-commit and pre-push hooks. They seem to work as advertised.

Changes seem reasonable to me by visual inspection.

There are some whitespace nits in Makefile.am that I commented on in the pull request.

Trac:
Reviewer: N/A to catalyst
Status: needs_review to needs_revision

Fixed the spacing in the new code in the existing commit, and added another commit that removes spaces from all the other file lines.

#30991 (moved) will automatically detect or fix issues like this in future.

Merged to master.

Trac:
Resolution: N/A to fixed
Actualpoints: 0.8 to 0.9
Status: needs_revision to closed
Reviewer: catalyst to catalyst, nickm

closed

changed time estimate to 4h

added 7h 12m of time spent

mentioned in issue #32249 (moved)

mentioned in issue #32260 (moved)

mentioned in issue #32663 (moved)

moved to tpo/core/tor#31919 (closed)

mentioned in issue tpo/core/tor#32249 (closed)

mentioned in issue tpo/core/tor#32663 (closed)