ExternalProject: Add option for quiet output

This option will pass the --quiet option to all git commands which support it and don't already have it defined in ExternalProject somewhere.

Topic-rename: ExternalProject-quiet
Fixes: #17302

Thanks for tackling this feature. Rather than being an option specific to git, this should really be implemented for all the download methods and the corresponding keyword should just be QUIET.

In my original comments in #17302 (comment 321718), rather than implementing this individually for each download method, I was thinking more along the lines of something similar to what the _ep_write_log_script() does. It wraps the command in a script so that it can capture the output to log files instead of showing the output. This has the advantage that it only needs to be implemented in one place and it applies to all download methods, including custom commands. A QUIET option could do something similar. It could capture to a variable instead of a log file, which is precisely what the FetchContent's QUIET option does. It can do that fairly easily because it invokes a separate sub-build to do things and that is a convenient place to capture output. For using ExternalProject directly, I don't know if that would be just as convenient. Looking at the implementation of _ep_write_log_script(), it is quiet complex. I don't know if we would need to go through similar complex logic just to capture to variables. If that looked overly complicated, we could potentially consider implementing QUIET by forcing logging to files and only show the content of those files if there was an error. I'd like to keep this as a fallback approach though and try to implement this without having to log to files first if we can.

We should also have tests to ensure the option works, but we can tackle that after we sort out the implementation.

I definitely read that original comment and when I decided to start messing with it that was the first approach I tried. I say tried but more like I got lost in the bowels of how execute_process() works. I'm a heavy ROS user and the stderr output search led me here first, so I figured there must be some unintentional message() calls somewhere throughout CMake that were not specifying the proper log level (therefore defaulting on NOTICE which would then output as stderr. Grepping through I could not find any but grepping for message\( was a lost cause. All that said I agree a ExternalProject-wide QUIET makes more sense than a git specific one, but this is what I had time to bang out on a Saturday. I can take a closer look at some point. Definitely no skin off my back if this isn't how you guys want it done.

Just a tad more to add here, I was looking at execute_process() because those calls are what's doing the git clone/checkout/etc and I don't see what is causing their output to go to stderr. The one that "spoils" the colcon output in ROS builds is the simple git clone and git checkout commands, neither of which print to stderr for me, even the "you're already on this branch" output, all return to stdout, not stderr. So I was trying to dig into execute_process() to see where that message() call is that's actually printing the stderr and just got overwhelmed looking for it. To me the issue is with a call like this:

 execute_process(
    COMMAND "@git_EXECUTABLE@" @git_options@
            clone @git_clone_options@ "@git_repository@" "@src_name@"
    WORKING_DIRECTORY "@work_dir@"
    RESULT_VARIABLE error_code
  )
...
if(error_code)
  message(FATAL_ERROR "Failed to clone repository: '@git_repository@'")
endif()

Which handles printing an error to the user, if the process returned one, which makes sense. So for the final output to have a stderr on a git clone which returns no error and prints nothing to stderr must be getting message(<wrong_log_level>) by someone in between?

Thanks for looking at this, and apologies for my delay in responding. After giving it a fair bit of thought, I do want to avoid a git-specific solution here. That would be a wart on the API that I am sure I would quickly regret. If we are going to implement quiet support for ExternalProject, it should apply to all download methods and use QUIET as its keyword. This would be predictable for users and also be consistent with what FetchContent already does with its QUIET keyword support too.

As for where the stderr output is coming from, that is indeed strange. I had a quick look now but couldn't see anything obvious. It would take a deeper investigation, which unfortunately I can't commit the time for at the moment.

execute_process by default passes through the stdout/stderr pipes from the calling process if no capture arguments are specified.

added workflow:wip label

changed title from Add GIT_QUIET option to ExternalProject. to ExternalProject: Add option for quiet output

changed the description

requested review from @craig.scott

changed milestone to %3.27.0

@cclaunch please rebase on upstream master for post-3.26 development, and set the versionadded field to 3.27.

Hi @brad.king , I had gathered from the comments that this wasn't the solution that was desired, so wasn't planning to continue development. Is this MR going to be accepted as is?

Oh, sorry. I was just posting this request to all open MRs not bound for 3.26.

I'll close this based on the above, thanks. Alternatives can be discussed back in the original issue.

removed milestone %3.27.0

closed