Horton

Thread

Commits

  1. Fix some OBJS lists in two Makefiles to be ordered alphabetically

  2. Split all OBJS style lines in makefiles into one-line-per-entry style.

  1. RFC: split OBJS lines to one object per line

    Andres Freund <andres@anarazel.de> — 2019-10-29T20:09:01Z

    Hi,

one of the most frequent conflicts I see is that two patches add files
to OBJS (or one of its other spellings), and there are conflicts because
another file has been added.

Right now there's two reasons why that's likely to happen:
1) By listing multiple objects for each line, we get a conflict whenever
   one of the other files on that lines gets modified
2) Due to our line-length conventions, we have to re-flow long lines,
   which often triggers reflowing subsequent lines too.

Now, obviously these types of conflicts are easy enough to resolve, but
it's still annoying.  It seems that this would be substantially less
often a problem if we just split such lines to one file per
line. E.g. instead of

OBJS_COMMON = base64.o config_info.o controldata_utils.o d2s.o exec.o f2s.o \
	file_perm.o ip.o keywords.o kwlookup.o link-canary.o md5.o \
	pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
	rmtree.o saslprep.o scram-common.o string.o  stringinfo.o \
	unicode_norm.o username.o wait_error.o

have

OBJS_COMMON = \
	base64.o \
	config_info.o \
	controldata_utils.o \
	d2s.o \
	exec.o \
	f2s.o \
	file_perm.o \
	ip.o \
	keywords.o \
	kwlookup.o \
	link-canary.o \
	md5.o \
	pg_lzcompress.o \
	pgfnames.o \
	psprintf.o \
	relpath.o \
	rmtree.o \
	saslprep.o \
	scram-common.o \
	string.o \
	stringinfo.o \
	unicode_norm.o \
	username.o \
	wait_error.o

a one-off conversion of this seems easy enough to script.

Comments?

- Andres

  2. Re: RFC: split OBJS lines to one object per line

    Peter Geoghegan <pg@bowt.ie> — 2019-10-29T20:16:49Z

    On Tue, Oct 29, 2019 at 1:09 PM Andres Freund <andres@anarazel.de> wrote:
> Comments?

I think that this is a good idea. I see no downside.


-- 
Peter Geoghegan

  3. Re: RFC: split OBJS lines to one object per line

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-10-29T20:31:11Z

    Andres Freund <andres@anarazel.de> writes:
> one of the most frequent conflicts I see is that two patches add files
> to OBJS (or one of its other spellings), and there are conflicts because
> another file has been added.
> ...
> Now, obviously these types of conflicts are easy enough to resolve, but
> it's still annoying.  It seems that this would be substantially less
> often a problem if we just split such lines to one file per
> line.

We did something similar not too long ago in configure.in (bfa6c5a0c),
and it seems to have helped.  +1

			regards, tom lane

  4. Re: RFC: split OBJS lines to one object per line

    Andres Freund <andres@anarazel.de> — 2019-10-30T06:32:09Z

    Hi,

On 2019-10-29 16:31:11 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > one of the most frequent conflicts I see is that two patches add files
> > to OBJS (or one of its other spellings), and there are conflicts because
> > another file has been added.
> > ...
> > Now, obviously these types of conflicts are easy enough to resolve, but
> > it's still annoying.  It seems that this would be substantially less
> > often a problem if we just split such lines to one file per
> > line.
> 
> We did something similar not too long ago in configure.in (bfa6c5a0c),
> and it seems to have helped.  +1

Cool. Any opinion on whether to got for

OBJS = \
	dest.o \
	fastpath.o \
...

or

OBJS = dest.o \
	fastpath.o \
...

I'm mildly inclined to go for the former.

Greetings,

Andres Freund

  5. Re: RFC: split OBJS lines to one object per line

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-10-30T06:56:14Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-10-29 16:31:11 -0400, Tom Lane wrote:
>> We did something similar not too long ago in configure.in (bfa6c5a0c),
>> and it seems to have helped.  +1

> Cool. Any opinion on whether to got for ...

Not here.

			regards, tom lane

  6. Re: RFC: split OBJS lines to one object per line

    Michael Paquier <michael@paquier.xyz> — 2019-10-30T07:40:45Z

    On Tue, Oct 29, 2019 at 11:32:09PM -0700, Andres Freund wrote:
> Cool. Any opinion on whether to got for
> 
> OBJS = \
> 	dest.o \
> 	fastpath.o \
> ...
> 
> or
> 
> OBJS = dest.o \
> 	fastpath.o \
> ...
> 
> I'm mildly inclined to go for the former.

FWIW, I am more used to the latter, but the former is also fine by
me.
--
Michael

  7. Re: RFC: split OBJS lines to one object per line

    Mark Dilger <hornschnorter@gmail.com> — 2019-10-31T16:48:46Z

    
On 10/29/19 11:32 PM, Andres Freund wrote:
> Hi,
> 
> On 2019-10-29 16:31:11 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> one of the most frequent conflicts I see is that two patches add files
>>> to OBJS (or one of its other spellings), and there are conflicts because
>>> another file has been added.
>>> ...
>>> Now, obviously these types of conflicts are easy enough to resolve, but
>>> it's still annoying.  It seems that this would be substantially less
>>> often a problem if we just split such lines to one file per
>>> line.
>>
>> We did something similar not too long ago in configure.in (bfa6c5a0c),
>> and it seems to have helped.  +1
> 
> Cool. Any opinion on whether to got for
> 
> OBJS = \
> 	dest.o \
> 	fastpath.o \
> ...
> 
> or
> 
> OBJS = dest.o \
> 	fastpath.o \
> ...
> 
> I'm mildly inclined to go for the former.

+1 for the former.

> 
> Greetings,
> 
> Andres Freund
> 
>

  8. Re: RFC: split OBJS lines to one object per line

    Andres Freund <andres@anarazel.de> — 2019-11-05T22:47:55Z

    Hi,

On 2019-10-29 23:32:09 -0700, Andres Freund wrote:
> On 2019-10-29 16:31:11 -0400, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > one of the most frequent conflicts I see is that two patches add files
> > > to OBJS (or one of its other spellings), and there are conflicts because
> > > another file has been added.
> > > ...
> > > Now, obviously these types of conflicts are easy enough to resolve, but
> > > it's still annoying.  It seems that this would be substantially less
> > > often a problem if we just split such lines to one file per
> > > line.
> > 
> > We did something similar not too long ago in configure.in (bfa6c5a0c),
> > and it seems to have helped.  +1
> 
> Cool. Any opinion on whether to got for
> 
> OBJS = \
> 	dest.o \
> 	fastpath.o \
> ...
> 
> or
> 
> OBJS = dest.o \
> 	fastpath.o \
> ...
> 
> I'm mildly inclined to go for the former.

Pushed a patch going with the former. Let's see what the buildfarm
says...

Greetings,

Andres Freund

  9. Re: RFC: split OBJS lines to one object per line

    Michael Paquier <michael@paquier.xyz> — 2019-11-07T02:24:37Z

    On Tue, Nov 05, 2019 at 02:47:55PM -0800, Andres Freund wrote:
> Pushed a patch going with the former. Let's see what the buildfarm
> says...

Thanks Andres.  On a rather related note, would it make sense to do
the same for regression and isolation tests in our in-core modules?
--
Michael

  10. Re: RFC: split OBJS lines to one object per line

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-07T17:02:04Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Nov 05, 2019 at 02:47:55PM -0800, Andres Freund wrote:
>> Pushed a patch going with the former. Let's see what the buildfarm
>> says...

> Thanks Andres.  On a rather related note, would it make sense to do
> the same for regression and isolation tests in our in-core modules?

I don't think it'd be a great idea to change parallel_schedule like
that.  Independently adding test scripts to the same parallel batch
probably won't end well: you might end up over the concurrency limit,
or the scripts might conflict through sharing table names or the like.
So I'd rather see that there's a conflict to worry about.

Anyway, merge conflicts there aren't so common IME.

			regards, tom lane

  11. Re: RFC: split OBJS lines to one object per line

    Andres Freund <andres@anarazel.de> — 2019-11-07T17:20:02Z

    Hi,

On 2019-11-07 11:24:37 +0900, Michael Paquier wrote:
> On Tue, Nov 05, 2019 at 02:47:55PM -0800, Andres Freund wrote:
> > Pushed a patch going with the former. Let's see what the buildfarm
> > says...
> 
> Thanks Andres.  On a rather related note, would it make sense to do
> the same for regression and isolation tests in our in-core modules?

