-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Shell scripts that are called by another scripts via sh foo.sh are not traced (./foo.sh is a workaround) #165
Comments
Hi! First, if /bin/sh is not bash (e.g., dash or similar), the --bash-parser= stuff will not work. You really should not have to use that, unless you want e.g., a specific version of bash to run with. As to the second issue, that unfortunately is a bit tricky. I think it's the same problem as in Issue #64 . If /bin/sh is really a symlink to bash, I think it would work though. How does it look on your system? |
Hi @SimonKagstrom, Fair point about the --bash stuff, consider that a debugging attempt ;) On this system, /bin/sh in a link to bash: Should sh be invoked with a full path rather than relative, maybe? |
OK, then it probably doesn't work when invoking that way. I think the reason is in Issue #51, bash scrubbing PS4 from the environment. Another thing to try is to use the --bash-method=DEBUG option, which doesn't use PS4, although I'm not that hopeful that it will propagate to new bashes either. |
@SimonKagstrom so I found that if I used:
instead of:
then it works. It seems to bail out early on large scripts though, winetricks is nearly 20k but kcov keeps capping out around 500 lines. Is that a known issue/user error, or should I file a new bug for that? |
OK, thanks for the test! In my case, it works if I use "bash foo", but not "sh foo" as well. But then I also have /bin/sh linked to dash:
Kcov doesn't have a script size limitation as such, but the parser is stupid and can get lost in the bash complexities. A proper shell script parser would be needed for the general case, see Issue #145. If you can find the exact line in the winetricks script, it might be obvious what goes wrong and a bandaid might be applied. I guess an alternative could be to make the parser "more stupid", and avoid trying to identify heredocs and other Bash peculiarities. Then it should not get lost as easily, but also mis-identify a lot of lines. |
The script aims to be POSIX, not bash, hence my trying to avoid bash workarounds. The sh syntax was an historical artifact, and ./ is fine for me to avoid the issue. winetricks has some heredocs, but that's not a bashism :). There shouldn't be any bashisms in winetricks, as I regularly run shellcheck and checkbashisms on it. That said, it definitely is a more complex script. I'll make a note to try seeing what breaks it, but I've got several plates in the air already, fyi. |
Sorry, what I mean is complexities in the regular shell syntax, not bash extensions. kcov doesn't understand shell script syntax through a real parser, but tries to keep track of e.g., heredocs and other multi-line constructions. Probably this is what happens here: It gets lost in a call to awk or something similar, and then thinks the rest of the file is non-code. A test could be to modify bash-engine.cc to only disregard empty lines and full-comment lines. Then I think the script should be fully parsed, but with a lot of false uncovered code lines. |
From a quick glance of the output, it's breaking on a usage of w_ahk_do(), which is a function takes a multiple line input that it then runs through AutoHotKey. There are two earlier usages that it parsed okay.
kcov analyzes up until line 1356, then bails. However, it shows those lines as unexecuted (they were executed). Thanks for the tip |
I've pushed a small patch which adds an option to run with a simpler bash parser. Run with
that might give better results with more complex scripts like winetricks. |
Hi @SimonKagstrom, so that definitely gets the line count up. It appears to scan the entire file now (~12k lines, which looks about right once newlines and comments are excluded). Unfortunately, it misses large portions of the file, only 160 lines are counted as executed. Most of the executed code appears to be in the main script, but not most functions (though a few pieces of code in smaller functions was executed). My guess is that nested functions or more complex shell constructs cause it to be ignored. I've subscribed to #145 and will be watching that. Let me know if you need any testing done :) |
That's strange though. If you run bash with -x, does those lines show up? I.e.,
kcov uses -x internally (unless --bash-method=DEBUG is passed on the command line). |
I must warn that #145 is not something I'm likely to work on in the short run. It's a big job and I personally mainly (almost exclusively) use kcov for compiled code. |
I figured as much, but thanks for explicitly confirming. I'm going ahead and getting the infrastructure in place to test, mostly because I haven't found anything that works better. Do you know of any viable alternatives for sh code? |
There are a few other coverage collectors for bash/sh (including my own shcov), but I believe they all use PS4 (bash -x) internally, or perhaps the debug method as outlined above. Thus, if the basic/simple parser doesn't work reliably, I don't think you'll get better results with another collector. Trying the --bash-method=DEBUG option could be good though, although I don't think it will make a difference. |
Hello,
I found kcov, and was very anxious to try it on my shell project (https://github.com/winetricks/winetricks). I built from source (HEAD 045d4e5), and ran kcov with:
that appeared to work, but got me coverage results for gmake. So, I switched to running the test wrapper script directly, and also tried bash/sh options (shebang is #/bin/sh), but that made no difference:
So, I tried making a simple testcase:
script 1:
script 2:
put both in same dir, and chmod +x, then try kcov:
So script2 was invoked, which is expected. But if you look at kcov/index.html, script2 is not listed (but script1 got 100% coverage, woohoo ;) ).
The text was updated successfully, but these errors were encountered: