Re: [PATCH] Add common/ dir in build directories

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 12 Feb 2018, at 12:00, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Hi Alan,
> Thanks for doing this...
> This change is about config/makefile, so we need to run configure/make
> on various platforms, they are helpful to expose some issues, if any,
> 
>  x86_64-linux native and cross,
>  aarch64-linux native and cross,
>  native i686-w64-mingw32 and x86_64-w64-mingw32 on linux,
>  cross with enable all targets,

Tested the new patch with the above combinations.

Using x86_64-w64-mingw32-gcc (GCC) 4.8.2 on ubuntu 14.04,
I have a build failure against HEAD for mingw32:
unittests/scoped_fd-selftests.c fails because it cannot find mkstemp().
Not sure if this is a bug, or error in my setup.
Commenting out mkstemp code fixes my builds.


> 
>> @@ -1531,35 +1531,35 @@ TAGFILES_NO_SRCDIR = $(SFILES) $(HFILES_NO_SRCDIR) $(ALLDEPFILES) \
>> TAGFILES_WITH_SRCDIR = $(HFILES_WITH_SRCDIR)
>> 
>> COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>> -	btrace-common.o \
>> -	buffer.o \
>> -	cleanups.o \
>> -	common-agent.o \
>> -	common-debug.o \
>> -	common-exceptions.o \
>> -	job-control.o \
>> -	common-regcache.o \
>> -	common-utils.o \
>> -	environ.o \
>> -	errors.o \
>> +	common/btrace-common.o \
>> +	common/buffer.o \
>> +	common/cleanups.o \
>> +	common/agent.o \
>> +	common/common-debug.o \
>> +	common/common-exceptions.o \
>> +	common/job-control.o \
>> +	common/common-regcache.o \
>> +	common/common-utils.o \
>> +	common/environ.o \
>> +	common/errors.o \
>> +	common/fileio.o \
>> +	common/filestuff.o \
>> +	common/format.o \
>> +	common/gdb_tilde_expand.o \
>> +	common/gdb_vecs.o \
>> +	common/new-op.o \
>> +	common/print-utils.o \
>> +	common/ptid.o \
>> +	common/rsp-low.o \
>> +	common/run-time-clock.o \
>> +	common/signals.o \
>> +	common/signals-state-save-restore.o \
>> +	common/vec.o \
>> +	common/xml-utils.o \
>> 	exec.o \
>> -	fileio.o \
>> -	filestuff.o \
>> -	format.o \
>> -	gdb_tilde_expand.o \
>> -	gdb_vecs.o \
>> 	mi/mi-common.o \
>> -	new-op.o \
>> -	print-utils.o \
>> -	ptid.o \
>> -	rsp-low.o \
>> -	run-time-clock.o \
>> -	signals.o \
>> -	signals-state-save-restore.o \
>> -	vec.o \
>> 	version.o \
>> 	xml-builtin.o \
>> -	xml-utils.o \
>> 	$(patsubst %.c,%.o,$(COMMON_SFILES)) \
> 
> *.c files in COMMON_SFILES is replaced to *.o files here, so IMO, the
> better way is to move common/*.c files in SFILES to COMMON_SFILES.

Done!


> 
>> @@ -1627,7 +1627,7 @@ $(CONFIG_DEP_SUBDIR):
>> python/%.o: INTERNAL_CFLAGS += $(PYTHON_CFLAGS)
>> 
>> # Rules for compiling .c files in the various source subdirectories.
>> -%.o: ${srcdir}/common/%.c
>> +common/%.o: ${srcdir}/common/%.c
>> 	$(COMPILE) $<
>> 	$(POSTCOMPILE)
>> 
> 
> Do we still need this rule?  We already had a generic one,
> 
> %.o: %.c | $(CONFIG_DEP_SUBDIR)
> 	$(COMPILE) $<
> 	$(POSTCOMPILE)
> 

We don’t. Good spot. Removed.



>> diff --git a/gdb/common/common.host b/gdb/common/common.host
>> index b7051d6eb7ab77dff989866a936a78b200e62d49..826088635937603c98f179184d4b97b41ccd760c 100644
>> --- a/gdb/common/common.host
>> +++ b/gdb/common/common.host
>> @@ -30,7 +30,7 @@ case "${host}" in
>> *-mingw*)	common_host_obs=mingw-strerror.o
> 
> This should be updated too.
> 

Done. Not sure why I missed that. I do remember seeing it.

>> 		;;
>> *)
>> -		common_host_obs=posix-strerror.o
>> +		common_host_obs=common/posix-strerror.o
>> 		;;
>> 
> 
>> @@ -537,10 +537,29 @@ ax.o: ax.c
>> 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
>> 	$(POSTCOMPILE)
>> 
>> +# Note: Between two matching pattern rules, GNU Make 3.81 chooses the first one.
>> +# Therefore, these need to be before "%.o: %.c" for it to be considered for
>> +# files such as linux-amd64-ipa.o generated from linux-amd64-ipa.c.
>> +#
>> +# Later versions of GNU Make choose the rule with the shortest stem, so it would
>> +# work in any order.
>> +
>> +arch/%-ipa.o: ../arch/%.c
>> +	$(IPAGENT_COMPILE) $<
>> +	$(POSTCOMPILE)
>> +
>> +common/%-ipa.o: ../common/%.c
>> +	$(IPAGENT_COMPILE) $<
>> +	$(POSTCOMPILE)
>> +
>> arch/%.o: ../arch/%.c
>> 	$(COMPILE) $<
>> 	$(POSTCOMPILE)
> 
>> I had to move around the rules in the gdbserver makefile, otherwise the build
>> gets confused.
> 
> I don't understand why do you move them?  You mentioned this move in the
> commit log, but I can't see the reason we must do the move.  Can you
> elaborate?
> 

I had problems with compiling common/tdesc-ipa.c. 
...But that file does not exist yet.
Instead, I've simplified the change and now only add the common dir.


New version below.

Thanks for reviewing!


Alan.


gdb/ChangeLog:

2018-02-13  Alan Hayward  <alan.hayward@arm.com>

	* Makefile.in: Add common directory in build.
	* common/common.host: Add common reference.
	* configure: Likewise.
	* configure.ac: Likewise.

gdb/gdbserver/ChangeLog:

2018-02-13  Alan Hayward  <alan.hayward@arm.com>

	* Makefile.in: Add common directory in build.
	* configure: Add common reference.
	* configure.ac: Likewise.

---
 gdb/Makefile.in            | 87 ++++++++++++++--------------------------------
 gdb/common/common.host     |  4 +--
 gdb/configure              |  2 +-
 gdb/configure.ac           |  2 +-
 gdb/gdbserver/Makefile.in  | 64 +++++++++++++++++-----------------
 gdb/gdbserver/configure    |  4 +--
 gdb/gdbserver/configure.ac |  4 +--
 7 files changed, 66 insertions(+), 101 deletions(-)


diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 957654c9bd09b5aa7cbe65f163bf4341a292340b..d91d8e01c32a8487010d7d58dca2a8c28a588da4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -524,7 +524,7 @@ CONFIG_INSTALL = @CONFIG_INSTALL@
 CONFIG_UNINSTALL = @CONFIG_UNINSTALL@
 HAVE_NATIVE_GCORE_TARGET = @HAVE_NATIVE_GCORE_TARGET@

-CONFIG_SRC_SUBDIR = arch cli mi compile tui unittests guile python target
+CONFIG_SRC_SUBDIR = arch cli mi common compile tui unittests guile python target
 CONFIG_DEP_SUBDIR = $(addsuffix /$(DEPDIR),$(CONFIG_SRC_SUBDIR))

 # -I. for config files.
@@ -937,6 +937,31 @@ COMMON_SFILES = \
 	cli-out.c \
 	coff-pe-read.c \
 	coffread.c \
+	common/agent.c \
+	common/btrace-common.c \
+	common/buffer.c \
+	common/cleanups.c \
+	common/common-debug.c \
+	common/common-exceptions.c \
+	common/common-regcache.c \
+	common/common-utils.c \
+	common/errors.c \
+	common/environ.c \
+	common/fileio.c \
+	common/filestuff.c \
+	common/format.c \
+	common/job-control.c \
+	common/gdb_tilde_expand.c \
+	common/gdb_vecs.c \
+	common/new-op.c \
+	common/print-utils.c \
+	common/ptid.c \
+	common/rsp-low.c \
+	common/run-time-clock.c \
+	common/signals.c \
+	common/signals-state-save-restore.c \
+	common/vec.c \
+	common/xml-utils.c \
 	complaints.c \
 	completer.c \
 	continuations.c \
@@ -1119,31 +1144,6 @@ SFILES = \
 	stub-termcap.c \
 	symfile-mem.c \
 	ui-file.h \
-	common/agent.c \
-	common/btrace-common.c \
-	common/buffer.c \
-	common/cleanups.c \
-	common/common-debug.c \
-	common/common-exceptions.c \
-	common/common-regcache.c \
-	common/common-utils.c \
-	common/errors.c \
-	common/environ.c \
-	common/fileio.c \
-	common/filestuff.c \
-	common/format.c \
-	common/job-control.c \
-	common/gdb_tilde_expand.c \
-	common/gdb_vecs.c \
-	common/new-op.c \
-	common/print-utils.c \
-	common/ptid.c \
-	common/rsp-low.c \
-	common/run-time-clock.c \
-	common/signals.c \
-	common/signals-state-save-restore.c \
-	common/vec.c \
-	common/xml-utils.c \
 	mi/mi-common.c \
 	$(SUBDIR_TARGET_SRCS) \
 	$(COMMON_SFILES) \
@@ -1533,35 +1533,10 @@ TAGFILES_NO_SRCDIR = $(SFILES) $(HFILES_NO_SRCDIR) $(ALLDEPFILES) \
 TAGFILES_WITH_SRCDIR = $(HFILES_WITH_SRCDIR)

 COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
-	btrace-common.o \
-	buffer.o \
-	cleanups.o \
-	common-agent.o \
-	common-debug.o \
-	common-exceptions.o \
-	job-control.o \
-	common-regcache.o \
-	common-utils.o \
-	environ.o \
-	errors.o \
 	exec.o \
-	fileio.o \
-	filestuff.o \
-	format.o \
-	gdb_tilde_expand.o \
-	gdb_vecs.o \
 	mi/mi-common.o \
-	new-op.o \
-	print-utils.o \
-	ptid.o \
-	rsp-low.o \
-	run-time-clock.o \
-	signals.o \
-	signals-state-save-restore.o \
-	vec.o \
 	version.o \
 	xml-builtin.o \
-	xml-utils.o \
 	$(patsubst %.c,%.o,$(COMMON_SFILES)) \
 	$(SUBDIR_TARGET_OBS) \
 	$(SUBDIR_GCC_COMPILE_OBS)
@@ -1629,10 +1604,6 @@ $(CONFIG_DEP_SUBDIR):
 python/%.o: INTERNAL_CFLAGS += $(PYTHON_CFLAGS)

 # Rules for compiling .c files in the various source subdirectories.
-%.o: ${srcdir}/common/%.c
-	$(COMPILE) $<
-	$(POSTCOMPILE)
-
 %.o: $(srcdir)/gdbtk/generic/%.c
 	$(COMPILE) $(all_gdbtk_cflags) $<
 	$(POSTCOMPILE)
@@ -1641,12 +1612,6 @@ python/%.o: INTERNAL_CFLAGS += $(PYTHON_CFLAGS)
 	$(COMPILE) $<
 	$(POSTCOMPILE)

-# Specify an explicit rule for gdb/common/agent.c, to avoid a clash with the
-# object file generate by gdb/agent.c.
-common-agent.o: $(srcdir)/common/agent.c
-	$(COMPILE) $(srcdir)/common/agent.c
-	$(POSTCOMPILE)
-
 installcheck:

 # The check target can not use subdir_do, because subdir_do does not
diff --git a/gdb/common/common.host b/gdb/common/common.host
index b7051d6eb7ab77dff989866a936a78b200e62d49..87f144c261ab3aaa78c0837af6ad745d08bfa110 100644
--- a/gdb/common/common.host
+++ b/gdb/common/common.host
@@ -27,10 +27,10 @@

 case "${host}" in

-*-mingw*)	common_host_obs=mingw-strerror.o
+*-mingw*)	common_host_obs=common/mingw-strerror.o
 		;;
 *)
-		common_host_obs=posix-strerror.o
+		common_host_obs=common/posix-strerror.o
 		;;

 esac
diff --git a/gdb/configure b/gdb/configure
index 81b35af521e2704e2aca14011362971a030c49fe..ab58187853337c632af046adb589f9c8a5b9e8a3 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -17661,7 +17661,7 @@ if $development; then

 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h

-  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) selftest.o selftest-arch.o"
+  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
   CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
 fi

diff --git a/gdb/configure.ac b/gdb/configure.ac
index 125e5f30e65a7b046b5fab038bde3c5a2fe485c3..698fc7b83456f8c5a63ae0050dc8ec65069290f7 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2299,7 +2299,7 @@ AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
 if $development; then
   AC_DEFINE(GDB_SELF_TEST, 1,
             [Define if self-testing features should be enabled])
-  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) selftest.o selftest-arch.o"
+  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
   CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
 fi

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 3ce086d70f23df445b174c49c489ec8415d7614a..fcb6e1e817f521385de3986861c430c31a1b7eec 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -231,48 +231,48 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}

 OBS = \
-	agent.o \
 	ax.o \
-	btrace-common.o \
-	buffer.o \
-	cleanups.o \
-	common-debug.o \
-	common-exceptions.o \
-	job-control.o \
-	common-regcache.o \
-	common-utils.o \
+	common/agent.o \
+	common/btrace-common.o \
+	common/buffer.o \
+	common/cleanups.o \
+	common/common-debug.o \
+	common/common-exceptions.o \
+	common/job-control.o \
+	common/common-regcache.o \
+	common/common-utils.o \
+	common/errors.o \
+	common/environ.o \
+	common/fileio.o \
+	common/filestuff.o \
+	common/format.o \
+	common/gdb_tilde_expand.o \
+	common/gdb_vecs.o \
+	common/new-op.o \
+	common/print-utils.o \
+	common/ptid.o \
+	common/rsp-low.o \
+	common/signals.o \
+	common/signals-state-save-restore.o \
+	common/vec.o \
+	common/xml-utils.o \
 	debug.o \
 	dll.o \
-	errors.o \
-	environ.o \
 	event-loop.o \
-	fileio.o \
-	filestuff.o \
-	format.o \
-	gdb_tilde_expand.o \
-	gdb_vecs.o \
 	hostio.o \
 	inferiors.o \
 	mem-break.o \
-	new-op.o \
 	notif.o \
-	print-utils.o \
-	ptid.o \
 	regcache.o \
 	remote-utils.o \
-	rsp-low.o \
 	server.o \
-	signals.o \
-	signals-state-save-restore.o \
 	symbol.o \
 	target.o \
 	tdesc.o \
 	tracepoint.o \
 	utils.o \
-	vec.o \
 	version.o \
 	waitstatus.o \
-	xml-utils.o \
 	$(DEPFILES) \
 	$(LIBOBJS) \
 	$(XML_BUILTIN)
@@ -392,17 +392,17 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)

 IPA_OBJS = \
 	ax-ipa.o \
-	common-utils-ipa.o \
-	errors-ipa.o \
-	format-ipa.o \
-	print-utils-ipa.o \
+	common/common-utils-ipa.o \
+	common/errors-ipa.o \
+	common/format-ipa.o \
+	common/print-utils-ipa.o \
+	common/rsp-low-ipa.o \
+	common/vec-ipa.o \
 	regcache-ipa.o \
 	remote-utils-ipa.o \
-	rsp-low-ipa.o \
 	tdesc-ipa.o \
 	tracepoint-ipa.o \
 	utils-ipa.o \
-	vec-ipa.o \
 	${IPA_DEPFILES}

 IPA_LIB = libinproctrace.so
@@ -562,7 +562,7 @@ arch/%.o: ../arch/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

-%-ipa.o: ../common/%.c
+common/%-ipa.o: ../common/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

@@ -580,7 +580,7 @@ arch/%-ipa.o: ../arch/%.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)

-%.o: ../common/%.c
+common/%.o: ../common/%.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)

diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 340ca7a19e5c0bc24ee2fc42226762185295d5ef..ab09261946442f77c85e7fd8d5ecc090527d1d4c 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -5815,7 +5815,7 @@ fi


 if $development; then
-  srv_selftest_objs="selftest.o"
+  srv_selftest_objs="common/selftest.o"

 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h

@@ -5858,7 +5858,7 @@ ac_config_commands="$ac_config_commands depdir"


 # Create sub-directories for objects and dependencies.
-CONFIG_SRC_SUBDIR="arch"
+CONFIG_SRC_SUBDIR="arch common"


 ac_config_commands="$ac_config_commands gdbdepdir"
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 6727959e61b656ddfc2101b830539fa18288ccb3..99801681ff47ee8dcd9ad2e5ae282dcd113c83e4 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -57,7 +57,7 @@ fi
 GDB_AC_LIBMCHECK(${libmcheck_default})

 if $development; then
-  srv_selftest_objs="selftest.o"
+  srv_selftest_objs="common/selftest.o"
   AC_DEFINE(GDB_SELF_TEST, 1,
             [Define if self-testing features should be enabled])
 fi
@@ -69,7 +69,7 @@ ACX_NONCANONICAL_HOST
 ZW_CREATE_DEPDIR

 # Create sub-directories for objects and dependencies.
-CONFIG_SRC_SUBDIR="arch"
+CONFIG_SRC_SUBDIR="arch common"
 AC_SUBST(CONFIG_SRC_SUBDIR)

 AC_CONFIG_COMMANDS([gdbdepdir],[