tool-cache: make extract functions quiet by default and more verbose if core.isDebug is set

[aibaars]

This avoids spamming the log when unzipping large zip files.

[aibaars]

@damccorm According to git you are the original author, could you have a look at this PR? Thanks!

[damccorm]

@bryanmacfarlane could you take a look?

[bryanmacfarlane]

If we do this, it should be an option. That would allow action authors to default to quite but be verbose if debug is set.

[aibaars]

@bryanmacfarlane That makes sense. How would I check if "debug" is set?

[bryanmacfarlane]

I think the toolkit api should take an option structure and the calling action would check for debug and set that option. Covered here action author would check ACTIONS_STEP_DEBUG and then set the option. Too bad we didn't start with q being the default so now the api default needs to be the same (not q) but the consuming action can choose to by default send q and not do it if debug is set. Hope that all makes sense.

[aibaars]

@bryanmacfarlane The API currently does not have a consistent default; if I recall correctly it is quiet for windows but verbose for Linux/OSX . Would it make sense to capture the output of unzip and call core.debug for each line of unzip output. If I understand correctly this should suppress the output unless ACTIONS_STEP_DEBUG is set.

[bryanmacfarlane]

I was thinking more along the lines of checking if debug is set and then passing q so you don't have to trap and check each line. I'll follow up to see if there's a reliable way to do that (I want to avoid checking the secrets collection because there's work defined to have that be a settable admin or workflow setting).

[bryanmacfarlane]

actions/runner#252

[aibaars]

Thanks @bryanmacfarlane for opening a feature request to expose whether debugging is turned on. In the meantime, would it make sense to suppress all output for now? This would make the implementation of unzipping consistent across operating systems. Quiet output is probably the best default for the tool-cache library.

[aibaars]

@bryanmacfarlane I pushed a commit adding a verbose flag to the extractZip function. I am not sure this is the right thing to do as it won't have any effect on Windows and I don't see an easy way to make it work on Windows.

The other extraction functions extractTar and extract7z are currently quiet; no verbose output for tar and level 1 output for 7z. For consistency those functions should also get a verbose flag.

[aibaars]

@bryanmacfarlane After looking a bit more carefully at the current implementation of extract* I found that 7z is verbose, tar is quiet and zip is quiet on windows but verbose on OSX/Linux. This "perfect" 50/50 split makes it difficult to choose whether quiet or verbose should be the default. My preference would be to make the functions quiet to make sure logs are less bloated/easier to read through.

I think the extract* functions should have verbose output in debug mode, which should be easy to implement once actions/runner#252 is done, except for zip on Windows.

I am not really in favour of adding a configuration flag to the API. The functions are meant for extracting tool archives and I don't think the extra control (and complexity) is necessary.

[bryanmacfarlane]

Sounds good. We'll add that change to the runner and the code in the toolkit can just check the envvar.

[bryanmacfarlane]

@TingluoHuang for context

[aibaars]

Thanks to @TingluoHuang for #278 ; I rebased on his work. Now all extract functions should be quiet by default and more verbose when core.isDebug is set.

[bryanmacfarlane]

@TingluoHuang - I think this behavior is in the runner now??

[TingluoHuang]

@bryanmacfarlane it's in, we just need to finish the rollout.

[aibaars]

@bryanmacfarlane Thanks for merging #278 ! I rebased this PR, hopefully it is ready to go now.

[thboop]

Hey @aibaars , I'm hoping to get this merged in shortly. Do you have some time to take care of the merge conflicts? If not, I can take over the PR.

Thank you!

[aibaars]

Thanks! I just rebased the PR.

[aibaars]

@thboop I have no idea what the failures of the unit tests are about. They seem unrelated to the changes.

[aibaars]

@thboop The tests seem to pass now; I should have thought about re-running myself.

[thboop]

LGTM

[thboop]

Thanks for the contribution @aibaars !

[bryanmacfarlane]

@aibaars I can release a new version of the tool-cache if needed. Was it your action that needed it or any other particular setup-* action that motivated the change? I can prioritize that if so

[aibaars]

Thanks, the github/codeql-action team switched to using tar.gz a few days back to silence the chatty logging, but will probably switch back to zip once possible (unzipping was slightly faster). No need to rush anything though.

@Daverlo for info ^