6 ms·

Grep one-liners as CI tasks

111 points by fphilipe 5y ago

jrockway 5y ago
Don't you worry about what happens when the translation software produces a semantically equivalent empty string, or something? Like `<string><![CDATA[]]></string>`. An XML parser will find that to be empty, grep will be like "ooh lots of text in there".
rtuin 5y ago
Oh the power of Linux. I’m also using grep in CI as a simple but effectieve check on dependency violations in some codebases.
gravypod 5y ago
I think this is a great example of where build systems, that understand the need for testing, can help out. In a build system I use quite often (Bazel) you can express this as an `sh_test()` [0] which provides documentation about what you're attempting to do and provides you with a way to reproduce the failure locally. You don't have to push the code and wait for CI to fail to find out, or debug, this error. Extra fun thing to do: print a message describing why the decision was made and how to resolve the error in the failure message of your test! [0] - https://docs.bazel.build/versions/main/be/shell.html#sh_test https://docs.bazel.build/versions/main/be/shell.html#sh_test
- joatmon-snoo 5y ago
  Tests are a good way to assert an invariant that you expect of your codebase, but as with all things, resolving the error can get a bit tricky/frustrating. The canonical example in my mind is any kind of autofix-able linter, where there's some kind of patch (or more nuanced autofix) that the linter can generate on-the-spot for you. With a sh_test construct (or any other test), you generally find yourself printing out some command that the user can run to fix things, which in a sufficiently large codebase can get really frustrating. (The worst offenders of this that I remember dealing with at Google were when generated code/artifacts had to be checked into the repo instead of getting being wired into the build system because <insert hermeticity problem> and a large-scale change you were doing looked at one of them the wrong way...) (My company - https://trunk.io https://trunk.io - is actually building a universal linter as part of our product offering, and we already have a system to write custom linters with varying levels of sophistication that can plug into both your IDE and CI system!)
  - vlovich123 5y ago
    The problem with automatic linting is that there’s no good answer. Either you just fail and ask the user to resubmit or you fix up the formatting on their behalf. If you do the latter, you’re likely also pushing to the review system meaning the code under review and your local version no longer match which causes problems when rebasing and whatnot. The secondary problem is that the lint tool chain running locally sometimes doesn’t version match what’s in CI which poses a set of problems too, although in practice I think that’s raised as a concern only by teams that don’t establish good hygiene of checking in the set of tools (maintenance of multiple platforms is real there though). I’ve yet to see a good answer to this problem though as each approach has upsides/downsides and no one ever ends up happy in a large enough team. Good luck.
    - joatmon-snoo 5y ago
      Thanks for the feedback! All of those are very real problems and we've spent a lot of time thinking about how to handle them: > Either you just fail and ask the user to resubmit or you fix up the formatting on their behalf. We only apply autofixes when running on the user's machine; when running on CI, as you point out, we only show them because of Git's decentralized nature here. > The secondary problem is that the lint tool chain running locally sometimes doesn’t version match what’s in CI which poses a set of problems too, although in practice I think that’s raised as a concern only by teams that don’t establish good hygiene of checking in the set of tools (maintenance of multiple platforms is real there though). We've built out our own system for managing (mostly) hermetic toolchain installs, and require users to version their linters in their trunk.yaml configuration right now. (There are some open questions here around how we handle version skew between our toolchain version and the system-installed toolchain version.) We're also well aware of the problems that emerge as you scale to a larger team; hierarchical configs are one of the things that we've discussed but are waiting until we have more traction to flesh out. (All of us working on the linter side have experience setting tech direction for orgs of 100+ engineers).
  - gravypod 5y ago
    > (The worst offenders of this that I remember dealing with at Google were when generated code/artifacts had to be checked into the repo instead of getting being wired into the build system because <insert hermeticity problem> and a large-scale change you were doing looked at one of them the wrong way...) Been there. It's unfortunate because there are `genrule()`s for this very reason. But, definitely. Linting is very important and trunk looks great! But linting and config validation are very different use cases. At a previous company I had test that loaded a set of configs from repo and made sure all of them were valid with app level logic. It was crude but helpful. Trunk would be ideal. Also, question, have you thought of writing your own linter+autoformatter with treesitter? Are you planning to expand into other tooling?
