Обсуждение: missing requirement on ccache in postgresql16-devel

Поиск
Список
Период
Сортировка

missing requirement on ccache in postgresql16-devel

От
Greg Hennessy
Дата:
The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a reference
to ccache but the rpm doesn't list this as a dependency. I would like
to submit that it is a bug that it does not.

Sincerely,
Greg Hennessy

Re: missing requirement on ccache in postgresql16-devel

От
Bruce Momjian
Дата:
On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
> The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a reference
> to ccache but the rpm doesn't list this as a dependency. I would like
> to submit that it is a bug that it does not.

Uh, ccache is an _optional_ way to speed up compilation.  It is not a
requirement/dependency.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.



Re: missing requirement on ccache in postgresql16-devel

От
Tom Lane
Дата:
Bruce Momjian <bruce@momjian.us> writes:
> On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
>> The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a reference
>> to ccache but the rpm doesn't list this as a dependency. I would like
>> to submit that it is a bug that it does not.

> Uh, ccache is an _optional_ way to speed up compilation.  It is not a
> requirement/dependency.

Yeah, but if the Makefile says "CC = ccache gcc" then anybody
trying to build under PGXS will fail if they don't have ccache
installed.  So I think Greg has a point.

On typical Red Hat setups, it's not necessary to mention ccache
explicitly to use it; instead that's managed via PATH.  For
example, I have

$ which gcc
/usr/lib64/ccache/gcc

$ ls -l /usr/lib64/ccache/gcc
lrwxrwxrwx. 1 root root 16 May 21 09:35 /usr/lib64/ccache/gcc -> ../../bin/ccache

so it Just Works.  I'd suggest arranging the rpm builds to be
done similarly, and then you get ccache usage in the rpm build,
while the distributed Makefile will just say "CC = gcc" so
it will work for users whether they use ccache or not.

            regards, tom lane



Re: missing requirement on ccache in postgresql16-devel

От
Bruce Momjian
Дата:
On Wed, Oct 25, 2023 at 01:21:44PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
> >> The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a reference
> >> to ccache but the rpm doesn't list this as a dependency. I would like
> >> to submit that it is a bug that it does not.
> 
> > Uh, ccache is an _optional_ way to speed up compilation.  It is not a
> > requirement/dependency.
> 
> Yeah, but if the Makefile says "CC = ccache gcc" then anybody
> trying to build under PGXS will fail if they don't have ccache
> installed.  So I think Greg has a point.

Well, the question is where can we see this file,
/usr/pgsql-16/lib/pgxs/src/Makefile.global?  Who created it?  What does
it contain?

Mine is built from Makefile.global.in, and has:

    CC = ccache gcc

but I define CC='ccache gcc' before I run autoconf.

> On typical Red Hat setups, it's not necessary to mention ccache
> explicitly to use it; instead that's managed via PATH.  For
> example, I have
> 
> $ which gcc
> /usr/lib64/ccache/gcc
> 
> $ ls -l /usr/lib64/ccache/gcc
> lrwxrwxrwx. 1 root root 16 May 21 09:35 /usr/lib64/ccache/gcc -> ../../bin/ccache

Oh, I have not done that.  I like to know when ccache is being used.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.



Re: missing requirement on ccache in postgresql16-devel

От
Devrim Gündüz
Дата:
Hi,

On Wed, 2023-10-25 at 13:21 -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
> > > The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a
> > > reference
> > > to ccache but the rpm doesn't list this as a dependency. I would
> > > like
> > > to submit that it is a bug that it does not.
>
> > Uh, ccache is an _optional_ way to speed up compilation.  It is not
> > a requirement/dependency.
>
> Yeah, but if the Makefile says "CC = ccache gcc" then anybody
> trying to build under PGXS will fail if they don't have ccache
> installed. 

Makefile.global has this reference to ccache on RHEL 9 and Fedora:

CLANG = /usr/lib64/ccache/clang

