Created attachment 10859 [details] Enhance log message by adding variable prefix, etc. CAVEAT: The uploaded patch was created during the work to support -gsplit-dwarf (FissionDwarf) of GCC. (See bug 10005 ) Some of the patches have interdependence and if earlier patches are not applied, the later ones may fail. I will re-create patches appropriately depending on how an earlier patch is incorporated (or dropped). The whole list of broken patches below were uploaded in bug 10005 for convenience, but now separate bugzilla is filed to follow up on each of it (except for the main -gsplit-dwarf support, in bug 10005). 0001-Harden-logging-perror-replaced-with-cc_log-and-fatal.patch 0002-hash-file-creation-failure-is-recorded-now.patch 0003-Use-x_unlink-instead-of-unlink-per-HACKING.txt.patch 0004-make-sure-the-result-of-stat-call-is-checked.patch *0005-Enhance-log-message-add-prefix-var.-name-and-a-few-a.patch 0006-warning-about-missing-x_unlinks-when-get_file_from_c.patch 0007-Support-for-gsplit-dwarf.patch 0008-Missing-copy-of-diagnostics-file-in-case-of-failure.patch What this entry is about: Short explanation. 5: 0005-Enhance-log-message-add-prefix-var.-name-and-a-few-a.patch Enhance log message: add prefix (variable name appropriately) and a few additions of cc_log() calls.. These enhanced log messages were crucial in developing and debugging the support for -gsplit-dwarf. Also, I could find the now fixed extra unnecessary copy between the cached and real object files by looking at the enhanced log messages. So I think this change is very important for enhancing ccache in the future or when subtle bugs may creep in. This is actually not related to -gsplit-dwarf itself, and so I am filing this separate bugzilla. Since this changes ccache.c, it may interfere with the later patch application if this is not applied to ccache repository. (see bug 10005) TIA
Not sure about this patch. I'm all for improved logging, but I don't really see why adding variable names to the log messages would make it easier to debug - don't the file names already contain enough information? For instance, cached_stderr has a ".stderr" suffix. (As mentioned in bug 11154, my view is that the reader of the ccache logfile is not expected to know about ccache's internal function/variable names.) I also don't quite see the point of adding log messages after calls to get_file_from_cache and put_file_in_cache since those functions already log what they have done. But I'm probably missing some aspect since you apparently feel that you were helped by the changes in the patch. :-) Perhaps you could describe in what way you were helped by the variable name prefixes? In my mind, a developer can easily map the essentially unique error messages to code location already without the mentioning of internal variable names in the log file.
(In reply to Joel Rosdahl from comment #1) Thank you for your comment. Re: > I also don't quite see the point of adding log messages after calls to > get_file_from_cache and put_file_in_cache since those functions already log > what they have done. One of the log message can go. Yes, there is a duplication. That said, my point is the necessity for a new developer who needs hack on the existing ccache code base as opposed to the ordinary user of ccache as you correctly pointed out: that the reader of the ccache logfile is not expected to know about ccache's internal function/variable names.". When a new developer comes along and tries to figure out what ccache is doing under its hood, prefixing the file name with the internal variable name was a huge plus (well, to be honest, the suffix doesn't mean much, or rather due to the longish filename after hash string part, following the suffix is not easy. I found the prefixing the output with the variable name was a huge plush: I mean that the developer does not need to grok the detailed hash string to understand the logical operation and when something goes wrong, only then can take a look at the hashed file name very carefully. The variable name prefix was really helpful (WITHOUT the need to scan past the lengthy file name to check for the suffix.) Maybe we can change the output based on a flag: a bit of an ugly solution, but to new unaided eyes the variable prefix was a great help. TIA
(In reply to Joel Rosdahl from comment #1) Dear Joel, I think hard about this patch, and decided that this log message patch can wait (wait forever if necessary). Instead, I would like to have the main patch to support "-gsplit-dwarf". So I refreshed the patch there. Please take a look there. I will wait for the merge of main patch iin 10005, which should be easier now that "profiling" support is in the repository since the handling of cov file is similar in nature to the handling of dwo file. So the patch for -gsplit-dwarf support now seems more familiar than before. I can resurrect this patch to see if I can improve the log messages for easier debugging when someone attempts major surgery like the addition of FissionDwarf or profiling support. TIA
Closing this now since no progress has been made for a long time. Please create a new issue at https://github.com/ccache/ccache/issues if wanted.
Thank you again for sharing ccache with developers community. I will come back when I need to have a detailed log facility when I need to hack the internal of ccache again. (I just found out a few days ago that, despite my successful use of ccache locally on my PC, mozilla's infrastructure is now using a similar but different system called sccache, which still does not understand -gsplit-dwarf. sccache seems to be tailored to Amazon S3, i.e., cloud usage. Oh well. My motivation to use ccache and support -gsplit-dwarf was to compile mozilla thunderbird fast on my PC for debugging.) ccache running locally on my Debian GNU/linux is indispensable. Happy Hacking (in the good sense of the word). Thank you again.