- hiptobecubic 5y ago
  I was really annoyed when I first started using bazel, but it really is just an excellent build system. All the things I thought I didn't like about it, I have come to like.
eminence32 5y ago
The semgrep[1] tool seems like the logical next step, for when you've outgrown plain ol' grep. [1] https://semgrep.dev/ https://semgrep.dev/
jeffbee 5y ago
grep and other shell tool one-liners also make excellent kubernetes liveness checks. I have some kubernetes daemonsets that don't do anything other than assert that things on the node are as they should, via grep exit status.
- nickjj 5y ago
  It's also useful for alternative health check commands depending on which environment you're in. For example you could configure a Docker Compose health check to curl an endpoint for a 200 in production but in development override the health check command to call /bin/true so you don't get log spam in development from your health check while ensuring the health check still passes since /bin/true is an extremely minimal command that returns an exit code of 0. This can be controlled by a single environment variable too, no changes needed to your docker-compose.yml file. I covered this pattern in a recent DockerCon talk at https://nickjanetakis.com/blog/best-practices-around-production-ready-web-apps-with-docker-compose https://nickjanetakis.com/blog/best-practices-around-product....
david_allison 5y ago
Android's default linting already contains a "missing translation" lint rule which you can activate: "MissingTranslation"[0] For Android specifically: Gradle and Android Studio both support a powerful linting framework (to the level that it can provide auto-fixes to the IDE). It's better to provide an in-editor to guide your contributors before it hits CI, then have CI nag if they didn't fix the in-editor warnings/errors: Some examples of custom lint rules[1] and the default rules which Android Studio runs[2]: [0] https://android.googlesource.com/platform/tools/base/+/32923407563b7e2c943cf372e3cd9d4398d09789/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/TranslationDetector.java#79 https://android.googlesource.com/platform/tools/base/+/32923... [1] https://github.com/ankidroid/Anki-Android/tree/master/lint-rules/src/main/java/com/ichi2/anki/lint/rules https://github.com/ankidroid/Anki-Android/tree/master/lint-r... [2] https://github.com/ankidroid/Anki-Android/blob/master/lint-release.xml https://github.com/ankidroid/Anki-Android/blob/master/lint-r...
excircul 5y ago
> I personally prefer ripgrep as it is much faster than grep, but usually that is not available on CI machines. I recommend git grep, which is comparable in speed to ripgrep, since it ignores non-tracked files and searches the object storage directly. It is also able to run in parallel.
cfors 5y ago
In today's world of excellent CLI tools I don't think grep is a good choice, especially for checking irregular languages like XML. [0] I use tools like `jq` [1] or `yq` [2] all the time for CI checks. One useful check, is we have a configuration file stored as several hundred lines of YAML. Its a nice thing to maintain a sorted order for that, so we have a git pre-commit hook that runs the following: > yq eval --inplace '.my_key|= sort' my_file.yaml Of course, a pre-commit hook or CI both work. There's pros and cons of both. For our team, the pre-commit hook is a low enough level of effort, and doesn't require a CI check for something that executes in milliseconds. [0] https://stackoverflow.com/a/1732454 https://stackoverflow.com/a/1732454 [1] https://github.com/stedolan/jq https://github.com/stedolan/jq [2] https://github.com/mikefarah/yq https://github.com/mikefarah/yq
zX41ZdbW 5y ago
Here is my favorite: https://github.com/ClickHouse/ClickHouse/blob/master/utils/check-style/check-style#L251 https://github.com/ClickHouse/ClickHouse/blob/master/utils/c... "Too many exclamation marks"
- darknavi 5y ago
  I wonder how they picked three as too many. Why not two? > Three shall be the number thou shalt count, and the number of the counting shall be three. Four shalt thou not count, neither count thou two, excepting that thou then proceed to three. Five is right out.
  - irrational 5y ago
    This is the obvious answer.
  - deleted 5y ago
    [deleted]
  - chucksmash 5y ago
    `!!thing` is a fairly common JS idiom for "I have a truthy/falsy thing and I want true or false."
  - rbonvall 5y ago
    Maybe because of the use of the "double bang" pattern to convert a value to boolean: !!41 === true !!0 === false
  - accoil 5y ago
    Two really does seem too many. I worry about these times when any passing coder can ni-gate at will.
