This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/8] bpf: new GCC port


On Thu, Aug 22, 2019 at 06:26:57AM -0400, Hans-Peter Nilsson wrote:
> On Wed, 21 Aug 2019, Segher Boessenkool wrote:
> > On Tue, Aug 20, 2019 at 04:05:40PM -0400, Hans-Peter Nilsson wrote:
> > > On Tue, 20 Aug 2019, Jose E. Marchesi wrote:
> > > >     > On Thu, Aug 15, 2019 at 12:22:46AM +0200, Jose E. Marchesi wrote:
> > > >     > > --- a/configure
> > > >     > > +++ b/configure
> > > > Yeah by mistake I used a Debian patched autoconf 2.96.  Will regenerate
> > > > using vanilla autoconf for subsequent versions of the patch.
> > >
> > > It's nice that this is identified and hopefully resolved, but
> > > since nobody mentioned it I'll just point out that
> > > it's preferable to *not at all* include generated files like
> > > configure in patches.  See
> > > <https://gcc.gnu.org/contribute.html#patches>.
> >
> > "Do not include generated files as part of the patch, just mention them
> > in the ChangeLog (e.g., "* configure: Regenerate.")."
> >
> > That's not common practice nowadays I think?
> 
> I don't know about that.  But, if that's a valid observation
> then I'd say people just tend to generally drift to the path of
> least resistance for the immediate task at hand.  It's rarely an
> indication of *good* practice by itself, but perhaps that it's
> time to bring it up.

(The changelog should just say "Regenerate.", I'm refering only to
posting the actual diff to the configure script).

> > It's also not good advice
> > for people who might get it wrong.
> 
> You mean the autoconf version, I guess.

For people who get the regeneration wrong in any way.  Wrong version of
autoxxxx is frequent, sure.

> That's about the only
> value of including generated files; for newcomers that miss
> using matched versions.  Including bulk in patches can also
> cause grief by messages being lost when they exceed the
> size-limit.

Again, I only meant the configuration scripts.

(What autogenerated files are much bigger than their source files, btw?
As diffs even?)

> >  Also, the patches on the mailing
> > list should preferably be exactly what is committed.  For sanity.
> 
> IMHO sanity for gcc-patches is maximum review-value and
> readability for others, avoiding redundancy.  I'd just think
> sending generated files for review is as redundant now as it was
> when that advice was added.

For many things you are right afaics.  But I see people getting autoxxxx
wrong often.

> Though, if you as a frequent reviewer believe that the valued
> majority of reviewers and readers thinks differently, I suggest
> taking steps to update the advice.  Either way, keep the advice
> up-to-date, for sanity.

:-)


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]