git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 00/21] C99: show meaningful <file>:<line> in trace2 via macros

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 16, 2021, 20:16 UTC
Message-ID
<211116.868rxnq2dy.gmgdl@evledraar.gmail.com>
In-Reply-To
<YZQITrlvO7eUZyDN@nand.local>
On Tue, Nov 16 2021, Taylor Blau wrote:
Show 46 quoted lines
> On Tue, Nov 16, 2021 at 07:58:01PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, Nov 16 2021, Taylor Blau wrote:
>> >> At the end of this series we expose a config variable to have
>> >> usage/die/warning emit line numbers. I.e. going from:
>> >>
>> >>     $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
>> >>     fatal: bad boolean config value 'y' for 'core.x'
>> >>
>> >> To:
>> >>
>> >>     $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
>> >>     fatal: config.c:1241: bad boolean config value 'y' for 'core.x'
>> >
>> > Just picking on this output change in particular. I agree that this is
>> > easier for folks hacking on Git to trace down errors. But I'm not sure
>> > that I could say then same about users, who will likely treat this extra
>> > output as noise.
>> >
>> > Now we may find it helpful if they include it in a bug report, but I
>> > feel reasonably comfortable saying that the value there is pretty
>> > marginal. I don't find it all that problematic to grep for a specific
>> > error string, and usually find myself in the right place.
>>
>> I wouldn't suggest exposing this to users, except perhaps as part of
>> some "how to submit a bugreport" instructions. It's thoroughly optional.
>>
>> I thought it was easy enough to do with the preceding steps since all
>> the data is there, and would help my workflow a lot.
>>
>> If you've got the file/line number like that you can make it clickable
>> in your terminal/compile mode, e.g. Emacs's M-x compile. Saves time over
>> having to grep manually select the string, grep for it etc.
>>
>> Anyway, I can certainly live with peeling this patch off the end and
>> just stopping at the trace2 data for now, if you/others feel strongly
>> about it.
>
> I don't feel strongly, and I was just noting that it seemed like users
> would treat this extra information more often as noise than anything
> else.
>
> When you talk about making it optional, do you mean through
> configuration / an environment variable, or by including / not including
> the patch? In other words, the latter seems much more like us making a
> decision on whether or not to include line numbers rather than
> presenting a new option to users, though I may be misunderstanding.

Not surprising, since I see that I screwed up the summary in both the CL and 21/21. I.e. both of these are =false (I copy/pasted the error around, but didn't adjust the command that was invoked):

    $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
    fatal: bad boolean config value 'y' for 'core.x'
    $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
    fatal: config.c:1241: bad boolean config value 'y' for 'core.x'

That second one should be core.usageAddSource=true, i.e. as seen in the 21/21 implentation and core.txt docs you only get these line numebers if you opt-in to them via configuration or the new GIT_TEST_* environment variable.

Show 13 quoted lines
>> As noted in 02/21 we're hard depending on this particular C99 feature
>> already fon a few releases now, the only change on that front in this
>> series is to stop committing to maintaining the non-C99 codepaths.
>
>> We've already had hard dependencies on various bits of C99 for years now
>> without any trouble, and I wouldn't expect any problems on this front
>> either.
>
> Interesting; so this and others are likely part of MSVC's kind-of
> support for C99 features? In other words, that MSVC supports some
> features from C99 (and we are depending on a subset of those) but not
> all features so that it could reasonably be called a spec-compliant
> compiler for the C99 standard? If so, makes sense.

Yes, I think this is one of the things that's the same or similar enough to C++ that MSVC has good support for it. See the "We try to support a wide range of C compilers[...]" section in the CodingGuidelines for a list of some other C99 features we've used for years already.

Previous: Taylor Blau
Message 34 of 34 in “C99: show meaningful <file>:<line> in trace2 via macros”
  1. 00/21 C99: show meaningful <file>:<line> in trace2 via macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  2. 01/21 git-compat-util.h: clarify GCC v.s. C99-specific in commentÆvar Arnfjörð Bjarmason, Nov 15, 2021
  3. 03/21 usage.c: add a die_message() routineÆvar Arnfjörð Bjarmason, Nov 15, 2021
  4. 02/21 C99 support: hard-depend on C99 variadic macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  5. 04/21 usage.c API users: use die_message() where appropriateÆvar Arnfjörð Bjarmason, Nov 15, 2021
  6. 05/21 usage.c + gc: add and use a die_message_errno()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  7. 07/21 common-main.c: call exit(), don't returnÆvar Arnfjörð Bjarmason, Nov 15, 2021
  8. 06/21 config API: don't use vreportf(), make it static in usage.cÆvar Arnfjörð Bjarmason, Nov 15, 2021
  9. 08/21 usage.c: add a non-fatal bug() function to go with BUG()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  10. 10/21 receive-pack: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  11. 11/21 cache-tree.c: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  12. 12/21 pack-objects: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  13. 17/21 usage.c: move usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  14. 21/21 usage API: add "core.usageAddSource" config to add <file>:<line>Ævar Arnfjörð Bjarmason, Nov 15, 2021
  15. 20/21 usage API: make the "{usage,fatal,error,warning,BUG}: " translatableÆvar Arnfjörð Bjarmason, Nov 15, 2021
  16. 19/21 usage API: use C99 macros for {usage,usagef,die,error,warning,die}*()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  17. Jeff HostetlerDec 27, 2021
  18. Ævar Arnfjörð BjarmasonDec 27, 2021
  19. Jeff HostetlerDec 28, 2021
  20. Elijah NewrenDec 28, 2021
  21. Ævar Arnfjörð BjarmasonDec 28, 2021
  22. Elijah NewrenDec 29, 2021
  23. Ævar Arnfjörð BjarmasonDec 28, 2021
  24. Jeff HostetlerDec 29, 2021
  25. 18/21 usage.c: move rename variables in usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  26. 16/21 usage.[ch] API: rename "warn" vars functions to "warning"Ævar Arnfjörð Bjarmason, Nov 15, 2021
  27. 14/21 usage API: create a new usage.h, move API docs thereÆvar Arnfjörð Bjarmason, Nov 15, 2021
  28. 15/21 usage.[ch] API users: use report_fn, not hardcoded prototypeÆvar Arnfjörð Bjarmason, Nov 15, 2021
  29. 13/21 strbuf.h: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  30. 09/21 parse-options.[ch] API: use bug() to improve error outputÆvar Arnfjörð Bjarmason, Nov 15, 2021
  31. Taylor BlauNov 16, 2021
  32. Ævar Arnfjörð BjarmasonNov 16, 2021
  33. Taylor BlauNov 16, 2021
  34. Ævar Arnfjörð BjarmasonNov 16, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.