- ljm 5y ago
  I enjoy seeing a nice shell script. This one is easy to follow and is consistently formatted.
matmann2001 5y ago
Wouldn't this example do the incorrect thing in the event of a grep error (exit code 2)?
- CGamesPlay 5y ago
  Depends on what you mean by wrong. If the tests can’t run successfully because they detect an error or because they have an error, it still is a valuable signal to raise to the developer. Consider what would happen if your unit test framework couldn’t compile the test files and failed with an error. Isn’t that also a test failure?
  - Normal_gaussian 5y ago
    What would happen is the test would pass, as the logical not of 2 is 0. To address this: many CI pipelines allow you to specify which exot codes to succeed and fail on per job, or if it doesn't the one liner will need some boolean logic to check the exit code.
- tyingq 5y ago
  I guess testing specifically for an exit code of 1 isn't as easy to remember. (grep -q notwanted /hay/stack ; test $? == 1) && echo "No violations found" But you're right...the simpler format would do the wrong thing if, for example, the file didn't exist or had bad permissions or was a directory instead of a file.
joatmon-snoo 5y ago
Shameless plug: if you find yourself wanting more advamced custom painting, you can try http://trunk.io http://trunk.io, which provides you with three different ways to write your own linters (pass/fail based on your script's exit code, spit out a patch that should be applied to your code, or LSP diagnostics) that can get propagated to VSCode and CI (as well as just as a CLI tool, if that's your preference). Disclaimer: I work on trunk :)
xorcist 5y ago
Wouldn't quick sanity checks like these make more sense in a git push hook? No reason to wait for the CI. There's also the risk of broken intermediary commits unless the CI check each and every commit in isolation.
- don-code 5y ago
  One of the benefits of CI is that I don't need to make assumptions about the environment I'm operating in - that is, I don't need to worry about a tool like `jq` being installed, whether I have GNU or BSD `date`, or the biggest one in this case, whether I've even installed a commit hook after cloning the repo. It only takes one new hire that hasn't set up the hooks to merge a change that makes the repo fail checks.
  - xorcist 5y ago
    A post-receive hook runs on your git server, in an environment just as known to you. It denies all pushes which doesn't meet the specified requirements with an error message, and the end user has to re-do the commit and push again. This is likely how your authorization is done today already. The alternative would be a pre-commit hook, but that runs when crafting the commit, and under the control of the end user. That can make for a better user experience since it runs even earlier in the process but isn't necessarily secure. Of course, one can have both.
- stavros 5y ago
  Agreed. The usual way I do this is I set up a pre-commit (pre-commit.com) configuration that runs locally, and then again on CI. That way, you get all the benefits on CI, but can optimize for speed by installing pre-commit locally (and run either the exact same or a subset of checks).
indigodaddy 5y ago
Learn something new everyday.. Never knew about —exclude/include, very cool.
chubot 5y ago
If you think of a CI configuration as a shell script, then this is normal and not surprising. A CI config is just a big job that invokes a bunch of tools like "go build" or "npm test", which is exactly what a shell script is too. I would get rid of the YAML and use shell for the whole thing :) Shell does have some problems, like needing to be more parallel, incremental, and reproducible, but the YAML-based CI systems all have those problems too. Related: http://www.oilshell.org/blog/2021/04/build-ci-comments.html#ci-services-as-a-bridge-between-shell-and-distributed-systems http://www.oilshell.org/blog/2021/04/build-ci-comments.html#... (and the entire post)
ilyash 5y ago
Interesting to see how nobody cares about exit code 2. Modify your grep command to have syntax error and you will always think you have all the translations. Edit: ah.. forgot the whole point.. In Next Generation Shell (author here) this doesn't happen: ngs -e '$(grep ...).not()' does right thing. grep exit code 0 becomes 1, 1 becomes 0, 2 becomes 240 (exception)

111 points by fphilipe 5y ago

Don't you worry about what happens when the translation software produces a semantically equivalent empty string, or something? Like `<string><![CDATA[]]></string>`. An XML parser will find that to be empty, grep will be like "ooh lots of text in there".

rtuin 5y ago

Oh the power of Linux. I’m also using grep in CI as a simple but effectieve check on dependency violations in some codebases.

gravypod 5y ago

I think this is a great example of where build systems, that understand the need for testing, can help out. In a build system I use quite often (Bazel) you can express this as an `sh_test()` [0] which provides documentation about what you're attempting to do and provides you with a way to reproduce the failure locally. You don't have to push the code and wait for CI to fail to find out, or debug, this error. Extra fun thing to do: print a message describing why the decision was made and how to resolve the error in the failure message of your test! [0] - https://docs.bazel.build/versions/main/be/shell.html#sh_test https://docs.bazel.build/versions/main/be/shell.html#sh_test

joatmon-snoo 5y ago
Tests are a good way to assert an invariant that you expect of your codebase, but as with all things, resolving the error can get a bit tricky/frustrating. The canonical example in my mind is any kind of autofix-able linter, where there's some kind of patch (or more nuanced autofix) that the linter can generate on-the-spot for you. With a sh_test construct (or any other test), you generally find yourself printing out some command that the user can run to fix things, which in a sufficiently large codebase can get really frustrating. (The worst offenders of this that I remember dealing with at Google were when generated code/artifacts had to be checked into the repo instead of getting being wired into the build system because <insert hermeticity problem> and a large-scale change you were doing looked at one of them the wrong way...) (My company - https://trunk.io https://trunk.io - is actually building a universal linter as part of our product offering, and we already have a system to write custom linters with varying levels of sophistication that can plug into both your IDE and CI system!)
- vlovich123 5y ago
  The problem with automatic linting is that there’s no good answer. Either you just fail and ask the user to resubmit or you fix up the formatting on their behalf. If you do the latter, you’re likely also pushing to the review system meaning the code under review and your local version no longer match which causes problems when rebasing and whatnot. The secondary problem is that the lint tool chain running locally sometimes doesn’t version match what’s in CI which poses a set of problems too, although in practice I think that’s raised as a concern only by teams that don’t establish good hygiene of checking in the set of tools (maintenance of multiple platforms is real there though). I’ve yet to see a good answer to this problem though as each approach has upsides/downsides and no one ever ends up happy in a large enough team. Good luck.
  - joatmon-snoo 5y ago
    Thanks for the feedback! All of those are very real problems and we've spent a lot of time thinking about how to handle them: > Either you just fail and ask the user to resubmit or you fix up the formatting on their behalf. We only apply autofixes when running on the user's machine; when running on CI, as you point out, we only show them because of Git's decentralized nature here. > The secondary problem is that the lint tool chain running locally sometimes doesn’t version match what’s in CI which poses a set of problems too, although in practice I think that’s raised as a concern only by teams that don’t establish good hygiene of checking in the set of tools (maintenance of multiple platforms is real there though). We've built out our own system for managing (mostly) hermetic toolchain installs, and require users to version their linters in their trunk.yaml configuration right now. (There are some open questions here around how we handle version skew between our toolchain version and the system-installed toolchain version.) We're also well aware of the problems that emerge as you scale to a larger team; hierarchical configs are one of the things that we've discussed but are waiting until we have more traction to flesh out. (All of us working on the linter side have experience setting tech direction for orgs of 100+ engineers).
- gravypod 5y ago
  > (The worst offenders of this that I remember dealing with at Google were when generated code/artifacts had to be checked into the repo instead of getting being wired into the build system because <insert hermeticity problem> and a large-scale change you were doing looked at one of them the wrong way...) Been there. It's unfortunate because there are `genrule()`s for this very reason. But, definitely. Linting is very important and trunk looks great! But linting and config validation are very different use cases. At a previous company I had test that loaded a set of configs from repo and made sure all of them were valid with app level logic. It was crude but helpful. Trunk would be ideal. Also, question, have you thought of writing your own linter+autoformatter with treesitter? Are you planning to expand into other tooling?
