On Nov 26, 2024, at 11:55 AM, David Malcolm <dmalc...@redhat.com> wrote: > > Ping
Ok. I'll punt on the diff documentation bits. > On Mon, 2024-11-18 at 16:22 -0500, David Malcolm wrote: >> Another ping for this patch; any pex experts out there? >> >> Thanks! >> Dave >> >> On Wed, 2024-05-29 at 17:06 -0400, David Malcolm wrote: >>> On Wed, 2024-05-29 at 16:35 -0400, Eric Gallager wrote: >>>> On Tue, May 28, 2024 at 1:21 PM David Malcolm >>>> <dmalc...@redhat.com> >>>> wrote: >>>>> >>>>> Ping. >>>>> >>>>> This patch has actually been *very* helpful to me when >>>>> debugging >>>>> selftest failures involving ASSERT_STREQ. >>>>> >>>>> Thanks >>>>> Dave >>>>> >>>> >>>> Currently `diff` is only listed under the "Tools/packages >>>> necessary >>>> for modifying GCC" section of install/prerequisites.html: >>>> https://gcc.gnu.org/install/prerequisites.html >>>> If it's going to become a dependency for actually running GCC, >>>> too, >>>> it >>>> should get moved to be documented elsewhere, IMO. >>> >>> All this is selftest code, and is turned off in a release >>> configuration >>> of GCC. The code path that invokes "diff" is when a selftest is >>> failing, which is immediately before a hard failure of the *build* >>> of >>> GCC. So arguably this is just a build-time thing for people >>> packaging/hacking on GCC, and thus not a new dependency for end- >>> usage. >>> >>> BTW I'm a bit hazy on the details of how "pex" is meant to work, so >>> hopefully someone more knowledgable than me can comment on that >>> aspect >>> of the patch. It seems to work though. >>> >>> Dave >>> >>>> >>>>> On Fri, 2024-05-17 at 15:51 -0400, David Malcolm wrote: >>>>>> Currently when ASSERT_STREQ or ASSERT_STREQ_AT fail we print >>>>>> both strings to stderr. However it can be hard to figure out >>>>>> the problem (e.g. for 1-character differences in long >>>>>> strings). >>>>>> >>>>>> Extend the output by writing out the strings to tempfiles and >>>>>> invoking "diff -up" on them when we have such a selftest >>>>>> failure, >>>>>> to (I hope) simplify debugging. >>>>>> >>>>>> Successfully bootstrapped & regrtested on x86_64-pc-linux- >>>>>> gnu. >>>>>> >>>>>> OK for trunk? >>>>>> >>>>>> gcc/ChangeLog: >>>>>> * selftest.cc (selftest::print_diff): New function. >>>>>> (selftest::assert_streq): Call it when we have non- >>>>>> equal >>>>>> non-null strings. >>>>>> >>>>>> Signed-off-by: David Malcolm <dmalc...@redhat.com> >>>>>> --- >>>>>> gcc/selftest.cc | 28 ++++++++++++++++++++++++++-- >>>>>> 1 file changed, 26 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/gcc/selftest.cc b/gcc/selftest.cc >>>>>> index 6438d86a6aa0..f58c0631908e 100644 >>>>>> --- a/gcc/selftest.cc >>>>>> +++ b/gcc/selftest.cc >>>>>> @@ -63,6 +63,26 @@ fail_formatted (const location &loc, const >>>>>> char >>>>>> *fmt, ...) >>>>>> abort (); >>>>>> } >>>>>> >>>>>> +/* Invoke "diff" to print the difference between VAL1 and >>>>>> VAL2 >>>>>> + on stdout. */ >>>>>> + >>>>>> +static void >>>>>> +print_diff (const location &loc, const char *val1, const >>>>>> char >>>>>> *val2) >>>>>> +{ >>>>>> + temp_source_file tmpfile1 (loc, ".txt", val1); >>>>>> + temp_source_file tmpfile2 (loc, ".txt", val2); >>>>>> + const char *args[] = {"diff", >>>>>> + "-up", >>>>>> + tmpfile1.get_filename (), >>>>>> + tmpfile2.get_filename (), >>>>>> + NULL}; >>>>>> + int exit_status = 0; >>>>>> + int err = 0; >>>>>> + pex_one (PEX_SEARCH | PEX_LAST, >>>>>> + args[0], CONST_CAST (char **, args), >>>>>> + NULL, NULL, NULL, &exit_status, &err); >>>>>> +} >>>>>> + >>>>>> /* Implementation detail of ASSERT_STREQ. >>>>>> Compare val1 and val2 with strcmp. They ought >>>>>> to be non-NULL; fail gracefully if either or both are >>>>>> NULL. >>>>>> */ >>>>>> @@ -89,8 +109,12 @@ assert_streq (const location &loc, >>>>>> if (strcmp (val1, val2) == 0) >>>>>> pass (loc, "ASSERT_STREQ"); >>>>>> else >>>>>> - fail_formatted (loc, "ASSERT_STREQ (%s, %s)\n >>>>>> val1=\"%s\"\n >>>>>> val2=\"%s\"\n", >>>>>> - desc_val1, desc_val2, val1, val2); >>>>>> + { >>>>>> + print_diff (loc, val1, val2); >>>>>> + fail_formatted >>>>>> + (loc, "ASSERT_STREQ (%s, %s)\n val1=\"%s\"\n >>>>>> val2=\"%s\"\n", >>>>>> + desc_val1, desc_val2, val1, val2); >>>>>> + } >>>>>> } >>>>>> }
