5 ms·

> > But why not for instance use a build system in some "container"? > I am not sure how this helps. > > I think the project could "bother" contributors with

> > But why not for instance use a build system in some "container"? > I am not sure how this helps. > > I think the project could "bother" contributors with something like that, couldn't it? > Which project? OpenZFS. But this is kind of a misunderstanding on my side. I thought you were talking about convince for local developers. But you're talking about a CI system? Did I now understand correctly? But still a local developer needs a local build environment that is similar to the one on the CI which will do the checks later on. It's a common practice to bundle the whole tool-chain and all kind of checkers and scanners in a "build container". You can run it than locally as on the CI. But how to do bare metal tests on GitHub I actually don't know. Kernel divers are something special I guess. Not the usual workload that can be completely put into a container. One needs at least some VM setup. Never did that on GitHub. > > An embedded C developer I've talked with quite often on some other forum, who imho is quite competent, said that Coverity is a poor tool that generates way too much false negatives and overlooks at the same time glaring issues. > He likely violated a license agreement with Coverity, since no one is allowed to say anything comparing Coverity to anything else. LOL > I have never heard about this function. It is a static analyzer whose checks are written in the CodeQL language. Need to take a second look I guess. I had it differently in mind, but OK. Thanks for the pointer. > That said, if you have suggestions on QA, I am willing to listen to them, although keep in mind that it takes time for me to do things, even if I think they are good ideas, and I already have a large number of ideas to implement since returning to the project earlier this year. I'm not sure I can say much without having actually deeper insides into issues with the current QA setup. It sounded like it would be difficult to introduce tools for static analysis. But I'm not completely sure whether this is more a technical issue or a developer convenience and workflow thingy. At least some of such tools can be integrated into a fully packaged build tool-chain, like I said. This way you can rule out for example that someone commits stuff that triggers false positives in some code analysis tool that you can run also against your local build. Of course you still need checks on the CI in place. But those checks should actually never trigger by something that someone should have seen and resolved already locally (and if it does trigger this needs investigation usually anyway as the local build does not mirror the CI build in every aspect any more). This would relief the need to "ignore" checks on the CI without commits, I guess. Just do the checks you can do locally locally. Deliver the tools to do so in a convenient "package" ("container", VM image / config, whatever; you know, something like Vagrant). Showing test results in PRs or even outright reject PRs based on test results can be done in GitHub with Actions. One can also trigger Actions by special comments, useful for example for checks and benchmarks that only need to run occasionally when something related changes. But I guess you know all that… :-) Regarding code analysis: I think the stuff that Microsoft Research does is also interesting, and wasn't mentioned yet. But I'm not sure how practically relevant this is (especially in Unix land). They have at least on paper some tools for C code analysis. But no clue how stuff works in reality. Never heard directly from anybody who used this things.

