Change Makefile to handle reeeeeally long lists of files

[blag]

Only the files that have changed are validated for PR CI runs, but for the weekly CI runs all files are checked.

The AWS pack has a lot of actions. Like a lot a lot of actions. So when, during the weekly CI run, the Makefile gathered a list of all of the YAML files and then tried to evaluate that in a Bash if [ ! $(CHANGED_YAML) ] statement, the number of arguments exceeded Bash's maximum number of arguments and the CI job failed (I set FORCE_CHECK_ALL_FILES to true so it would behave like the weekly CI runs).

Due to that, I had to change up how all of that was processed. It doesn't really make sense to me to have the FORCE_CHECK_ALL_FILES environment variable option in all of the git-changes sub-scripts, since that option was only used in the Makefile and when that was specified the returned list of files had nothing to do with git at all. So I moved that logic into the Makefile and had it execute commands for each file instead of trying to interpolate the huge list into a string and then re-interpreting that string into a list of arguments to a Bash for command.

All of that made some lines in the Makefile really long, so then I also reformatted some of the recipes using backticks. Now it looks like a spilled box of toothpicks (eg: there are backticks ending a lot of lines), but there's a lot less horizontal scrolling required now and it kinda sorta reads like the Bash script that it is.

I hope/plan on rewriting the Makefile to use Python's excellent invoke library soon, so hopefully this problem will be avoided entirely in the future.

[arm4b]

👍 assuming all the testing with CI changes were done end-to-end/verified.

Looks like AWS pack made our life harder once again

[blag]

Eh, I think the blame here lies with Bash, or at least the way we used it. I love Bash, but it really is terrible sometimes. From a Bash user perspective, if a command accepts filenames as arguments, and it works for 10 filenames/arguments, then why shouldn't it work for 10 thousand filenames/arguments? And filename globbing and passing the list of filenames to a script is an extremely common (and apparently subtly limited/brittle) paradigm in Bash.

Even though none of the Bash scripts changed, Bash still broke because the environment changed. It just goes to show how brittle Bash actually is compared to more elegant solutions. The fact that the find command has had an -exec flag since forever is really a testament to how common this problem is, and how limited/hacky Bash is internally.

And I love Bash to death. I've written thousands of lines of Bash in my life. I've even written an (extremely simple) package manager for Bash profiles, in Bash itself. I still consider Bash to be terrible.

But once we/I convert to invoke, this shouldn't be an issue, since Python (and by extension, everything written in it) is perfect. 😆

Figure 1: A visual representation of Python and the Python ecosystem.

[arm4b]

From the build failure make: execvp: /bin/sh: Argument list too long
It's not the bash itself, it's ARG_MAX Linux kernel-level limits.
See https://www.in-ulm.de/~mascheck/various/argmax/

getconf ARG_MAX

Someone from CircleCI changed security OS limits and we suddenly hit this CI failure when number of possible arguments program/cli can accept exceeded the allowed bar.

---

I don't think replacing Makefile/bash with something else will make life much easier, it'll just trade one set of issues for another.

[Kami]

We could recompile kernel with a larger value for ARG_MAX

---

On a more serious note - I'm personally not a fan of (convoluted) Makefile.

It's similar to the StackStorm coverage Makefile changes which made Makefile so convoluted so it's very hard to understand what is going on and make any changes to it (I gave up last then when I wanted to improve it).

I think it's more more clear and easier to understand by having that logic in a separate bash script or similar.

And part of the reason that logic was in those git-changes script was so it can potentially be re-used by other pack developers for their pack CI.

I'm not sure if anyone is using it though. @nmaludy are you usig git-changes script for your pack CI?

[blag]

Regarding modifying the git-changes scripts, I think it was a poor decision to put FORCE_CHECK_ALL_FILES in them to begin with, since it combined two different concerns into one script: files that changed in git, and all files that matched a pattern when weekly CI was being run. Separate those concerns into their appropriate places.

A more concrete reason to remove the FORCE_CHECK_ALL_FILES check is that there is no way to check the results of the script when it returns too many files. Whatever way you try, Bash/the kernel has to process wayyy too many arguments to one process, which causes the error seen in the AWS pack. We need to avoid returning all of the files in the first place, and instead run our checks on each file as we find it, and the only place we can do that (as far as I know) is the Makefile.

@Kami Check out my work in StackStorm/st2#4782 - it's not done, and I'm still figuring out how to (re-) implement a few tasks, but it's a good first stab at breaking up a convoluted Makefile into the individual tasks/modules.

A major draw of using Invoke for me is the ability to pass options to individual tasks. This is exceptionally useful for running tests with and without coverage enabled, because instead of being a completely separate make target, it's the same Invoke task, you just pass it a flag to enable coverage.

Make is great when you are building artifacts that you want to cache. But for any interpreted language, or as a generic task runner, it is absolutely awful. It's the wrong tool for that job, and we shouldn't use make to begin with if we're just going to mark every single target as .PHONY, like we do in this repo.