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

[Yao Qi <qiyaoltc at gmail dot com>]

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,

> @@ -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.

>  	$(SUBDIR_TARGET_OBS) \
>  	$(SUBDIR_GCC_COMPILE_OBS)



> @@ -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)

> 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.

>  		;;
>  *)
> -		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?

-- 
Yao (齐尧)