ryao 4y ago
> OpenZFS. But this is kind of a misunderstanding on my side. I thought you were talking about convince for local developers. But you're talking about a CI system? Did I now understand correctly? I was actually asked by another OpenZFS developer about a good local tool for this. I did not have an answer then and I still do not have a great answer for it now, but I am getting closer to one. Some build system patches + Clang's static analyzer + codechecker seems to be the most promising combination since that would not only be able to cross translation unit analysis, but also be able to do differential reporting so only new reports are visible, but I still need to figure out all of the details. Also, I suspect the end result will be far less useful in practice than it sounds because of just how long the analysis takes. We would need a way to do incremental analysis like we can do incremental builds for it to be truly useful, and I currently do not have an answer for incremental analysis, although I suspect maybe something could be done in CodeChecker based on filesystem time stamps. Supposedly PVS Studio can do that, but as I explained my previous comment (in an edit that I probably made after you initially read it), PVS Studio is unlikely to be an option beyond its free trial period. That said, QA would be better achieved by integrating these tools into our continuous integration infrastructure so that they could be run whenever a pull request is done so that all newly reported issues are made available to all reviewers and we can avoid merging things that introduce regressions that static analyzers can catch in the first place. > But still a local developer needs a local build environment that is similar to the one on the CI which will do the checks later on. It's a common practice to bundle the whole tool-chain and all kind of checkers and scanners in a "build container". You can run it than locally as on the CI. It is easy to do builds on any Linux / FreeBSD machine. The test suite really should be run in a VM since running experimental code on your kernel is not advisable and is not always feasible. Machines that have root on ZFS cannot have that code loaded into the running kernel without significant inconveniences, which include breaking the system if you mess up in a way that completely breaks the driver. You can still use the ZFS stochastic testing tool called ztest. That does testing on a userland version of ZFS, although it is different from running the test suite. > But how to do bare metal tests on GitHub I actually don't know. Kernel divers are something special I guess. Not the usual workload that can be completely put into a container. One needs at least some VM setup. Never did that on GitHub. I think we are talking about different things here. github can integrate with buildbots and has its own runners that you can use via github workflows, but running the test suite has nothing to do with static analyzers, which I thought were the focus of the discussion. > Need to take a second look I guess. I had it differently in mind, but OK. I revised my response to ask if you had mistaken CodeQL for Coccinelle. > It sounded like it would be difficult to introduce tools for static analysis. It is infrastructure work. It is doable, but it takes time. I have been working on it. The addition of CodeQL to github pull requests was the first thing to come out of that. > But I'm not completely sure whether this is more a technical issue or a developer convenience and workflow thingy. If the developer is inundated with reports, it is useless. That is why we need to ensure that developers only see new reports from their code changes. We also need to be able to mark existing reports as false positives so that we do not have a bunch of unresolved reports. > At least some of such tools can be integrated into a fully packaged build tool-chain, like I said. This way you can rule out for example that someone commits stuff that triggers false positives in some code analysis tool that you can run also against your local build. Commits can only be done by a select few people and they are only done following code review. Asking the reviewers to download the code locally to redundantly run all of these tools on their local machines (which also implies inundating them with reports from pre-existing issues) is a fantastic way to cause developer burn out. That might sound silly, but real problems occur when you require developers to do menial tasks that require them to process huge amounts of mostly useless information. I have been through all of that myself and the burnout problem hit me. Excessive local testing is also a productivity killer, which becomes a morale killer when you put your tested patch in a pull request and then the buildbot not only finds a problem with it, but then demonstrates that all of your local testing was a waste of time since you now need to start from the beginning in retesting everything. It is far better to have the tools' reports added to the pull request by a machine for the reviewers to see in the PR, with pre-existing issues filtered from those reports. It avoids the "please end my suffering now" part of the development process that occurs when developers manually run all of the things they can locally every single time they make a small change, only to have the continuous integration infrastructure tell them to start over. > Of course you still need checks on the CI in place. But those checks should actually never trigger by something that someone should have seen and resolved already locally (and if it does trigger this needs investigation usually anyway as the local build does not mirror the CI build in every aspect any more). They do all the time. You simply cannot test all of the architectures, OS version combinations, etcetera locally that are tested by the continuous integration infrastructure. > This would relief the need to "ignore" checks on the CI without commits, I guess. I do not understand. > Just do the checks you can do locally locally. I used to do this. My productivity was several times lower, I still missed things that the continuous integration software caught and I burned myself out doing menial work by needing to manually run all of this. I really do not recommend it. Now I do much less of this and rely on the buildbot to do the menial work. > Deliver the tools to do so in a convenient "package" ("container", VM image / config, whatever; you know, something like Vagrant). I have a suspicion nobody would use it since everything they need is already fairly simple to setup. > Showing test results in PRs or even outright reject PRs based on test results can be done in GitHub with Actions. OpenZFS already does this. > One can also trigger Actions by special comments, useful for example for checks and benchmarks that only need to run occasionally when something related changes. But I guess you know all that… I did not know that part, but I find it to be more useful to always run all tests all the time. > Regarding code analysis: I think the stuff that Microsoft Research does is also interesting, and wasn't mentioned yet. But I'm not sure how practically relevant this is (especially in Unix land). They have at least on paper some tools for C code analysis. But no clue how stuff works in reality. Never heard directly from anybody who used this things. If they published them for third party use, I would be willing to look.

ryao 4y ago

