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

print "still waiting" for long running commands if we serialize on them #678

Closed
wants to merge 1 commit into from

Conversation

sgraham
Copy link
Contributor

@sgraham sgraham commented Oct 23, 2013

Heuristic: if the currently-running count drops to 1, and that process has been running for 3s+, then print-and-shame.

It won't work if soon-to-be-long-running process just started with at least one other process, and then before 3s has elapsed, all the other processes terminate.

Similar to #111 but I didn't see a simple way of doing exactly that (because if nothing's happening I think we're blocked waiting for processes?). I might be misreading though.

@buildhive
Copy link

Evan Martin » ninja #637 SUCCESS
This pull request looks good
(what's this?)

@nico
Copy link
Collaborator

nico commented Oct 24, 2013

Maybe reverting 555431a (and fixing the bug in the old code) is enough? Currently finishing commands don't print anything either, and that might be sufficient?

@sgraham
Copy link
Contributor Author

sgraham commented Oct 24, 2013

555431a is just trying to print when the edge is done, right? (and only on dumb terminals)

I wanted to know what's happening "right now" when there's only one process running though. Did you mean printing something other than the edge that just finished, when it finishes?

@nico
Copy link
Collaborator

nico commented Oct 24, 2013

Right, it prints when the edge is done. On dumb displays, edges are currently only printed on start, so if you start 10 slow tasks you will get 10 lines of output immediately and then nothing until all 10 are completed currently (assuming further progress is blocked on these tasks). Tasks in pools aren't even started until their pools allows them too though, so maybe that doesn't help for windows linking.

@sgraham
Copy link
Contributor Author

sgraham commented Nov 19, 2013

I've been using this for about a month locally and prefer it. Is there a better approach? 555431a only was for dumb terminals which I don't care about much, but I can try to make dumb work too if you'd like.

@maximuska
Copy link
Contributor

I subjectively prefer (#629), which our team use is since I published it (~4months?), and people like it. But its main advantage is not the 'still running' message, rather the fact that the output is printed for solo tasks instantly, which is pretty much required to run unit-tests from the ninja. But yes, the patch is a bit complex and doesn't support windows (not that the later is not solvable).

Talking about a simplest approach for smart terminals I would just always print the name of one of the 'still running processes' from BuildEdgeFinished(). This gives the semantics of 'the task displayed is still running', which is the sufficient minimum?

@ilor
Copy link
Contributor

ilor commented Jun 20, 2014

I often have long-running jobs that block the entire build and sometimes have to resort to ps to figure out what's still running. Having a variant of this merged would be really nice.

@nico
Copy link
Collaborator

nico commented Nov 11, 2015

Let's close this as a dupe of #629.

@nico nico closed this Nov 11, 2015
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

Successfully merging this pull request may close these issues.

5 participants