hiptobecubic 5y ago
I was really annoyed when I first started using bazel, but it really is just an excellent build system. All the things I thought I didn't like about it, I have come to like.

eminence32 5y ago

The semgrep[1] tool seems like the logical next step, for when you've outgrown plain ol' grep. [1] https://semgrep.dev/ https://semgrep.dev/

jeffbee 5y ago

grep and other shell tool one-liners also make excellent kubernetes liveness checks. I have some kubernetes daemonsets that don't do anything other than assert that things on the node are as they should, via grep exit status.

nickjj 5y ago
It's also useful for alternative health check commands depending on which environment you're in. For example you could configure a Docker Compose health check to curl an endpoint for a 200 in production but in development override the health check command to call /bin/true so you don't get log spam in development from your health check while ensuring the health check still passes since /bin/true is an extremely minimal command that returns an exit code of 0. This can be controlled by a single environment variable too, no changes needed to your docker-compose.yml file. I covered this pattern in a recent DockerCon talk at https://nickjanetakis.com/blog/best-practices-around-production-ready-web-apps-with-docker-compose https://nickjanetakis.com/blog/best-practices-around-product....

david_allison 5y ago

Android's default linting already contains a "missing translation" lint rule which you can activate: "MissingTranslation"[0] For Android specifically: Gradle and Android Studio both support a powerful linting framework (to the level that it can provide auto-fixes to the IDE). It's better to provide an in-editor to guide your contributors before it hits CI, then have CI nag if they didn't fix the in-editor warnings/errors: Some examples of custom lint rules[1] and the default rules which Android Studio runs[2]: [0] https://android.googlesource.com/platform/tools/base/+/32923407563b7e2c943cf372e3cd9d4398d09789/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/TranslationDetector.java#79 https://android.googlesource.com/platform/tools/base/+/32923... [1] https://github.com/ankidroid/Anki-Android/tree/master/lint-rules/src/main/java/com/ichi2/anki/lint/rules https://github.com/ankidroid/Anki-Android/tree/master/lint-r... [2] https://github.com/ankidroid/Anki-Android/blob/master/lint-release.xml https://github.com/ankidroid/Anki-Android/blob/master/lint-r...

excircul 5y ago

> I personally prefer ripgrep as it is much faster than grep, but usually that is not available on CI machines. I recommend git grep, which is comparable in speed to ripgrep, since it ignores non-tracked files and searches the object storage directly. It is also able to run in parallel.

cfors 5y ago

In today's world of excellent CLI tools I don't think grep is a good choice, especially for checking irregular languages like XML. [0] I use tools like `jq` [1] or `yq` [2] all the time for CI checks. One useful check, is we have a configuration file stored as several hundred lines of YAML. Its a nice thing to maintain a sorted order for that, so we have a git pre-commit hook that runs the following: > yq eval --inplace '.my_key|= sort' my_file.yaml Of course, a pre-commit hook or CI both work. There's pros and cons of both. For our team, the pre-commit hook is a low enough level of effort, and doesn't require a CI check for something that executes in milliseconds. [0] https://stackoverflow.com/a/1732454 https://stackoverflow.com/a/1732454 [1] https://github.com/stedolan/jq https://github.com/stedolan/jq [2] https://github.com/mikefarah/yq https://github.com/mikefarah/yq

zX41ZdbW 5y ago

Here is my favorite: https://github.com/ClickHouse/ClickHouse/blob/master/utils/check-style/check-style#L251 https://github.com/ClickHouse/ClickHouse/blob/master/utils/c... "Too many exclamation marks"

darknavi 5y ago
I wonder how they picked three as too many. Why not two? > Three shall be the number thou shalt count, and the number of the counting shall be three. Four shalt thou not count, neither count thou two, excepting that thou then proceed to three. Five is right out.
- irrational 5y ago
  This is the obvious answer.
- deleted 5y ago
  [deleted]
- chucksmash 5y ago
  `!!thing` is a fairly common JS idiom for "I have a truthy/falsy thing and I want true or false."