> OpenZFS. But this is kind of a misunderstanding on my side. I thought you were talking about convince for local developers. But you're talking about a CI system? Did I now understand correctly? I was actually asked by another OpenZFS developer about a good local tool for this. I did not have an answer then and I still do not have a great answer for it now, but I am getting closer to one. Some build system patches + Clang's static analyzer + codechecker seems to be the most promising combination since that would not only be able to cross translation unit analysis, but also be able to do differential reporting so only new reports are visible, but I still need to figure out all of the details. Also, I suspect the end result will be far less useful in practice than it sounds because of just how long the analysis takes. We would need a way to do incremental analysis like we can do incremental builds for it to be truly useful, and I currently do not have an answer for incremental analysis, although I suspect maybe something could be done in CodeChecker based on filesystem time stamps. Supposedly PVS Studio can do that, but as I explained my previous comment (in an edit that I probably made after you initially read it), PVS Studio is unlikely to be an option beyond its free trial period. That said, QA would be better achieved by integrating these tools into our continuous integration infrastructure so that they could be run whenever a pull request is done so that all newly reported issues are made available to all reviewers and we can avoid merging things that introduce regressions that static analyzers can catch in the first place. > But still a local developer needs a local build environment that is similar to the one on the CI which will do the checks later on. It's a common practice to bundle the whole tool-chain and all kind of checkers and scanners in a "build container". You can run it than locally as on the CI. It is easy to do builds on any Linux / FreeBSD machine. The test suite really should be run in a VM since running experimental code on your kernel is not advisable and is not always feasible. Machines that have root on ZFS cannot have that code loaded into the running kernel without significant inconveniences, which include breaking the system if you mess up in a way that completely breaks the driver. You can still use the ZFS stochastic testing tool called ztest. That does testing on a userland version of ZFS, although it is different from running the test suite. > But how to do bare metal tests on GitHub I actually don't know. Kernel divers are something special I guess. Not the usual workload that can be completely put into a container. One needs at least some VM setup. Never did that on GitHub. I think we are talking about different things here. github can integrate with buildbots and has its own runners that you can use via github workflows, but running the test suite has nothing to do with static analyzers, which I thought were the focus of the discussion. > Need to take a second look I guess. I had it differently in mind, but OK. I revised my response to ask if you had mistaken CodeQL for Coccinelle. > It sounded like it would be difficult to introduce tools for static analysis. It is infrastructure work. It is doable, but it takes time. I have been working on it. The addition of CodeQL to github pull requests was the first thing to come out of that. > But I'm not completely sure whether this is more a technical issue or a developer convenience and workflow thingy. If the developer is inundated with reports, it is useless. That is why we need to ensure that developers only see new reports from their code changes. We also need to be able to mark existing reports as false positives so that we do not have a bunch of unresolved reports. > At least some of such tools can be integrated into a fully packaged build tool-chain, like I said. This way you can rule out for example that someone commits stuff that triggers false positives in some code analysis tool that you can run also against your local build. Commits can only be done by a select few people and they are only done following code review. Asking the reviewers to download the code locally to redundantly run all of these tools on their local machines (which also implies inundating them with reports from pre-existing issues) is a fantastic way to cause developer burn out. That might sound silly, but real problems occur when you require developers to do menial tasks that require them to process huge amounts of mostly useless information. I have been through all of that myself and the burnout problem hit me. Excessive local testing is also a productivity killer, which becomes a morale killer when you put your tested patch in a pull request and then the buildbot not only finds a problem with it, but then demonstrates that all of your local testing was a waste of time since you now need to start from the beginning in retesting everything. It is far better to have the tools' reports added to the pull request by a machine for the reviewers to see in the PR, with pre-existing issues filtered from those reports. It avoids the "please end my suffering now" part of the development process that occurs when developers manually run all of the things they can locally every single time they make a small change, only to have the continuous integration infrastructure tell them to start over. > Of course you still need checks on the CI in place. But those checks should actually never trigger by something that someone should have seen and resolved already locally (and if it does trigger this needs investigation usually anyway as the local build does not mirror the CI build in every aspect any more). They do all the time. You simply cannot test all of the architectures, OS version combinations, etcetera locally that are tested by the continuous integration infrastructure. > This would relief the need to "ignore" checks on the CI without commits, I guess. I do not understand. > Just do the checks you can do locally locally. I used to do this. My productivity was several times lower, I still missed things that the continuous integration software caught and I burned myself out doing menial work by needing to manually run all of this. I really do not recommend it. Now I do much less of this and rely on the buildbot to do the menial work. > Deliver the tools to do so in a convenient "package" ("container", VM image / config, whatever; you know, something like Vagrant). I have a suspicion nobody would use it since everything they need is already fairly simple to setup. > Showing test results in PRs or even outright reject PRs based on test results can be done in GitHub with Actions. OpenZFS already does this. > One can also trigger Actions by special comments, useful for example for checks and benchmarks that only need to run occasionally when something related changes. But I guess you know all that… I did not know that part, but I find it to be more useful to always run all tests all the time. > Regarding code analysis: I think the stuff that Microsoft Research does is also interesting, and wasn't mentioned yet. But I'm not sure how practically relevant this is (especially in Unix land). They have at least on paper some tools for C code analysis. But no clue how stuff works in reality. Never heard directly from anybody who used this things. If they published them for third party use, I would be willing to look.