coreutils
coreutils copied to clipboard
simple custom log macro to avoid additional dependencies
draft 3 for #6284, alternative for #6313 and #6314
The main idea here is that we have a own log macro to avoid additional dependencies.
Same as with #6314, the idea is that the logs are not part of the stable code-base. They would always be removed when debuggin is done. This way, there is no explicit activation mechanism needed and the test-framework can always specify the logfile-path via environmental variable. It will check after execution if the logfile was written to and then dumps the content.
GNU testsuite comparison:
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
GNU test failed: tests/du/bind-mount-dir-cycle. tests/du/bind-mount-dir-cycle is passing on 'main'. Maybe you have to rebase?
I still don't quite see the advantage of this. So you're suggesting that we change:
old strategy: just insert eprintln!() temporarily whenever you debug something, and delete it before pushing (and if you forget, CI will remind you)
new strategy: just insert uu_log!() temporarily whenever you debug something, and delete it before pushing (and if you forget, a weird environment variable will cause file writes in production that GNU doesn't do)
So what is the advantage then?
EDIT: Ah, I was hoping this would reset the "changes requested" status. I'm neutral on this PR, and don't understand how to tell github about it.
Thanks for neutral rating. I added now a feature toggle for the uu_log!(). Let me help you to see the benefit:
I'd formulate it like this:
- old strategy: Insert eprintln!(), disable the stderr assert of the test and add an additional statement into the test that enusres the stderr is logged before the failing assert. All this needs to be reverted before pushing, and if you forget, CI is not telling you either.
- new strategy: Insert uu_log!() and activate the "uu_log" feature in "src/uucore/Cargo.toml". No test-code needs to be modied. Testframework will automatically ensure that the content of the logfile is printed when the test is executed. Before you push, you remove the "uu_log" feature from "src/uucore/Cargo.toml" and you can optionally revert the uu_log!() calls. If you forget, human reviewer would easily spot the change in "src/uucore/Cargo.toml" in more than 90% of the cases. And if this is not yet enough, one could introduce a seperate test in the CI that is red if this feature is accidentially left over.
@sylvestre Can you please review this proposal. I think complexity-wise you probably would rather agree here.
GNU testsuite comparison:
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
I am sorry but I still don't see the benefit of logging in general ?!
GNU testsuite comparison:
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
There are no remaining negative side effects left that I know of. I explained the benefits. These are convencience reasons only and are not mandatory, of course. But I'm spending the efforts on this topic for a good reason. I think it would significantly improve the debugging experience for the topics I'm looking at. I'm sure that some more people like me will find it usefull.
Sure but you are repeating the what when I care about the why?
GNU testsuite comparison:
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Sure but you are repeating the what when I care about the why?
I apprechiate your quick response. The issue that I have is, that some people apparently argue about the disadvantages of the "what" before they actually undestood or agreed on the "why". From my perspective the "why" is already explained in the issue #6284 and I repeated it in the message above (https://github.com/uutils/coreutils/pull/6315#issuecomment-2094202212). I know that these are no super strong reasons, but I was expecting that the "why" is clear in general. At least I do not know what I could add.