Re: [PATCH roland/Versions.def] Get rid of Versions.def source file

["Carlos O'Donell" <carlos at redhat dot com>]

On 02/28/2014 04:45 PM, Roland McGrath wrote:
> This is pretty much the minimal change to get rid of the hand-maintained
> Versions.def file.  It just generates a file that (nearly) matches what we
> had checked in.  At some point we should probably revamp the generation of
> the version maps et al more thoroughly.  It's really rather byzantine now.
> 
> This would have caught the particular motivating instance, but it won't
> necessarily catch all such cases.  That is, if, say glibc-2.42 made some
> symbols obsolete for link-time with:
> 	#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_42)
> but no GLIBC_2.42 version set were added to any Versions file--i.e. a
> release where we obsolete a symbol but do not add any new symbols--then the
> same scenario that broke us recently would happen again.  What would catch
> that is -Werror=undef, which we will get to separately.  The fix then would
> be to just add:
> 	libc {
> 	  GLIBC_2.42 {
> 	  }
> 	}
> to some Versions file, rather than adding it to Versions.def.

That's certainly better than where we are now.

However, the scenario you describe is quite likely, so I better get on
the fixes for -Wundef.
 
> I've tested the build and 'make check-abi' on x86_64-linux-gnu.  Proper
> paranoid testing would make sure that none of the installed files changes
> at all (binaries, gnu/lib-names.h, etc.).

I've built a pristine glibc, and a glibc with this patch.

Installed both and compared.

No real differences in installed files.

I had to disable build-id's and use hexdump to rule out timestamp
changes in executables. The static archives had only differences
in their archive names because of the different install paths.

I also compared Versions.def.v and Versions.def, and I must admit
I like the compactness of the auto-generated version, which is
expected.

I expect that this is now sufficiently paranoid.

> 2014-02-28  Roland McGrath  <roland@hack.frob.com>
> 
> 	* scripts/versionlist.awk: New file.
> 	* Makerules [$(build-shared) = yes]
> 	(postclean-generated): Add Versions.def, not Versions.def.v and
> 	Versions.def.v.i.
> 	($(common-objpfx)Versions.def.v.i): Target removed.
> 	($(common-objpfx)Versions.def): New target.
> 	($(common-objpfx)Versions.all): Depend on that rather that
> 	$(common-objpfx)Versions.def.v.
> 	* Versions.def: File removed.

OK to checkin.

> --- a/Makerules
> +++ b/Makerules
> @@ -288,19 +288,22 @@ ifeq ($(build-shared),yes)
>  $(addprefix $(common-objpfx),$(version-maps)): $(common-objpfx)sysd-versions
>  common-generated += $(version-maps)
>  postclean-generated += sysd-versions Versions.all abi-versions.h \
> -		       Versions.def.v.i Versions.def.v Versions.v.i Versions.v
> +		       Versions.def Versions.v.i Versions.v

OK. Now Versions.def is auto-generated.

>  
>  ifndef avoid-generated
>  ifneq ($(sysd-versions-subdirs),$(sorted-subdirs) $(config-sysdirs))
>  sysd-versions-force = FORCE
>  FORCE:
>  endif
> -# See %.v/%.v.i implicit rules in Makeconfig.
> -$(common-objpfx)Versions.def.v.i: $(..)Versions.def \
> -				  $(wildcard $(add-ons:%=$(..)%/Versions.def))
> +
> +$(common-objpfx)Versions.def: $(..)scripts/versionlist.awk \
> +			      $(common-objpfx)Versions.v
> +	LC_ALL=C $(AWK) -f $^ > $@T
> +	mv -f $@T $@

OK.

> +
>  $(common-objpfx)Versions.all: $(..)scripts/firstversions.awk \
>  			      $(common-objpfx)soversions.i \
> -			      $(common-objpfx)Versions.def.v
> +			      $(common-objpfx)Versions.def

OK.

