Skip to content
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

builtin support for "run as CI in a worktree" #6

Open
peff opened this issue Jan 11, 2017 · 4 comments
Open

builtin support for "run as CI in a worktree" #6

peff opened this issue Jan 11, 2017 · 4 comments

Comments

@peff
Copy link
Contributor

peff commented Jan 11, 2017

It seems like the main utility of this over a rebase -x method (besides the result caching) is to run it asynchronously. I wonder how hard it would be to have a git test ci command that Just Works. I'm fooling with something like this in my Git scripts: Meta/ci.

It's mostly generic, but the config.mak setup is specific to the Git project. I suppose that step could be shoved into the test command.

I've also noticed that it gets annoying when you do have a failure, because it will play the sad trombone every 30 seconds. Perhaps it should just quit after a failure, but then that would force you to restart it manually. It might make sense for git test to give a different exit code for a cached failure, versus a case where it actually ran the tests and failed. I dunno.

@mhagger
Copy link
Owner

mhagger commented Jan 12, 2017

Cool, I like the idea (especially the 😢 sad trombone 🎺 part)!

Ideally, something like this would somehow be hooked into the post-commit hook (and possibly post-applypatch, post-receive, ... what's the full list that would be needed?) rather than polling every 30 s.

git test could certainly distinguish between new and old breakages. It's a bit awkward because it mostly tries to propagate the error code of the test command that failed [1]. But there could be an option like --cached-failure-retcode=<N> that changes cached failures into the specified value.

But really, the decision about whether to rerun the script "for real" is, to a good approximation, just

git log --format=%T $range | grep -q $previously_broken_tree

This could maybe be made into a git test command for convenience.

A third alternative would be to teach git test to output its test results, commit by commit, in machine-readable format with an option to output only cached results. The calling script could decide what to do based on that output. It's a little bit hard to get other information out of the script because it's pretty undisciplined about what it writes to stdout and stderr, let alone what the test scripts are allowed to write there. But the machine-readable output could, for example, be written to a user-supplied file descriptor, or we could force the other script output all to stderr and write the machine-readable output to stdout. The format might be one line per commit, say in the following format:

$commit_sha1 $tree_sha1 [good|bad|unknown]

It would be nice to have a command like this anyway, so maybe the easiest alternative would be to run git test run as usual, and if it fails run git test show-cached-results to find out why.

[1] Actually, it only does so for fresh failures. When failures are stored in notes, the error code is not recorded, so cached failures always result in retcode==1. I'd like to change that by storing something like bad:<retcode> in the notes and replaying the old value for cached failure.

@peff
Copy link
Contributor Author

peff commented Jan 14, 2017

Ideally, something like this would somehow be hooked into the post-commit hook (and possibly post-applypatch, post-receive, ... what's the full list that would be needed?) rather than polling every 30 s.

Yeah, the polling is obviously gross. If the idea is for this to be started and running all the time in the background (which is my general conception), then it really wants to respond anytime the tip of HEAD is updated (either a new commit, or moved to a new branch). Perhaps something like inotifywait could be used to look for updates to HEAD or to $(git symbolic-ref HEAD).

Technically that does not cover the case where @{upstream} is updated, but that's not really that interesting (generally it would move forward, so it would shrink the set of commits to test).

Regarding output of the cache, I would have thought that some variant of git log --format=%N would be helpful. But it's not, because the notes are on the trees, not the commits. And while you can easily get a list of the matching trees, there's no git notes command for bulk output of note content.

It seems like that's something that git could do better, and then you wouldn't need a special git test command.

@mhagger
Copy link
Owner

mhagger commented Jan 14, 2017

Perhaps something like inotifywait could be used to look for updates to HEAD or to $(git symbolic-ref HEAD).

I'd prefer something more portable.

Regarding output of the cache, I would have thought that some variant of git log --format=%N would be helpful. But it's not, because the notes are on the trees, not the commits. And while you can easily get a list of the matching trees, there's no git notes command for bulk output of note content.

Yes, a definite disadvantage of attaching notes to trees, not commits, is that the likes of git log can't report them, nor can tools built on the command line like gitk. It seems like this would be a useful feature to add on the Git side. It would also be nice if git notes supported batch queries.

For now I'm not too squeamish about running git notes once per commit. Anybody who has the bright idea to run git notes on all of the commits in their repository, all the time, can submit a PR to fix this :trollface:

Back to the topic of hook scripts, it would be unnecessarily awkward for git notes to link itself into githooks. This is a general problem. There is only one hooks/post-receive hook, for example, and it gets part of its input on stdin. So if there is already a script there, it is not feasible to add your own script.

I think Git should support hookdirs, like hooks/post-receive.d/, and would invoke each of the hook scripts in the directory in sorted order. That would make it much easier for diverse programs to add/remove hooks.

@peff
Copy link
Contributor Author

peff commented Jan 17, 2017

I'd prefer something more portable. [than inotify]

Fair enough. The nice thing about inotify (or similar systems) is that you don't have need support from git at all. The logic is all in the ci script.

I did add inotifywait support to my ci script (80c65b57542c88fdf5937426c41c75b6d405f534). It's quite convenient, at least when you are testing changes to the script and are making a bunch of little test commits. ;)

I think Git should support hookdirs, like hooks/post-receive.d/, and would invoke each of the hook scripts in the directory in sorted order. That would make it much easier for diverse programs to add/remove hooks.

Yeah, I think I agree. There are some policy questions to figure out (like how one hook's outcome affects whether we run the others). You can just have a master hook script that calls the others, and implements whatever policy you want there. But the input-handling does get nasty. It would be a lot more convenient if Git just handled the details.

hlovdal added a commit to hlovdal/git-test that referenced this issue Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants