[SERVER-66580] ninja build output uses present active tense but its should be past tense Created: 19/May/22  Updated: 29/Oct/23  Resolved: 26/May/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.1.0-rc0

Type: Bug Priority: Major - P3
Reporter: Daniel Moody Assignee: Alex Neben
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Dev Platform 2022-05-30
Participants:

 Description   

Each time ninja prints a job description to the command line, it's printing the job which has most recently completed. Out ninja generation is configured to using active present tense in regards to what it's doing which is not really correct.

It should be updated in the ninja tool to use past tense.



 Comments   
Comment by Mathias Stearn [ 01/Jun/22 ]

> Now I see the wrong tense at the start of the build!

Also at the end of the build when it goes single-threaded to do the final link and install task. Eg, we will say we finished linking mongod while we are still linking it. One solution is just to avoid using tenses entirely. In the old module I just used the rule name and $out like description = CXX $out, which is neutral about whether it is currently happening or has completed. One exception was when checking error codes (which doesn't really have a meaningful $out) where I had it say explicitly "Checking error codes and waiting for next compile to finish" because it ran very quickly but was often on the screen for a long time so it got blamed for slow build times. If you don't like including the rule name, you could also use the imparitive mood (eg "Compile foo.o") which is also neutral on present vs past tense.

> Thats annoying, even the maintainer thinks so: https://github.com/ninja-build/ninja/pull/999

Assuming you are referring to Nico's last comment, I read it the opposite way. I think he wishes that with -v it would behave like a dumb terminal and print both at start and finish rather than only when printing at the end (ie do the normal behavior but with \n instead of \r before each line). FWIW, I mostly think it has the right behavior for smart terminals (although it perhaps would be nice to have a different description var for start vs finish), and I've used the only-at-end behavior when writing to a file with -j1 and a custom NINJA_STATUS in order to get independent times for each build action.

PS- I almost forgot about the special case. Anything that uses the magic pool = console only prints its description at the beginning so that the description is before the output. We should definintely make sure not to use past tense for those tasks.

Comment by Alex Neben [ 31/May/22 ]

Good catch. I still think this change makes the ninja output "more correct". An average dev will run with >200 tasks which means for any long running task it is reasonable to expect that another task will finish before the long task does. We want to avoid misleading a dev who might attribute a long running task to something that is actually very short.

Comment by Mathias Stearn [ 30/May/22 ]

Unfortunately it is more complicated than this ticket implies. By default, if you just run ninja targets it prints the description both when it starts and when it finishes a task. You can see that by running ninja -j4 with the following build.ninja:

rule sleep
    command = sleep $out
rule echo
    command = echo $out
 
build 1 : sleep
build 2 : sleep
build 3 : sleep
build done : echo 1 2 3
 
default done

You will (likely) see sleep 3 printed immediately, followed by 1, 2, then 3 again breifly before done is printed. (The actual order that they will be printed initially depends on the value of pointers returned by malloc).

Now if you redirect stdout (ninja | cat) or pass -v to ninja, then it will only print tasks as they finish. But I don't think that is the most common way for developers to look at ninja output.

That said, I'm not opposed to this change in wording if you still think it is the right way to go, I just don't want the decision to be based on a false premise.

Comment by Githook User [ 26/May/22 ]

Author:

{'name': 'Alexander Neben', 'email': 'alexander.neben@mongodb.com', 'username': 'IamXander'}

Message: SERVER-66580 Fixed tense for ninja files
Branch: master
https://github.com/mongodb/mongo/commit/0cc2ede603fd954224356ea2bde2cad8bea65afa

Generated at Thu Feb 08 06:05:50 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.