- rbonvall 5y ago
  Maybe because of the use of the "double bang" pattern to convert a value to boolean: !!41 === true !!0 === false
- accoil 5y ago
  Two really does seem too many. I worry about these times when any passing coder can ni-gate at will.
ljm 5y ago
I enjoy seeing a nice shell script. This one is easy to follow and is consistently formatted.

matmann2001 5y ago

Wouldn't this example do the incorrect thing in the event of a grep error (exit code 2)?

CGamesPlay 5y ago
Depends on what you mean by wrong. If the tests can’t run successfully because they detect an error or because they have an error, it still is a valuable signal to raise to the developer. Consider what would happen if your unit test framework couldn’t compile the test files and failed with an error. Isn’t that also a test failure?
- Normal_gaussian 5y ago
  What would happen is the test would pass, as the logical not of 2 is 0. To address this: many CI pipelines allow you to specify which exot codes to succeed and fail on per job, or if it doesn't the one liner will need some boolean logic to check the exit code.
tyingq 5y ago
I guess testing specifically for an exit code of 1 isn't as easy to remember. (grep -q notwanted /hay/stack ; test $? == 1) && echo "No violations found" But you're right...the simpler format would do the wrong thing if, for example, the file didn't exist or had bad permissions or was a directory instead of a file.

joatmon-snoo 5y ago

Shameless plug: if you find yourself wanting more advamced custom painting, you can try http://trunk.io http://trunk.io, which provides you with three different ways to write your own linters (pass/fail based on your script's exit code, spit out a patch that should be applied to your code, or LSP diagnostics) that can get propagated to VSCode and CI (as well as just as a CLI tool, if that's your preference). Disclaimer: I work on trunk :)

xorcist 5y ago

Wouldn't quick sanity checks like these make more sense in a git push hook? No reason to wait for the CI. There's also the risk of broken intermediary commits unless the CI check each and every commit in isolation.

don-code 5y ago
One of the benefits of CI is that I don't need to make assumptions about the environment I'm operating in - that is, I don't need to worry about a tool like `jq` being installed, whether I have GNU or BSD `date`, or the biggest one in this case, whether I've even installed a commit hook after cloning the repo. It only takes one new hire that hasn't set up the hooks to merge a change that makes the repo fail checks.
- xorcist 5y ago
  A post-receive hook runs on your git server, in an environment just as known to you. It denies all pushes which doesn't meet the specified requirements with an error message, and the end user has to re-do the commit and push again. This is likely how your authorization is done today already. The alternative would be a pre-commit hook, but that runs when crafting the commit, and under the control of the end user. That can make for a better user experience since it runs even earlier in the process but isn't necessarily secure. Of course, one can have both.
stavros 5y ago
Agreed. The usual way I do this is I set up a pre-commit (pre-commit.com) configuration that runs locally, and then again on CI. That way, you get all the benefits on CI, but can optimize for speed by installing pre-commit locally (and run either the exact same or a subset of checks).

indigodaddy 5y ago

Learn something new everyday.. Never knew about —exclude/include, very cool.

chubot 5y ago

If you think of a CI configuration as a shell script, then this is normal and not surprising. A CI config is just a big job that invokes a bunch of tools like "go build" or "npm test", which is exactly what a shell script is too. I would get rid of the YAML and use shell for the whole thing :) Shell does have some problems, like needing to be more parallel, incremental, and reproducible, but the YAML-based CI systems all have those problems too. Related: http://www.oilshell.org/blog/2021/04/build-ci-comments.html#ci-services-as-a-bridge-between-shell-and-distributed-systems http://www.oilshell.org/blog/2021/04/build-ci-comments.html#... (and the entire post)

ilyash 5y ago

Interesting to see how nobody cares about exit code 2. Modify your grep command to have syntax error and you will always think you have all the translations. Edit: ah.. forgot the whole point.. In Next Generation Shell (author here) this doesn't happen: ngs -e '$(grep ...).not()' does right thing. grep exit code 0 becomes 1, 1 becomes 0, 2 becomes 240 (exception)