>  	{ while read which lib version setname; do \
>  	    test x"$$which" = xDEFAULT || continue; \
>  	    test -z "$$setname" || echo "$$lib : $$setname"; \
> --- a/Versions.def
> +++ /dev/null
> @@ -1,151 +0,0 @@
> -libBrokenLocale {
> -  GLIBC_2.0
> -}
> -libc {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.1.1
> -  GLIBC_2.1.2
> -  GLIBC_2.1.3
> -  GLIBC_2.1.4
> -  GLIBC_2.2
> -  GLIBC_2.2.1
> -  GLIBC_2.2.2
> -  GLIBC_2.2.3
> -  GLIBC_2.2.4
> -  GLIBC_2.2.5
> -  GLIBC_2.2.6
> -  GLIBC_2.3
> -  GLIBC_2.3.1
> -  GLIBC_2.3.2
> -  GLIBC_2.3.3
> -  GLIBC_2.3.4
> -  GLIBC_2.4
> -  GLIBC_2.5
> -  GLIBC_2.6
> -  GLIBC_2.7
> -  GLIBC_2.8
> -  GLIBC_2.9
> -  GLIBC_2.10
> -  GLIBC_2.11
> -  GLIBC_2.12
> -  GLIBC_2.13
> -  GLIBC_2.14
> -  GLIBC_2.15
> -  GLIBC_2.16
> -  GLIBC_2.17
> -  GLIBC_2.18
> -  GLIBC_2.19
> -  HURD_CTHREADS_0.3
> -%ifdef EXPORT_UNWIND_FIND_FDE
> -  GCC_3.0
> -%endif
> -  GLIBC_PRIVATE
> -}
> -libcrypt {
> -  GLIBC_2.0
> -}
> -libdl {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.3.3
> -  GLIBC_2.3.4
> -}
> -libm {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.2
> -  GLIBC_2.2.3
> -  GLIBC_2.3
> -  GLIBC_2.3.4
> -  GLIBC_2.4
> -  GLIBC_2.15
> -  GLIBC_2.18
> -  GLIBC_2.19
> -}
> -libnsl {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.2
> -}
> -libnss_compat {
> -  GLIBC_PRIVATE
> -}
> -libnss_dns {
> -  GLIBC_PRIVATE
> -}
> -libnss_db {
> -  GLIBC_PRIVATE
> -}
> -libnss_files {
> -  GLIBC_PRIVATE
> -}
> -libnss_hesiod {
> -  GLIBC_PRIVATE
> -}
> -libnss_nis {
> -  GLIBC_PRIVATE
> -}
> -libnss_nisplus {
> -  GLIBC_PRIVATE
> -}
> -libpthread {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.1.1
> -  GLIBC_2.1.2
> -  GLIBC_2.2
> -  GLIBC_2.2.3
> -  GLIBC_2.2.6
> -  GLIBC_2.3
> -  GLIBC_2.3.2
> -  GLIBC_2.3.3
> -  GLIBC_2.3.4
> -  GLIBC_2.4
> -  GLIBC_2.6
> -  GLIBC_2.11
> -  GLIBC_2.12
> -  GLIBC_2.18
> -  GLIBC_2.19
> -  GLIBC_PRIVATE
> -}
> -libresolv {
> -  GLIBC_2.0
> -  GLIBC_2.2
> -  GLIBC_2.3.2
> -  GLIBC_2.9
> -  GLIBC_PRIVATE
> -}
> -librt {
> -  GLIBC_2.1
> -  GLIBC_2.2
> -  GLIBC_2.3
> -  GLIBC_2.3.3
> -  GLIBC_2.3.4
> -  GLIBC_2.4
> -  GLIBC_2.7
> -  GLIBC_2.17
> -}
> -libutil {
> -  GLIBC_2.0
> -  GLIBC_2.9
> -}
> -ld {
> -  GLIBC_2.0
> -  GLIBC_2.1
> -  GLIBC_2.3
> -  GLIBC_2.4
> -  GLIBC_PRIVATE
> -}
> -libthread_db {
> -  GLIBC_2.1.3
> -  GLIBC_2.2.3
> -  GLIBC_2.3
> -  GLIBC_2.3.3
> -}
> -libanl {
> -  GLIBC_2.2.3
> -}
> -libcidn {
> -  GLIBC_PRIVATE
> -}

OK.

> --- /dev/null
> +++ b/scripts/versionlist.awk
> @@ -0,0 +1,39 @@
> +# Extract ordered list of version sets from Versions files.
> +# Copyright (C) 2014 Free Software Foundation, Inc.
> +
> +BEGIN { in_lib = ""; in_version = 0 }

OK. Initialize vars.

> +
> +!in_lib && NF == 2 && $2 == "{" { in_lib = $1; next }

OK. If we are not parsing a library description and we have
two fields and the second field is "{" then we are entering a
library and save the library name in in_lib as $1 (first column).
That seems to hold true for all Versions files and is the
expected format.

I thought this broke with "# {" comment, but then remembered
this works on Version.v after comment removal.

> +!in_lib { next }

OK. Skip lines unless we are in a lib e.g. skip empty lines
or skip lines that didn't match the first pattern which
sets in_lib.

> +
> +NF == 2 && $2 == "{" {
> +  in_version = 1;
> +  libs[in_lib] = libs[in_lib] "  " $1 "\n";
> +  lib_versions[in_lib, $1] = 1;
> +  all_versions[$1] = 1;
> +  next
> +}

OK. If we are on a version line (the lib line was skipped
by the pattern rule above) store the version with the lib
and in two other arrays for indexing.

> +
> +in_version && $1 == "}" { in_version = 0; next }

OK. Terminates version processing.

> +in_version { next }

OK. Walks through the version lines.

> +
> +$1 == "}" { in_lib = ""; next }

OK. Terminates library processing.

> +
> +END {
> +  nlibs = asorti(libs, libs_order);
> +  for (i = 1; i <= nlibs; ++i) {
> +    lib = libs_order[i];
> +
> +    for (v in all_versions) {
> +      if (!((in_lib, v) in lib_versions)) {
> +        libs[lib] = libs[lib] "  " v "\n";
> +      }
> +    }
> +
> +    print lib, "{";
> +    sort = "sort -u -t. -k 1,1 -k 2n,2n -k 3";
> +    printf "%s", libs[lib] | sort;
> +    close(sort);
> +    print "}";
> +  }
> +}
> 

OK.

Cheers,
Carlos.