[4/5] iconvdata: Move gconv-modules configuration to gconv-modules.conf

Message ID 20210607085221.43612-5-siddhesh@sourceware.org
State New
Headers show
Series
  • gconv module configuration improvements
Related show

Commit Message

Adhemerval Zanella via Libc-alpha June 7, 2021, 8:52 a.m.
Move all gconv-modules configuration files to gconv-modules.conf.
That is, the S390 extensions now become gconv-modules-s390.conf.  Move
both configuration files into gconv-modules.d.

Now GCONV_PATH/gconv-modules is read only for backward compatibility
for third-party gconv modules directories.
---
 iconvdata/Makefile                            | 46 +++++++++++--------
 .../{gconv-modules => gconv-modules.conf}     |  0
 localedata/Makefile                           |  4 +-
 sysdeps/s390/Makefile                         | 16 ++++++-
 ...{gconv-modules => gconv-modules-s390.conf} |  0
 5 files changed, 44 insertions(+), 22 deletions(-)
 rename iconvdata/{gconv-modules => gconv-modules.conf} (100%)
 rename sysdeps/s390/{gconv-modules => gconv-modules-s390.conf} (100%)

-- 
2.31.1

Comments

Adhemerval Zanella via Libc-alpha June 9, 2021, 1:33 a.m. | #1
Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:

> +gconv-modules = gconv-modules.conf

> +modpfx = $(objpfx)gconv-modules.d/


Ok

>  install-others	= $(addprefix $(inst_gconvdir)/, $(modules.so))	\

> -		  $(inst_gconvdir)/gconv-modules

> +		  $(addprefix $(inst_gconvdir)/gconv-modules.d/, \

> +			      $(gconv-modules))


from .../gconv-modules to .../gconv-modules.d/gconv-modules.conf, ok

>  ifdef objpfx

> -generated += gconv-modules

> +generated += $(addprefix gconv-modules.d/,$(gconv-modules))

>  endif


Same, ok

> -$(inst_gconvdir)/gconv-modules: $(objpfx)gconv-modules $(+force)

> +$(addprefix $(inst_gconvdir)/gconv-modules.d/, $(gconv-modules)): \

> +    $(inst_gconvdir)/gconv-modules.d/%: $(modpfx)% $(+force)

>  	$(do-install)


Ok.

> @@ -303,28 +307,29 @@ $(objpfx)mtrace-tst-loading.out: $(objpfx)tst-loading.out

>  	$(common-objpfx)malloc/mtrace $(objpfx)tst-loading.mtrace > $@; \

>  	$(evaluate-test)

>  

> -$(objpfx)bug-iconv1.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv1.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv2.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv2.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv3.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv3.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv5.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv5.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)tst-loading.out: $(objpfx)gconv-modules \

> +$(objpfx)tst-loading.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			  $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)tst-iconv4.out: $(objpfx)gconv-modules \

> +$(objpfx)tst-iconv4.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)tst-iconv7.out: $(objpfx)gconv-modules \

> +$(objpfx)tst-iconv7.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv10.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv10.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			  $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv12.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv12.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			  $(addprefix $(objpfx),$(modules.so))

> -$(objpfx)bug-iconv14.out: $(objpfx)gconv-modules \

> +$(objpfx)bug-iconv14.out: $(addprefix $(modpfx), $(gconv-modules)) \

>  			  $(addprefix $(objpfx),$(modules.so))


Ok.

> -$(objpfx)iconv-test.out: run-iconv-test.sh $(objpfx)gconv-modules \

> +$(objpfx)iconv-test.out: run-iconv-test.sh \

> +			 $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so)) \

>  			 $(common-objdir)/iconv/iconv_prog TESTS


Ok.

> -$(objpfx)tst-tables.out: tst-tables.sh $(objpfx)gconv-modules \

> +$(objpfx)tst-tables.out: tst-tables.sh \

> +			 $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so)) \

>  			 $(objpfx)tst-table-from $(objpfx)tst-table-to

>  	$(SHELL) $< $(common-objpfx) $(common-objpfx)iconvdata/ \


Ok.

> -$(objpfx)gconv-modules: gconv-modules

> -	cat $(sysdeps-gconv-modules) $^ > $@

> +$(modpfx):

> +	mkdir -p $@

> +

> +$(modpfx)%: % $(modpfx)

> +	cp $< $@


Ok.

>  # Test requires BIG5HKSCS.

> -$(objpfx)tst-iconv-big5-hkscs-to-2ucs4.out: $(objpfx)gconv-modules \

> +$(objpfx)tst-iconv-big5-hkscs-to-2ucs4.out: \

> +			 $(addprefix $(modpfx), $(gconv-modules)) \