which is caused by:

%if 0%{?rhel} && 0%{?rhel} == 8
        export CLANG=%{_bindir}/clang LLVM_CONFIG=%{_bindir}/llvm-config-64
%endif

(so this issue does not exist on RHEL 8).

Pushed a fix to PostgreSQL 17. I'll check the results tomorrow, and then
will patch PostgreSQL 11+.

Greg: Thanks for the report. The changes will appear in next month's
minor release set (Nov 9). I'm not inclined to push an update set just
for this issue.

Regards,
--
Devrim Gündüz
Open Source Solution Architect, PostgreSQL Major Contributor
Twitter: @DevrimGunduz , @DevrimGunduzTR



Re: missing requirement on ccache in postgresql16-devel

От
Bruce Momjian
Дата:
On Thu, Oct 26, 2023 at 10:42:10PM +0100, Devrim Gunduz wrote:
> 
> Hi,
> 
> On Wed, 2023-10-25 at 13:21 -0400, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
> > > > The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a
> > > > reference
> > > > to ccache but the rpm doesn't list this as a dependency. I would
> > > > like
> > > > to submit that it is a bug that it does not.
> > 
> > > Uh, ccache is an _optional_ way to speed up compilation.  It is not
> > > a requirement/dependency.
> > 
> > Yeah, but if the Makefile says "CC = ccache gcc" then anybody
> > trying to build under PGXS will fail if they don't have ccache
> > installed. 
> 
> Makefile.global has this reference to ccache on RHEL 9 and Fedora:
> 
> CLANG = /usr/lib64/ccache/clang
> 
> which is caused by:
> 
> %if 0%{?rhel} && 0%{?rhel} == 8
>         export CLANG=%{_bindir}/clang LLVM_CONFIG=%{_bindir}/llvm-config-64
> %endif
> 
> (so this issue does not exist on RHEL 8).
> 
> Pushed a fix to PostgreSQL 17. I'll check the results tomorrow, and then
> will patch PostgreSQL 11+.
> 
> Greg: Thanks for the report. The changes will appear in next month's
> minor release set (Nov 9). I'm not inclined to push an update set just
> for this issue.

Great, thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.



Re: missing requirement on ccache in postgresql16-devel

От
Greg Hennessy
Дата:
Thank you very much. I agree that waiting till the next minor release is fine.

Greg


On Thu, Oct 26, 2023 at 5:42 PM Devrim Gündüz <devrim@gunduz.org> wrote:

Hi,

On Wed, 2023-10-25 at 13:21 -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Wed, Oct 25, 2023 at 11:25:15AM -0400, Greg Hennessy wrote:
> > > The file /usr/pgsql-16/lib/pgxs/src/Makefile.global has a
> > > reference
> > > to ccache but the rpm doesn't list this as a dependency. I would
> > > like
> > > to submit that it is a bug that it does not.
>
> > Uh, ccache is an _optional_ way to speed up compilation.  It is not
> > a requirement/dependency.
>
> Yeah, but if the Makefile says "CC = ccache gcc" then anybody
> trying to build under PGXS will fail if they don't have ccache
> installed. 

Makefile.global has this reference to ccache on RHEL 9 and Fedora:

CLANG = /usr/lib64/ccache/clang

which is caused by:

%if 0%{?rhel} && 0%{?rhel} == 8
        export CLANG=%{_bindir}/clang LLVM_CONFIG=%{_bindir}/llvm-config-64
%endif

(so this issue does not exist on RHEL 8).

Pushed a fix to PostgreSQL 17. I'll check the results tomorrow, and then
will patch PostgreSQL 11+.

Greg: Thanks for the report. The changes will appear in next month's
minor release set (Nov 9). I'm not inclined to push an update set just
for this issue.

Regards,
--
Devrim Gündüz
Open Source Solution Architect, PostgreSQL Major Contributor
Twitter: @DevrimGunduz , @DevrimGunduzTR