Fix Makefile.in in data-directory

Fix Makefile.in in data-directory

[Jie Zhang]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

The current install rule in the Makefile.in of data-directory uses 
recursive make but does not pass FLAGS_TO_PASS. This causes an issue 
when people try to override prefix with command line option "make 
install", i.e. data-directory will still be installed under the 
configure time prefix. This patch fixes it.  OK?


Regards,
-- 
Jie Zhang
CodeSourcery

[-- Attachment #2: gdb-fix-data-directory-Makefile.diff --]
[-- Type: text/x-patch, Size: 730 bytes --]


	* data-directory/Makefile.in (install): Add dependency on
	install-only.  Delete code making it.

Index: data-directory/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/data-directory/Makefile.in,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile.in
--- data-directory/Makefile.in	13 Oct 2010 20:08:44 -0000	1.3
+++ data-directory/Makefile.in	20 Oct 2010 11:54:39 -0000
@@ -145,8 +145,7 @@ uninstall-python:
 # time it takes for make to check that all is up to date.
 # install-only is intended to address that need.
 .PHONY: install
-install: all
-	$(MAKE) install-only
+install: all install-only
 
 .PHONY: install-only
 install-only: install-syscalls install-python

Re: Fix Makefile.in in data-directory

[Doug Evans]

On Wed, Oct 20, 2010 at 5:09 AM, Jie Zhang <jie@codesourcery.com> wrote:
>
> The current install rule in the Makefile.in of data-directory uses recursive make but does not pass FLAGS_TO_PASS. This causes an issue when people try to override prefix with command line option "make install", i.e. data-directory will still be installed under the configure time prefix. This patch fixes it.  OK?

[Apologies for the resend.]

Thanks for the bug report.  Forgot about that.

This patch is not ok though.  A parallel make (e.g. -jN) might try to
build all and install-only simultaneously.

maybe:
$(MAKE) install-only $(FLAGS_TO_PASS)


For reference sake, there are other instances of "install: all install-only".
They need to be fixed too.

Re: Fix Makefile.in in data-directory

[Jie Zhang]

On 10/21/2010 04:13 AM, Doug Evans wrote:
> On Wed, Oct 20, 2010 at 5:09 AM, Jie Zhang<jie@codesourcery.com>  wrote:
>>
>> The current install rule in the Makefile.in of data-directory uses recursive make but does not pass FLAGS_TO_PASS. This causes an issue when people try to override prefix with command line option "make install", i.e. data-directory will still be installed under the configure time prefix. This patch fixes it.  OK?
>
> [Apologies for the resend.]
>
> Thanks for the bug report.  Forgot about that.
>
> This patch is not ok though.  A parallel make (e.g. -jN) might try to
> build all and install-only simultaneously.
>
> maybe:
> $(MAKE) install-only $(FLAGS_TO_PASS)
>
>
> For reference sake, there are other instances of "install: all install-only".
> They need to be fixed too.

I don't think this is an issue. I think in the last more than 10 years, 
no one did "make -j all install" since the same instance in 
gdb/Makefile.in has been there since it was created. Actually if you do 
that in current gdb, it will not install gdb at all. I usually issue

make -j4 all
make install

to make gdb. I believe people have been used to do that in this way 
after so many years. So I think it's not an issue to do the same for 
data-directory. No?


-- 
Jie Zhang
CodeSourcery

Re: Fix Makefile.in in data-directory

[Pedro Alves]

On Thursday 21 October 2010 01:10:14, Jie Zhang wrote:
> I don't think this is an issue. I think in the last more than 10 years, 
> no one did "make -j all install" since the same instance in 
> gdb/Makefile.in has been there since it was created. Actually if you do 
> that in current gdb, it will not install gdb at all. I usually issue
> 
> make -j4 all
> make install

FWIW, I got surprised before learning that "make -j4 install" misbehaves
like that.  I'd rather have it fixed.  It just sounds like a buglet people
got used to dealing with.

-- 
Pedro Alves

Re: Fix Makefile.in in data-directory

[Jie Zhang]

On 10/21/2010 08:14 AM, Pedro Alves wrote:
> On Thursday 21 October 2010 01:10:14, Jie Zhang wrote:
>> I don't think this is an issue. I think in the last more than 10 years,
>> no one did "make -j all install" since the same instance in
>> gdb/Makefile.in has been there since it was created. Actually if you do
>> that in current gdb, it will not install gdb at all. I usually issue
>>
>> make -j4 all
>> make install
>
> FWIW, I got surprised before learning that "make -j4 install" misbehaves
> like that.  I'd rather have it fixed.  It just sounds like a buglet people
> got used to dealing with.
>
I didn't mean that "make -j4 install" misbehaves for gdb. It's "make -j4 
all install" that misbehaves.

-- 
Jie Zhang
CodeSourcery

Re: Fix Makefile.in in data-directory

[Pedro Alves]

On Thursday 21 October 2010 01:17:42, Jie Zhang wrote:
> On 10/21/2010 08:14 AM, Pedro Alves wrote:
> > On Thursday 21 October 2010 01:10:14, Jie Zhang wrote:
> >> I don't think this is an issue. I think in the last more than 10 years,
> >> no one did "make -j all install" since the same instance in
> >> gdb/Makefile.in has been there since it was created. Actually if you do
> >> that in current gdb, it will not install gdb at all. I usually issue
> >>
> >> make -j4 all
> >> make install
> >
> > FWIW, I got surprised before learning that "make -j4 install" misbehaves
> > like that.  I'd rather have it fixed.  It just sounds like a buglet people
> > got used to dealing with.
> >
> I didn't mean that "make -j4 install" misbehaves for gdb. It's "make -j4 
> all install" that misbehaves.

Well, okay, it's the same difference.

It's caused by the same problem.  If you stop making "install" depend
on "all install-only", but make it depend on "all" only, as Doug was
saying, then "make -j4 all install" stops misbehaving, because "install"
depends on "all", so "install" only proceeds after "all" is done.

-- 
Pedro Alves

Re: Fix Makefile.in in data-directory

[Jie Zhang]

On 10/21/2010 08:28 AM, Pedro Alves wrote:
> On Thursday 21 October 2010 01:17:42, Jie Zhang wrote:
>> On 10/21/2010 08:14 AM, Pedro Alves wrote:
>>> On Thursday 21 October 2010 01:10:14, Jie Zhang wrote:
>>>> I don't think this is an issue. I think in the last more than 10 years,
>>>> no one did "make -j all install" since the same instance in
>>>> gdb/Makefile.in has been there since it was created. Actually if you do
>>>> that in current gdb, it will not install gdb at all. I usually issue
>>>>
>>>> make -j4 all
>>>> make install
>>>
>>> FWIW, I got surprised before learning that "make -j4 install" misbehaves
>>> like that.  I'd rather have it fixed.  It just sounds like a buglet people
>>> got used to dealing with.
>>>
>> I didn't mean that "make -j4 install" misbehaves for gdb. It's "make -j4
>> all install" that misbehaves.
>
> Well, okay, it's the same difference.
>
> It's caused by the same problem.  If you stop making "install" depend
> on "all install-only", but make it depend on "all" only, as Doug was
> saying, then "make -j4 all install" stops misbehaving, because "install"
> depends on "all", so "install" only proceeds after "all" is done.
>
I don't think it's really necessary to fix that. binutils and gcc also 
don't allow "make -j4 install" to directly after "configure". "make -j4 
all install" also fails for binutils and gcc. I think it's already an 
convention that those software are built with

./configure
make
make install

Do we really need the effort to fix GDB?


-- 
Jie Zhang
CodeSourcery

Re: Fix Makefile.in in data-directory

[Doug Evans]

On Wed, Oct 20, 2010 at 7:15 PM, Jie Zhang <jie@codesourcery.com> wrote:
> I don't think it's really necessary to fix that. binutils and gcc also don't
> allow "make -j4 install" to directly after "configure". "make -j4 all
> install" also fails for binutils and gcc. I think it's already an convention
> that those software are built with
>
> ./configure
> make
> make install
>
> Do we really need the effort to fix GDB?

fwiw, I think the answer is yes.

Note that we don't need to consider ./configure ; make -j4 install.

The issue arises with a simple "$EDITOR foo.c ; make -j4 install".

Re: Fix Makefile.in in data-directory

[Jie Zhang]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

On 10/21/2010 10:29 AM, Doug Evans wrote:
> On Wed, Oct 20, 2010 at 7:15 PM, Jie Zhang<jie@codesourcery.com>  wrote:
>> I don't think it's really necessary to fix that. binutils and gcc also don't
>> allow "make -j4 install" to directly after "configure". "make -j4 all
>> install" also fails for binutils and gcc. I think it's already an convention
>> that those software are built with
>>
>> ./configure
>> make
>> make install
>>
>> Do we really need the effort to fix GDB?
>
> fwiw, I think the answer is yes.
>
> Note that we don't need to consider ./configure ; make -j4 install.
>
> The issue arises with a simple "$EDITOR foo.c ; make -j4 install".

Reasonable although I never used it before. This is an updated patch 
which does that. It fixes it by passing FLAGS_TO_PASS when recursively 
make install-only. It fixes my issue. I also tested by touching a source 
file and do "make -j4 install" in gdb directory. It behaves as expected. 
OK now?


Regards,
-- 
Jie Zhang
CodeSourcery

[-- Attachment #2: gdb-fix-install-only.diff --]
[-- Type: text/x-patch, Size: 4396 bytes --]


	* Makefile.in (install): Remove dependency of install-only and
	recursively invoke make for install-only.
	* data-directory/Makefile.in: Add FLAGS_TO_PASS variable.
	(install): Pass FLAGS_TO_PASS when recursively make install-only.

	gdbserver/
	* Makefile.in: Add FLAGS_TO_PASS variable.
	(install): Remove dependency of install-only and recursively
	invoke make for install-only.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1136
diff -u -p -r1.1136 Makefile.in
--- Makefile.in	6 Oct 2010 16:46:12 -0000	1.1136
+++ Makefile.in	21 Oct 2010 04:22:12 -0000
@@ -970,7 +970,8 @@ gdb.z:gdb.1
 # source file and doesn't care about rebuilding or just wants to save the
 # time it takes for make to check that all is up to date.
 # install-only is intended to address that need.
-install: all install-only 
+install: all
+	@$(MAKE) $(FLAGS_TO_PASS) install-only
 
 install-only: $(CONFIG_INSTALL)
 	transformed_name=`t='$(program_transform_name)'; \
Index: data-directory/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/data-directory/Makefile.in,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile.in
--- data-directory/Makefile.in	13 Oct 2010 20:08:44 -0000	1.3
+++ data-directory/Makefile.in	21 Oct 2010 04:22:12 -0000
@@ -54,6 +54,38 @@ PYTHON_FILES = \
 	gdb/__init__.py \
 	gdb/types.py
 
+FLAGS_TO_PASS = \
+	"prefix=$(prefix)" \
+	"exec_prefix=$(exec_prefix)" \
+	"infodir=$(infodir)" \
+	"datarootdir=$(datarootdir)" \
+	"docdir=$(docdir)" \
+	"htmldir=$(htmldir)" \
+	"pdfdir=$(pdfdir)" \
+	"libdir=$(libdir)" \
+	"mandir=$(mandir)" \
+	"datadir=$(datadir)" \
+	"includedir=$(includedir)" \
+	"against=$(against)" \
+	"DESTDIR=$(DESTDIR)" \
+	"AR=$(AR)" \
+	"AR_FLAGS=$(AR_FLAGS)" \
+	"CC=$(CC)" \
+	"CFLAGS=$(CFLAGS)" \
+	"CXX=$(CXX)" \
+	"CXXFLAGS=$(CXXFLAGS)" \
+	"DLLTOOL=$(DLLTOOL)" \
+	"LDFLAGS=$(LDFLAGS)" \
+	"RANLIB=$(RANLIB)" \
+	"MAKEINFO=$(MAKEINFO)" \
+	"MAKEHTML=$(MAKEHTML)" \
+	"MAKEHTMLFLAGS=$(MAKEHTMLFLAGS)" \
+	"INSTALL=$(INSTALL)" \
+	"INSTALL_PROGRAM=$(INSTALL_PROGRAM)" \
+	"INSTALL_DATA=$(INSTALL_DATA)" \
+	"RUNTEST=$(RUNTEST)" \
+	"RUNTESTFLAGS=$(RUNTESTFLAGS)"
+
 .PHONY: all
 all: stamp-syscalls stamp-python
 
@@ -146,7 +178,7 @@ uninstall-python:
 # install-only is intended to address that need.
 .PHONY: install
 install: all
-	$(MAKE) install-only
+	@$(MAKE) $(FLAGS_TO_PASS) install-only
 
 .PHONY: install-only
 install-only: install-syscalls install-python
Index: gdbserver/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
retrieving revision 1.98
diff -u -p -r1.98 Makefile.in
--- gdbserver/Makefile.in	4 Oct 2010 23:32:40 -0000	1.98
+++ gdbserver/Makefile.in	21 Oct 2010 04:22:12 -0000
@@ -151,6 +151,38 @@ XML_BUILTIN = @srv_xmlbuiltin@
 IPA_DEPFILES = @IPA_DEPFILES@
 extra_libraries = @extra_libraries@
 
+FLAGS_TO_PASS = \
+	"prefix=$(prefix)" \
+	"exec_prefix=$(exec_prefix)" \
+	"infodir=$(infodir)" \
+	"datarootdir=$(datarootdir)" \
+	"docdir=$(docdir)" \
+	"htmldir=$(htmldir)" \
+	"pdfdir=$(pdfdir)" \
+	"libdir=$(libdir)" \
+	"mandir=$(mandir)" \
+	"datadir=$(datadir)" \
+	"includedir=$(includedir)" \
+	"against=$(against)" \
+	"DESTDIR=$(DESTDIR)" \
+	"AR=$(AR)" \
+	"AR_FLAGS=$(AR_FLAGS)" \
+	"CC=$(CC)" \
+	"CFLAGS=$(CFLAGS)" \
+	"CXX=$(CXX)" \
+	"CXXFLAGS=$(CXXFLAGS)" \
+	"DLLTOOL=$(DLLTOOL)" \
+	"LDFLAGS=$(LDFLAGS)" \
+	"RANLIB=$(RANLIB)" \
+	"MAKEINFO=$(MAKEINFO)" \
+	"MAKEHTML=$(MAKEHTML)" \
+	"MAKEHTMLFLAGS=$(MAKEHTMLFLAGS)" \
+	"INSTALL=$(INSTALL)" \
+	"INSTALL_PROGRAM=$(INSTALL_PROGRAM)" \
+	"INSTALL_DATA=$(INSTALL_DATA)" \
+	"RUNTEST=$(RUNTEST)" \
+	"RUNTESTFLAGS=$(RUNTESTFLAGS)"
+
 # Prevent Sun make from putting in the machine type.  Setting
 # TARGET_ARCH to nothing works for SunOS 3, 4.0, but not for 4.1.
 .c.o:
@@ -163,7 +195,9 @@ all: gdbserver$(EXEEXT) gdbreplay$(EXEEX
 # source file and doesn't care about rebuilding or just wants to save the
 # time it takes for make to check that all is up to date.
 # install-only is intended to address that need.
-install: all install-only
+install: all
+	@$(MAKE) $(FLAGS_TO_PASS) install-only
+
 install-only:
 	n=`echo gdbserver | sed '$(program_transform_name)'`; \
 	if [ x$$n = x ]; then n=gdbserver; else true; fi; \

Re: Fix Makefile.in in data-directory

[Pedro Alves]

On Thursday 21 October 2010 03:29:58, Doug Evans wrote:
> On Wed, Oct 20, 2010 at 7:15 PM, Jie Zhang <jie@codesourcery.com> wrote:
> > I don't think it's really necessary to fix that. binutils and gcc also don't
> > allow "make -j4 install" to directly after "configure". "make -j4 all
> > install" also fails for binutils and gcc. I think it's already an convention
> > that those software are built with
> >
> > ./configure
> > make
> > make install
> >
> > Do we really need the effort to fix GDB?
> 

GDB's makefile's in question reads:

># Traditionally "install" depends on "all".  But it may be useful
># not to; for example, if the user has made some trivial change to a
># source file and doesn't care about rebuilding or just wants to save the
># time it takes for make to check that all is up to date.
># install-only is intended to address that need.
>install: all install-only 

(enphasis on the comment.)  Maybe it's the 'Traditionally "install" depends
on "all".' part that's not so traditional afterall?  I really don't know.

(most users are better taught separate "make" and "make install" steps,
because it's most often necessary to need extra previleges
and "sudo make install" to install to /usr/local, but not to "make".)

> fwiw, I think the answer is yes.
> 
> Note that we don't need to consider ./configure ; make -j4 install.
> 
> The issue arises with a simple "$EDITOR foo.c ; make -j4 install".

Yeah, that's the scenerio where I got surprised that gdb started
both building and installing _simultaneously_, which has all the looks
of a dependency bug.  If install always behaved like install-only, I
wouldn't have been surprised!

FWIW, if "install" depending on "all" in the first place is
not so common (even "wrong?") afterall in GNU projects (it seems
automake does not?), I'd be personally okay to stop doing it in
gdb as well, and get rid of install-only, keeping only "install".
I never used install-only myself, and it does seem weird to behave
differently from bfd (which is automake based).

(To be clear, your new patch is fine with me as well, thanks!  Please
wait for Doug's review though.)

-- 
Pedro Alves

Re: Fix Makefile.in in data-directory

[Joel Brobecker]

> ># Traditionally "install" depends on "all".  But it may be useful
> ># not to; for example, if the user has made some trivial change to a
> ># source file and doesn't care about rebuilding or just wants to save the
> ># time it takes for make to check that all is up to date.
> ># install-only is intended to address that need.
> >install: all install-only 
> 
> (enphasis on the comment.)  Maybe it's the 'Traditionally "install" depends
> on "all".' part that's not so traditional afterall?  I really don't know.
> 
> (most users are better taught separate "make" and "make install" steps,
> because it's most often necessary to need extra previleges
> and "sudo make install" to install to /usr/local, but not to "make".)

AFAIK, the FSF recommends against making "make install" also do the build.

-- 
Joel

Re: Fix Makefile.in in data-directory

[Doug Evans]

On Wed, Oct 20, 2010 at 10:11 PM, Jie Zhang <jie@codesourcery.com> wrote:
> On 10/21/2010 10:29 AM, Doug Evans wrote:
>>
>> On Wed, Oct 20, 2010 at 7:15 PM, Jie Zhang<jie@codesourcery.com>  wrote:
>>>
>>> I don't think it's really necessary to fix that. binutils and gcc also
>>> don't
>>> allow "make -j4 install" to directly after "configure". "make -j4 all
>>> install" also fails for binutils and gcc. I think it's already an
>>> convention
>>> that those software are built with
>>>
>>> ./configure
>>> make
>>> make install
>>>
>>> Do we really need the effort to fix GDB?
>>
>> fwiw, I think the answer is yes.
>>
>> Note that we don't need to consider ./configure ; make -j4 install.
>>
>> The issue arises with a simple "$EDITOR foo.c ; make -j4 install".
>
> Reasonable although I never used it before. This is an updated patch which
> does that. It fixes it by passing FLAGS_TO_PASS when recursively make
> install-only. It fixes my issue. I also tested by touching a source file and
> do "make -j4 install" in gdb directory. It behaves as expected. OK now?

Thanks for persevering.

The patch is fine with me, though there is one obvious improvement.
It could be left for another pass.

FLAGS_TO_PASS is big, and there are now multiple copies of it.
IWBN if there were only one copy.
I wonder how much duplication there is in the makefiles, and if a
Makefile.common is reasonable.  It could be pasted in by configure.
[I realize gdbserver has its own configure, but it could, I think,
still use the same Makefile.common.]
I'm not sure it's warranted just for FLAGS_TO_PASS, so it could be
left for another day.

Feel free to check this in as is if you prefer.

Re: Fix Makefile.in in data-directory

[Doug Evans]

On Thu, Oct 21, 2010 at 3:07 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> (enphasis on the comment.)  Maybe it's the 'Traditionally "install" depends
> on "all".' part that's not so traditional afterall?  I really don't know.

Yeah, I don't know either.
If it is now, I'm pretty sure it didn't use to be traditional
(speaking generally, not just gnu).

Re: Fix Makefile.in in data-directory

[Jie Zhang]

On 10/22/2010 05:05 AM, Doug Evans wrote:
> On Wed, Oct 20, 2010 at 10:11 PM, Jie Zhang<jie@codesourcery.com>  wrote:
>> Reasonable although I never used it before. This is an updated patch which
>> does that. It fixes it by passing FLAGS_TO_PASS when recursively make
>> install-only. It fixes my issue. I also tested by touching a source file and
>> do "make -j4 install" in gdb directory. It behaves as expected. OK now?
>
> Thanks for persevering.
>
> The patch is fine with me, though there is one obvious improvement.
> It could be left for another pass.
>
> FLAGS_TO_PASS is big, and there are now multiple copies of it.
> IWBN if there were only one copy.
> I wonder how much duplication there is in the makefiles, and if a
> Makefile.common is reasonable.  It could be pasted in by configure.
> [I realize gdbserver has its own configure, but it could, I think,
> still use the same Makefile.common.]
> I'm not sure it's warranted just for FLAGS_TO_PASS, so it could be
> left for another day.
>
> Feel free to check this in as is if you prefer.

Committed now. Thanks.

-- 
Jie Zhang
CodeSourcery

Re: Fix Makefile.in in data-directory

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Maybe it's the 'Traditionally "install" depends on "all".' part
Pedro> that's not so traditional afterall?

My recollection is that this was a tradition among a certain group of
GNU maintainers.  It isn't in the GNU standards but was a common
extension.

Pedro> FWIW, if "install" depending on "all" in the first place is
Pedro> not so common (even "wrong?") afterall in GNU projects (it seems
Pedro> automake does not?)

I believe Automake does have install depend on all.  The generated code
is fairly roundabout, though.

Tom

Re: Fix Makefile.in in data-directory

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> AFAIK, the FSF recommends against making "make install" also do the build.

The standards don't explicitly say that, they just kind of imply it in a
funny way:

     If possible, write the `install' target rule so that it does not
     modify anything in the directory where the program was built,
     provided `make all' has just been done.  This is convenient for
     building the program under one user name and installing it under
     another.

This doesn't say anything about the behavior when 'all' was not just
invoked...

Though, ISTR that gdb always rebuilds some stuff on "make all".
That is bad independently though.

Tom