>  			 $(addprefix $(objpfx),$(modules.so))


Ok.

> diff --git a/iconvdata/gconv-modules b/iconvdata/gconv-modules.conf

> similarity index 100%

> rename from iconvdata/gconv-modules

> rename to iconvdata/gconv-modules.conf


Ok.

> diff --git a/localedata/Makefile b/localedata/Makefile

>  

> -tests: $(objdir)/iconvdata/gconv-modules

> +tests: $(objdir)/iconvdata/gconv-modules.d/gconv-modules.conf


Ok.

> -$(objdir)/iconvdata/gconv-modules:

> +$(objdir)/iconvdata/gconv-modules.d/gconv-modules.conf:

>  	$(MAKE) -C ../iconvdata subdir=iconvdata $@


Ok.

> diff --git a/sysdeps/s390/Makefile b/sysdeps/s390/Makefile

> -install-others  += $(patsubst %, $(inst_gconvdir)/%.so, $(s390x-iconv-modules))

> +install-others  += $(patsubst %, $(inst_gconvdir)/%.so, \

> +				 $(s390x-iconv-modules)) \

> +		   $(inst_gconvdir)/gconv-modules.d/gconv-modules-s390.conf


Ok

>  $(patsubst %, $(inst_gconvdir)/%.so, $(s390x-iconv-modules)) : \

>  $(inst_gconvdir)/%.so: $(objpfx)%.so $(+force)

>  	$(do-install-program)

>  

> -sysdeps-gconv-modules = ../sysdeps/s390/gconv-modules

> +ifdef objpfx

> +generated += gconv-modules.d/gconv-modules-s390.conf

> +endif

> +

> +$(inst_gconvdir)/gconv-modules.d/gconv-modules-s390.conf: \

> +		$(modpfx)gconv-modules-s390.conf $(+force)

> +	$(do-install)

> +

> +$(modpfx)gconv-modules-s390.conf: ../sysdeps/s390/gconv-modules-s390.conf \

> +				  $(modpfx)

> +	cp $< $@

>  endif


Ok

> diff --git a/sysdeps/s390/gconv-modules b/sysdeps/s390/gconv-modules-s390.conf

> similarity index 100%

> rename from sysdeps/s390/gconv-modules

> rename to sysdeps/s390/gconv-modules-s390.conf


Ok

Reviewed-by: DJ Delorie <dj@redhat.com>
Andreas Schwab June 10, 2021, 10:23 a.m. | #2
On Jun 07 2021, Siddhesh Poyarekar via Libc-alpha wrote:

> Move all gconv-modules configuration files to gconv-modules.conf.

> That is, the S390 extensions now become gconv-modules-s390.conf.  Move

> both configuration files into gconv-modules.d.

>

> Now GCONV_PATH/gconv-modules is read only for backward compatibility

> for third-party gconv modules directories.


Why is that needed?  Why can't GCONV_PATH/gconv-modules remain the
master?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Adhemerval Zanella via Libc-alpha June 10, 2021, 11:11 a.m. | #3
On 6/10/21 3:53 PM, Andreas Schwab wrote:
> On Jun 07 2021, Siddhesh Poyarekar via Libc-alpha wrote:

> 

>> Move all gconv-modules configuration files to gconv-modules.conf.

>> That is, the S390 extensions now become gconv-modules-s390.conf.  Move

>> both configuration files into gconv-modules.d.

>>

>> Now GCONV_PATH/gconv-modules is read only for backward compatibility

>> for third-party gconv modules directories.

> 

> Why is that needed?  Why can't GCONV_PATH/gconv-modules remain the

> master?


I did it to move all configuration files into a single place in 
gconv-modules.d/ since that seemed more consistent than having 
GCONV_PATH/gconv-modules and 
GCONV_PATH/gconv-modules.d/gconv-modules-extra.conf.

Siddhesh
Andreas Schwab June 10, 2021, 11:24 a.m. | #4
On Jun 10 2021, Siddhesh Poyarekar wrote:

> I did it to move all configuration files into a single place in

> gconv-modules.d/ since that seemed more consistent than having 

> GCONV_PATH/gconv-modules and

> GCONV_PATH/gconv-modules.d/gconv-modules-extra.conf.


Why is that a problem?  After all, GCONV_PATH/gconv-modules is still
being read.  Usually, *.d is reserved for add-ons.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Adhemerval Zanella via Libc-alpha June 10, 2021, 11:28 a.m. | #5
On 6/10/21 4:54 PM, Andreas Schwab wrote:
> On Jun 10 2021, Siddhesh Poyarekar wrote:

> 

>> I did it to move all configuration files into a single place in

>> gconv-modules.d/ since that seemed more consistent than having

>> GCONV_PATH/gconv-modules and

>> GCONV_PATH/gconv-modules.d/gconv-modules-extra.conf.

> 

> Why is that a problem?  After all, GCONV_PATH/gconv-modules is still

> being read.  Usually, *.d is reserved for add-ons.


It was really just an aesthetic choice on my part.  How about:

GCONV_PATH/gconv-modules:
All of the core modules (UTF*, etc.)

GCONV_PATH/gconv-modules.d/gconv-modules-extra.conf:
All remaining modules

GCONV_PATH/gconv-modules.d/gconv-modules-s390.conf:
S390 modules

I'll post a patch to rejig them once we agree on the structure.

Siddhesh
Andreas Schwab June 10, 2021, 11:58 a.m. | #6
On Jun 10 2021, Siddhesh Poyarekar wrote:

> It was really just an aesthetic choice on my part.  How about:

>

> GCONV_PATH/gconv-modules:

> All of the core modules (UTF*, etc.)

>

> GCONV_PATH/gconv-modules.d/gconv-modules-extra.conf:

> All remaining modules

>

> GCONV_PATH/gconv-modules.d/gconv-modules-s390.conf:

> S390 modules


Looks ok.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Patch

diff --git a/iconvdata/Makefile b/iconvdata/Makefile
index 6eeb92f4b3..8925a684eb 100644
--- a/iconvdata/Makefile
+++ b/iconvdata/Makefile
@@ -137,10 +137,13 @@  charmaps = ../localedata/charmaps
 extra-modules-left := $(modules)
 include extra-module.mk
 
+gconv-modules = gconv-modules.conf
+modpfx = $(objpfx)gconv-modules.d/
 
 extra-objs	+= $(modules.so)
 install-others	= $(addprefix $(inst_gconvdir)/, $(modules.so))	\
-		  $(inst_gconvdir)/gconv-modules
+		  $(addprefix $(inst_gconvdir)/gconv-modules.d/, \
+			      $(gconv-modules))
 
 # We can build the conversion tables for numerous charsets automatically.
 
@@ -182,7 +185,7 @@  generated += $(generated-modules:=.h) $(generated-modules:=.stmp) \
 	     iconv-test.out iconv-rules tst-loading.mtrace	 \
 	     mtrace-tst-loading.out tst-tables.out iconv-test.xxx
 ifdef objpfx
-generated += gconv-modules
+generated += $(addprefix gconv-modules.d/,$(gconv-modules))
 endif
 
 # Rules to generate the headers.
@@ -250,7 +253,8 @@  headers: $(addprefix $(objpfx), $(generated-modules:=.h))
 $(addprefix $(inst_gconvdir)/, $(modules.so)): \
     $(inst_gconvdir)/%: $(objpfx)% $(+force)
 	$(do-install-program)
-$(inst_gconvdir)/gconv-modules: $(objpfx)gconv-modules $(+force)
+$(addprefix $(inst_gconvdir)/gconv-modules.d/, $(gconv-modules)): \
+    $(inst_gconvdir)/gconv-modules.d/%: $(modpfx)% $(+force)
 	$(do-install)
 ifeq (no,$(cross-compiling))
 # Update the $(prefix)/lib/gconv/gconv-modules.cache file. This is necessary
@@ -303,28 +307,29 @@  $(objpfx)mtrace-tst-loading.out: $(objpfx)tst-loading.out
 	$(common-objpfx)malloc/mtrace $(objpfx)tst-loading.mtrace > $@; \
 	$(evaluate-test)
 
-$(objpfx)bug-iconv1.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv1.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv2.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv2.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv3.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv3.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv5.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv5.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)tst-loading.out: $(objpfx)gconv-modules \
+$(objpfx)tst-loading.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			  $(addprefix $(objpfx),$(modules.so))
-$(objpfx)tst-iconv4.out: $(objpfx)gconv-modules \
+$(objpfx)tst-iconv4.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)tst-iconv7.out: $(objpfx)gconv-modules \
+$(objpfx)tst-iconv7.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv10.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv10.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			  $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv12.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv12.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			  $(addprefix $(objpfx),$(modules.so))
-$(objpfx)bug-iconv14.out: $(objpfx)gconv-modules \
+$(objpfx)bug-iconv14.out: $(addprefix $(modpfx), $(gconv-modules)) \
 			  $(addprefix $(objpfx),$(modules.so))
 
-$(objpfx)iconv-test.out: run-iconv-test.sh $(objpfx)gconv-modules \
+$(objpfx)iconv-test.out: run-iconv-test.sh \
+			 $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so)) \
 			 $(common-objdir)/iconv/iconv_prog TESTS
 	iconv_modules="$(modules)" \