I don't see them as being frequent sources of conflicts (partially
because we don't change linebreaks due to line length limits, I think),
so I don't think it's really worthwhile.

One I could see some benefit in, would be the SUBDIRS makefile lines.

Greetings,

Andres Freund

  12. Re: RFC: split OBJS lines to one object per line

    Michael Paquier <michael@paquier.xyz> — 2019-11-08T09:07:57Z

    On Thu, Nov 07, 2019 at 12:02:04PM -0500, Tom Lane wrote:
> I don't think it'd be a great idea to change parallel_schedule like
> that.  Independently adding test scripts to the same parallel batch
> probably won't end well: you might end up over the concurrency limit,
> or the scripts might conflict through sharing table names or the like.
> So I'd rather see that there's a conflict to worry about.
> 
> Anyway, merge conflicts there aren't so common IME.

FWIW, I was not referring to the schedule files here, just to REGRESS
and ISOLATION in the modules' Makefiles.  If you think that's not
worth doing it, let's drop my suggestion then.
--
Michael

  13. Re: RFC: split OBJS lines to one object per line

    Mahendra Singh Thalor <mahi6run@gmail.com> — 2019-12-17T18:10:17Z

    On Fri, 8 Nov 2019 at 14:38, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Nov 07, 2019 at 12:02:04PM -0500, Tom Lane wrote:
> > I don't think it'd be a great idea to change parallel_schedule like
> > that.  Independently adding test scripts to the same parallel batch
> > probably won't end well: you might end up over the concurrency limit,
> > or the scripts might conflict through sharing table names or the like.
> > So I'd rather see that there's a conflict to worry about.
> >
> > Anyway, merge conflicts there aren't so common IME.
>
> FWIW, I was not referring to the schedule files here, just to REGRESS
> and ISOLATION in the modules' Makefiles.  If you think that's not
> worth doing it, let's drop my suggestion then.
> --

I found some inconsistency in alphabetical order in
src/backend/tsearch/Makefile, src/backend/utils/Makefile and
src/pl/plpython/Makefile files.  Attached patch is fixing those order
related inconsistency.

Thanks and Regards
Mahendra Thalor
EnterpriseDB: http://www.enterprisedb.com

  14. Re: RFC: split OBJS lines to one object per line

    Michael Paquier <michael@paquier.xyz> — 2019-12-18T01:53:37Z

    On Tue, Dec 17, 2019 at 11:40:17PM +0530, Mahendra Singh wrote:
> I found some inconsistency in alphabetical order in
> src/backend/tsearch/Makefile, src/backend/utils/Makefile and
> src/pl/plpython/Makefile files.  Attached patch is fixing those order
> related inconsistency.

Thanks, committed.  The one-liner style is also used in ifaddrs, but
fmgrtab.c is generated so I have left that out.  Now, have you tried
to compile plpython before sending this patch?  Because as you forgot
to add one backslash after WIN32RES, compilation was failing there.
And you also forgot to remove two backslashes at the end of the other
two lists modified :)
--
Michael

  15. Re: RFC: split OBJS lines to one object per line

    Mahendra Singh Thalor <mahi6run@gmail.com> — 2019-12-18T03:00:24Z

    On Wed, 18 Dec 2019 at 07:23, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Dec 17, 2019 at 11:40:17PM +0530, Mahendra Singh wrote:
> > I found some inconsistency in alphabetical order in
> > src/backend/tsearch/Makefile, src/backend/utils/Makefile and
> > src/pl/plpython/Makefile files.  Attached patch is fixing those order
> > related inconsistency.
>
> Thanks, committed.  The one-liner style is also used in ifaddrs, but

Thanks Michael for quick response.

> fmgrtab.c is generated so I have left that out.  Now, have you tried
> to compile plpython before sending this patch?  Because as you forgot
> to add one backslash after WIN32RES, compilation was failing there.
> And you also forgot to remove two backslashes at the end of the other
> two lists modified :)

Sorry, I forgot to add backslashes. I will take care from next time.

Thanks and Regards
Mahendra Thalor
EnterpriseDB: http://www.enterprisedb.com