From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8234 invoked by alias); 9 Mar 2014 23:16:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 8220 invoked by uid 89); 9 Mar 2014 23:16:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS,URIBL_BLACK autolearn=no version=3.3.2 X-HELO: mail-yh0-f52.google.com Received: from mail-yh0-f52.google.com (HELO mail-yh0-f52.google.com) (209.85.213.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sun, 09 Mar 2014 23:16:14 +0000 Received: by mail-yh0-f52.google.com with SMTP id a41so6315304yho.25 for ; Sun, 09 Mar 2014 16:16:12 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.236.101.227 with SMTP id b63mr39320107yhg.37.1394406972306; Sun, 09 Mar 2014 16:16:12 -0700 (PDT) Received: by 10.170.214.130 with HTTP; Sun, 9 Mar 2014 16:16:12 -0700 (PDT) In-Reply-To: <531A110B.4060502@redhat.com> References: <531A110B.4060502@redhat.com> Date: Sun, 09 Mar 2014 23:16:00 -0000 Message-ID: Subject: Re: target-delegates.c needs some TLC [was Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN] From: Doug Evans To: Pedro Alves Cc: Yao Qi , Tom Tromey , Hui Zhu , gdb-patches ml Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2014-03/txt/msg00222.txt.bz2 On Fri, Mar 7, 2014 at 10:33 AM, Pedro Alves wrote: > On 03/06/2014 05:20 AM, Doug Evans wrote: >> On Mon, Mar 3, 2014 at 5:36 PM, Yao Qi wrote: >>> On 03/04/2014 09:18 AM, Hui Zhu wrote: >>>> I cannot understand about this OB is not right. I have 2 questions to you: >>>> 1. Before my patch, does target-delegates.c that generated by make-target-delegates is same with current target-delegates.c? >>> >>> No, as I said, I forgot to re-generate target-delegates.c. >> >> Hmmm.... >> >> I don't even see target-delegates.c in Makefile.in. That feels like a >> bug. [Could be blind of course. :-)] >> I realize there's a comment in target-delegates.c that says how to >> regenerate it, but these kind of things are part of what makefiles are >> for. > > This also crossed my mind when initially reviewing the series (and > I'm sure Tom's too when writing it, as it's such an obvious thing), > but realized this is really no different from e.g., gdbarch.h|c. > > So given the precedent, I don't consider this a particular bug of > target-delegates.c, but a more generic "IWBN if we had Makefile > rules for our generated files". It wouldn't be gdb if finding one bug didn't expose one or several more. :-) In this case, these kinds of things are trivial to write, IMO they should be part of the patch. It's s.o.p. > Of course, I'd welcome patches in that direction. I'd say let's not make the problem worse. This is trivial stuff, and someone has already tripped over this. [--enable-maintainer-mode would likely *not* have prevented it the first time for any individual (they're likely not aware of --enable-maintainer-mode), but we *should* be providing the means for this to automagically work] >> I'm not sure I'd want to require perl for --enable-maintainer-mode >> (which is a common trigger for enabling in makefiles the appropriate >> rules to auto-regenerate checked-in machine-generated files), but it's >> one thought. > > I don't see a problem there. automake is perl as well, for instance, > and it's common for --enable-maintainer-mode to trigger automake/aclocal. I recognize the *technical* need of having perl installed for use by automake, but that's not the context in which my comment is phrased. How much perl do I need to know in order to use automake? [If perl isn't installed and I do "yum install automake", the necessary bits of perl get installed (I hope) but Joe Developer needn't be aware of that.] My comment is phrased the way it is because I was not prepared to a-priori declare one needs to know perl when --enable-maintainer-mode is turned on: If the user turns it on and that exposes a problem/issue/whatever with target-delegates.c, then I expect the user to have to deal with it, it's one more piece if implementation infrastructure we've imposed on developers. Today, if a developer turns on --enable-maintainer-mode I'm not going to apologize for having imposed on them some minimal comfortability with autoconf. Typically one needn't know very much (hopefully just having the right version installed will solve their problem), but IME that's not always the case. Help is always at hand of course, still there's a non-trivial additional load being imposed on developers, and I'm not prepared to a-priori impose it. btw, for reference sake, I can imagine another reason for wanting something other than --enable-maintainer-mode: Depending on what files one is expecting to be editing (configure.ac or target.h or ...?) one might not have the right versions of the tools installed in order to avoid spurious difference in the generated files, and I can imagine IWBN if I was hacking on one file to not want to have to make sure I have the right versions of all the tools that --enable-maintainer-mode uses. This is more IWBN though, the converse of course is a growing number of --enable-foo options which has its own problems. > Even if that weren't true, by configuring with --enable-maintainer-mode, > by definition you're asserting you have the tools required for > regular gdb maintenance, and given make-target-delegates is perl, > well, any maintainer must have it handy.