@@ -332,7 +337,8 @@  $(objpfx)iconv-test.out: run-iconv-test.sh $(objpfx)gconv-modules \
 		 '$(run-program-env)' > $@; \
 	$(evaluate-test)
 
-$(objpfx)tst-tables.out: tst-tables.sh $(objpfx)gconv-modules \
+$(objpfx)tst-tables.out: tst-tables.sh \
+			 $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so)) \
 			 $(objpfx)tst-table-from $(objpfx)tst-table-to
 	$(SHELL) $< $(common-objpfx) $(common-objpfx)iconvdata/ \
@@ -345,9 +351,13 @@  do-tests-clean common-mostlyclean: tst-tables-clean
 tst-tables-clean:
 	-rm -f $(objpfx)tst-*.table $(objpfx)tst-EUC-TW.irreversible
 
-$(objpfx)gconv-modules: gconv-modules
-	cat $(sysdeps-gconv-modules) $^ > $@
+$(modpfx):
+	mkdir -p $@
+
+$(modpfx)%: % $(modpfx)
+	cp $< $@
 
 # Test requires BIG5HKSCS.
-$(objpfx)tst-iconv-big5-hkscs-to-2ucs4.out: $(objpfx)gconv-modules \
+$(objpfx)tst-iconv-big5-hkscs-to-2ucs4.out: \
+			 $(addprefix $(modpfx), $(gconv-modules)) \
 			 $(addprefix $(objpfx),$(modules.so))
diff --git a/iconvdata/gconv-modules b/iconvdata/gconv-modules.conf
similarity index 100%
rename from iconvdata/gconv-modules
rename to iconvdata/gconv-modules.conf
diff --git a/localedata/Makefile b/localedata/Makefile
index 14e04cd3c5..364d7c758d 100644
--- a/localedata/Makefile
+++ b/localedata/Makefile
@@ -183,7 +183,7 @@  install-others := $(addprefix $(inst_i18ndir)/, \
 			      $(locales))
 endif
 
-tests: $(objdir)/iconvdata/gconv-modules
+tests: $(objdir)/iconvdata/gconv-modules.d/gconv-modules.conf
 
 tests-static += tst-langinfo-newlocale-static tst-langinfo-setlocale-static
 
@@ -464,5 +464,5 @@  $(objpfx)mtrace-tst-leaks.out: $(objpfx)tst-leaks.out
 bug-setlocale1-ENV-only = LOCPATH=$(objpfx) LC_CTYPE=de_DE.UTF-8
 bug-setlocale1-static-ENV-only = $(bug-setlocale1-ENV-only)
 
-$(objdir)/iconvdata/gconv-modules:
+$(objdir)/iconvdata/gconv-modules.d/gconv-modules.conf:
 	$(MAKE) -C ../iconvdata subdir=iconvdata $@
diff --git a/sysdeps/s390/Makefile b/sysdeps/s390/Makefile
index a8c49c928f..ade8663218 100644
--- a/sysdeps/s390/Makefile
+++ b/sysdeps/s390/Makefile
@@ -21,13 +21,25 @@  lib := iconvdata
 include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
 
 extra-objs      += $(addsuffix .so, $(s390x-iconv-modules))
-install-others  += $(patsubst %, $(inst_gconvdir)/%.so, $(s390x-iconv-modules))
+install-others  += $(patsubst %, $(inst_gconvdir)/%.so, \
+				 $(s390x-iconv-modules)) \
+		   $(inst_gconvdir)/gconv-modules.d/gconv-modules-s390.conf
 
 $(patsubst %, $(inst_gconvdir)/%.so, $(s390x-iconv-modules)) : \
 $(inst_gconvdir)/%.so: $(objpfx)%.so $(+force)
 	$(do-install-program)
 
-sysdeps-gconv-modules = ../sysdeps/s390/gconv-modules
+ifdef objpfx
+generated += gconv-modules.d/gconv-modules-s390.conf
+endif
+
+$(inst_gconvdir)/gconv-modules.d/gconv-modules-s390.conf: \
+		$(modpfx)gconv-modules-s390.conf $(+force)
+	$(do-install)
+
+$(modpfx)gconv-modules-s390.conf: ../sysdeps/s390/gconv-modules-s390.conf \
+				  $(modpfx)
+	cp $< $@
 endif
 
 ifeq ($(subdir),elf)
diff --git a/sysdeps/s390/gconv-modules b/sysdeps/s390/gconv-modules-s390.conf
similarity index 100%
rename from sysdeps/s390/gconv-modules
rename to sysdeps/s390/gconv-modules-s390.conf