Clean up unnecessary symbol existence checks #2370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
BurntBrunch
force-pushed
the
vendor-libbpf
branch
4 times, most recently
from
794b9af
to
a83b0d1
Compare
BurntBrunch
force-pushed
the
vendor-libbpf
branch
2 times, most recently
from
9a3de98
to
ad05431
Compare
BurntBrunch
force-pushed
the
vendor-libbpf
branch
7 times, most recently
from
5fc8cee
to
456d08e
Compare
Currently, the test runner can use variables in the `except` branch
before they're initialized. Hoist all the local state variables to
before the try: to fix that.
Also, make BEFORE_PID logic run only if the test uses `{{BEFORE_PID}}`.
BurntBrunch
force-pushed
the
vendor-libbpf
branch
from
456d08e
to
eb5b7db
Compare
|
Alright, I think I found a fix for the flaky usdt test (or at least made it better?). For reasons I don't understand, I was only able to fix it by writing a new C binary that forks and execs the necessary two instances. Doing so from bash made the BEFORE phase time out (wtf GH Actions; I suspect some layer of security hooks or auditing slowing things down). Doing so from a bash script doesn't work because |
added 10 commits
November 1, 2022 15:21
The way the test runner determines that the BEFORE clause has started is by splitting the string by whitespace and calling `pidof` with the last substring. In this test, we launch the same program twice, so there exists a time between the two invocations where `pidof` will return the pid for the first process but we wouldn't have launched the second one yet. bpftrace would then launch and only attach to the first process and the test will time out. The fix is somewhat hacky - we build a small C binary which runs one instance as a child and one instance as the parent process. (I tried bash-based fixes in previous iterations to no avail; there is something going on inside GH Actions but I can't figure out what.)
BurntBrunch
force-pushed
the
vendor-libbpf
branch
from
eb5b7db
to
5fe7b07
Compare
viktormalik
approved these changes
Nov 2, 2022
I'll document it in INSTALL.md and README.md and send a separate PR |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(This is based on top of #2369, look at the libbpf: and libbcc: commits only in this PR)
(Part of #2334 work)
Now that we have strict requirements for minimum library versions, this PR removes unnecessary libbpf and bcc symbol checks in the bpftrace codebase. Now that these are cleaned up, we may even be able to set up libbpf and bcc as ExternalProjects, if we so desired (though I'd personally prefer to keep them as git submodules).