[gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Michał Górny]

Right now, autotools-utils.eclass punts .la files only with
USE=-static-libs. We'd like to broaden the range of it and strip .la
files when they are not necessary for static linkage as well.

The following patch introduces an initial support for that. It assumes
that the .la file can be removed if the library is mentioned in any of
pkg-config files installed by the package, or if doesn't specify any
dependency libs nor linker flags.

The code would probably use some refactoring but we will handle that
after more testing (and possibly extensions) to the concept itself.
---
 autotools-utils.eclass |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/autotools-utils.eclass b/autotools-utils.eclass
index 7905d44..ce6613b 100644
--- a/autotools-utils.eclass
+++ b/autotools-utils.eclass
@@ -132,7 +132,7 @@ _check_build_dir() {
 }
 
 # @FUNCTION: remove_libtool_files
-# @USAGE: [all|none]
+# @USAGE: [all|only-not-required|none]
 # @DESCRIPTION:
 # Determines unnecessary libtool files (.la) and libtool static archives (.a)
 # and removes them from installation image.
@@ -145,11 +145,32 @@ _check_build_dir() {
 remove_libtool_files() {
 	debug-print-function ${FUNCNAME} "$@"
 
+	if [[ "$1" == 'only-not-required' ]]; then
+		local pc_libs=''
+
+		for arg in $(find "${D}" -name '*.pc' -exec sed -n -e 's;^Libs:;;p' {} +); do
+			if [[ ${arg} == -l* ]]; then
+				pc_libs="${pc_libs} lib${arg#-l}.la"
+			fi
+		done
+	fi
+
 	local f
 	for f in $(find "${D}" -type f -name '*.la'); do
 		# Keep only .la files with shouldnotlink=yes - likely plugins
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
 		if [[  "$1" == 'all' || -z ${shouldnotlink} ]]; then
+			if [[ "$1" == 'only-not-required' ]]; then
+				# remove .la files only when .pc files provide the libs
+				# already or they don't give any information
+				! has $(basename "${f}") ${pc_libs} \
+						&& [[ -n "$(sed -n \
+							-e "s/^dependency_libs='\(.*\)'$/\1/p" \
+							-e "s/^inherited_linker_flags='\(.*\)'$/\1/p" \
+							"${f}")" ]] \
+						&& continue
+			fi
+
 			if [[ "$1" != 'none' ]]; then
 				echo "Removing unnecessary ${f}"
 				rm -f "${f}"
@@ -246,7 +267,7 @@ autotools-utils_src_install() {
 
 	# Remove libtool files and unnecessary static libs
 	local args
-	has static-libs ${IUSE//+} && ! use static-libs || args='none'
+	has static-libs ${IUSE//+} && ! use static-libs || args='only-not-required'
 	remove_libtool_files ${args}
 }
 
-- 
1.7.6.1

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Donnie Berkholz]

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

On 21:57 Mon 12 Sep     , Michał Górny wrote:
> Right now, autotools-utils.eclass punts .la files only with
> USE=-static-libs. We'd like to broaden the range of it and strip .la
> files when they are not necessary for static linkage as well.
> 
> The following patch introduces an initial support for that. It assumes
> that the .la file can be removed if the library is mentioned in any of
> pkg-config files installed by the package, or if doesn't specify any
> dependency libs nor linker flags.

If I understand correctly, this will break for any packages that don't 
use pkg-config to link. The maintainers will manually need to add 
pkg-config calls to the ebuilds of anything that could statically link 
against a library using only libtool and not pkg-config. Is that 
accurate?

It might be worthwhile to add an easy way to force this argument on for 
every package for the purposes of testing, e.g. an environment variable.

>  # @FUNCTION: remove_libtool_files
> -# @USAGE: [all|none]
> +# @USAGE: [all|only-not-required|none]

Is there a way to document the arguments of eclass functions? You added 
the name of the arg but didn't describe its purpose or why anyone would 
want to use it.

On a semantic note, that argument name (only-not-required) doesn't make 
sense to me. I might do something more helpful like pkgconfig-duplicates 
instead.

> +	if [[ "$1" == 'only-not-required' ]]; then

This is way more quoting than you need within double brackets.

>  	local f
>  	for f in $(find "${D}" -type f -name '*.la'); do
>  		# Keep only .la files with shouldnotlink=yes - likely plugins
>  		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
>  		if [[  "$1" == 'all' || -z ${shouldnotlink} ]]; then
> +			if [[ "$1" == 'only-not-required' ]]; then

Is there a case where one of those arguments might be $2 but you'd still 
want to run this?

I feel like that shouldnotlink thing is really confusing the logic, 
because there's multiple nested tests for different values of $1 in here 
instead of just testing the args once at the top and setting variables.

> +				# remove .la files only when .pc files provide the libs
> +				# already or they don't give any information
> +				! has $(basename "${f}") ${pc_libs} \
> +						&& [[ -n "$(sed -n \

The comment says "or" but I see an "and" here.

> +							-e "s/^dependency_libs='\(.*\)'$/\1/p" \
> +							-e "s/^inherited_linker_flags='\(.*\)'$/\1/p" \
> +							"${f}")" ]] \
> +						&& continue
> +			fi
> +

-- 
Thanks,
Donnie

Donnie Berkholz
Council Member / Sr. Developer
Gentoo Linux
Blog: http://dberkholz.com

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Samuli Suominen]

On 09/13/2011 12:00 AM, Donnie Berkholz wrote:
> On 21:57 Mon 12 Sep     , Michał Górny wrote:
>> Right now, autotools-utils.eclass punts .la files only with
>> USE=-static-libs. We'd like to broaden the range of it and strip .la
>> files when they are not necessary for static linkage as well.
>>
>> The following patch introduces an initial support for that. It assumes
>> that the .la file can be removed if the library is mentioned in any of
>> pkg-config files installed by the package, or if doesn't specify any
>> dependency libs nor linker flags.
> 
> If I understand correctly, this will break for any packages that don't 
> use pkg-config to link. The maintainers will manually need to add 
> pkg-config calls to the ebuilds of anything that could statically link 
> against a library using only libtool and not pkg-config. Is that 
> accurate?

Yes, seems accurate.

I can think of 'export PKG_CONFIG="$($(tc-getPKG_CONFIG) --static)' or
something like 'export FOO_LIBS="$($(tc-getPKG_CONFIG) --libs --static
foo)"' to accomplish getting static flags from an ebuild using
toolchain-funcs.eclass if required.

Or they do it like lvm2 and cryptsetup at upstream level and add
support for statically linking the tools in the build-system.

The .la files are not helping packages not using libtool in any case,
for example, those using cmake as build-system.

And I've yet to see a real, in portage residing, example of where this
would really break anything and when I will, I'll gladly help migrating
it to the example mentioned above... Overall, corner cases that can be
easily worked around, yet punting the *harmful* .la files.

- Samuli

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Donnie Berkholz]

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

On 00:46 Tue 13 Sep     , Samuli Suominen wrote:
> > If I understand correctly, this will break for any packages that 
> > don't use pkg-config to link. The maintainers will manually need to 
> > add pkg-config calls to the ebuilds of anything that could 
> > statically link against a library using only libtool and not 
> > pkg-config. Is that accurate?
> 
> Yes, seems accurate.
> 
> I can think of 'export PKG_CONFIG="$($(tc-getPKG_CONFIG) --static)' or 
> something like 'export FOO_LIBS="$($(tc-getPKG_CONFIG) --libs --static 
> foo)"' to accomplish getting static flags from an ebuild using 
> toolchain-funcs.eclass if required.
> 
> Or they do it like lvm2 and cryptsetup at upstream level and add 
> support for statically linking the tools in the build-system.
> 
> The .la files are not helping packages not using libtool in any case, 
> for example, those using cmake as build-system.
> 
> And I've yet to see a real, in portage residing, example of where this 
> would really break anything and when I will, I'll gladly help 
> migrating it to the example mentioned above... Overall, corner cases 
> that can be easily worked around, yet punting the *harmful* .la files.

That's rather shocking. All it would take is trying to statically build 
a package not using pkg-config that links against anything X11-related 
(since all of them have .pc files).

It's probably more that "nobody" cares about static building than that 
there aren't packages that would break.

-- 
Thanks,
Donnie

Donnie Berkholz
Council Member / Sr. Developer
Gentoo Linux
Blog: http://dberkholz.com

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Michał Górny]

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

On Mon, 12 Sep 2011 16:00:20 -0500
Donnie Berkholz <dberkholz@gentoo.org> wrote:

> >  # @FUNCTION: remove_libtool_files
> > -# @USAGE: [all|none]
> > +# @USAGE: [all|only-not-required|none]
> 
> Is there a way to document the arguments of eclass functions? You
> added the name of the arg but didn't describe its purpose or why
> anyone would want to use it.
> 
> On a semantic note, that argument name (only-not-required) doesn't
> make sense to me. I might do something more helpful like
> pkgconfig-duplicates instead.

I thinked about 'as-needed' or sth like that. Maybe the new argument
should be (temporarily) not public instead?

> > +	if [[ "$1" == 'only-not-required' ]]; then
> 
> This is way more quoting than you need within double brackets.

It's nice visual quoting, just to match the others.

> >  	local f
> >  	for f in $(find "${D}" -type f -name '*.la'); do
> >  		# Keep only .la files with shouldnotlink=yes -
> > likely plugins local shouldnotlink=$(sed -ne
> > '/^shouldnotlink=yes$/p' "${f}") if [[  "$1" == 'all' || -z
> > ${shouldnotlink} ]]; then
> > +			if [[ "$1" == 'only-not-required' ]]; then
> 
> Is there a case where one of those arguments might be $2 but you'd
> still want to run this?

Er? What are you referring to?

> I feel like that shouldnotlink thing is really confusing the logic, 
> because there's multiple nested tests for different values of $1 in
> here instead of just testing the args once at the top and setting
> variables.

As mentioned earlier, the code needs to be refactored. First things
first, then we'll rewrite it to be nice and clean. I don't really want
to waste time doing this if we would need to rewrite it for more logic
in the future.

> > +				# remove .la files only when .pc
> > files provide the libs
> > +				# already or they don't give any
> > information
> > +				! has $(basename "${f}")
> > ${pc_libs} \
> > +						&& [[ -n "$(sed -n
> > \
> 
> The comment says "or" but I see an "and" here.

Because everything's negated here. Boolean magic :D.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Donnie Berkholz]

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

On 23:58 Mon 12 Sep     , Michał Górny wrote:
> On Mon, 12 Sep 2011 16:00:20 -0500
> Donnie Berkholz <dberkholz@gentoo.org> wrote:
> > >  	local f
> > >  	for f in $(find "${D}" -type f -name '*.la'); do
> > >  		# Keep only .la files with shouldnotlink=yes -
> > > likely plugins local shouldnotlink=$(sed -ne
> > > '/^shouldnotlink=yes$/p' "${f}") if [[  "$1" == 'all' || -z
> > > ${shouldnotlink} ]]; then
> > > +			if [[ "$1" == 'only-not-required' ]]; then
> > 
> > Is there a case where one of those arguments might be $2 but you'd
> > still want to run this?
> 
> Er? What are you referring to?

Two things.

1. This is only reached if shouldnotlink is false. That means it's only 
the things that you are already assuming are plugins, right? If so, why 
is this even done?

2. What happens if I call it with `remove_libtool_files all 
only-not-required`? Nobody ever does any checking of the # of args.

> > I feel like that shouldnotlink thing is really confusing the logic, 
> > because there's multiple nested tests for different values of $1 in 
> > here instead of just testing the args once at the top and setting 
> > variables.
> 
> As mentioned earlier, the code needs to be refactored. First things 
> first, then we'll rewrite it to be nice and clean. I don't really want 
> to waste time doing this if we would need to rewrite it for more logic 
> in the future.

I'd rewrite it once as soon as you get all the reviews and before 
committing. Rewrites tend to never happen, since it turns out that 
people have other things to do with their time.

> > > +				# remove .la files only when .pc
> > > files provide the libs
> > > +				# already or they don't give any
> > > information
> > > +				! has $(basename "${f}")
> > > ${pc_libs} \
> > > +						&& [[ -n "$(sed -n
> > > \
> > 
> > The comment says "or" but I see an "and" here.
> 
> Because everything's negated here. Boolean magic :D.

OK, got it. Stop writing confusing logic. =P

-- 
Thanks,
Donnie

Donnie Berkholz
Council Member / Sr. Developer
Gentoo Linux
Blog: http://dberkholz.com

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Samuli Suominen]

On 09/13/2011 12:51 AM, Donnie Berkholz wrote:
> On 00:46 Tue 13 Sep     , Samuli Suominen wrote:
>>> If I understand correctly, this will break for any packages that 
>>> don't use pkg-config to link. The maintainers will manually need to 
>>> add pkg-config calls to the ebuilds of anything that could 
>>> statically link against a library using only libtool and not 
>>> pkg-config. Is that accurate?
>>
>> Yes, seems accurate.
>>
>> I can think of 'export PKG_CONFIG="$($(tc-getPKG_CONFIG) --static)' or 
>> something like 'export FOO_LIBS="$($(tc-getPKG_CONFIG) --libs --static 
>> foo)"' to accomplish getting static flags from an ebuild using 
>> toolchain-funcs.eclass if required.
>>
>> Or they do it like lvm2 and cryptsetup at upstream level and add 
>> support for statically linking the tools in the build-system.
>>
>> The .la files are not helping packages not using libtool in any case, 
>> for example, those using cmake as build-system.
>>
>> And I've yet to see a real, in portage residing, example of where this 
>> would really break anything and when I will, I'll gladly help 
>> migrating it to the example mentioned above... Overall, corner cases 
>> that can be easily worked around, yet punting the *harmful* .la files.
> 
> That's rather shocking. All it would take is trying to statically build 
> a package not using pkg-config that links against anything X11-related 
> (since all of them have .pc files).

Those packages that have pkg-config file, like libX11, are meant to be
used through pkg-config, so the bug would be in the package not using
the .pc, not in the package lacking the .la

> It's probably more that "nobody" cares about static building than that 
> there aren't packages that would break.

I'm looking forward in catching those packages trying to link statically
to a package providing valid pkg-config file, yet not using it...

Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.

[Michał Górny]

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

On Mon, 12 Sep 2011 17:10:49 -0500
Donnie Berkholz <dberkholz@gentoo.org> wrote:

> On 23:58 Mon 12 Sep     , Michał Górny wrote:
> > On Mon, 12 Sep 2011 16:00:20 -0500
> > Donnie Berkholz <dberkholz@gentoo.org> wrote:
> > > >  	local f
> > > >  	for f in $(find "${D}" -type f -name '*.la'); do
> > > >  		# Keep only .la files with shouldnotlink=yes -
> > > > likely plugins local shouldnotlink=$(sed -ne
> > > > '/^shouldnotlink=yes$/p' "${f}") if [[  "$1" == 'all' || -z
> > > > ${shouldnotlink} ]]; then
> > > > +			if [[ "$1" == 'only-not-required' ]];
> > > > then
> > > 
> > > Is there a case where one of those arguments might be $2 but you'd
> > > still want to run this?
> > 
> > Er? What are you referring to?
> 
> Two things.
> 
> 1. This is only reached if shouldnotlink is false. That means it's
> only the things that you are already assuming are plugins, right? If
> so, why is this even done?

That simply means that we're never removing .la files for plugins
(right now) because plugin loaders may need them with shared linking.
The other case are regular libraries where .la files are removed as
described above.

> 2. What happens if I call it with `remove_libtool_files all 
> only-not-required`? Nobody ever does any checking of the # of args.

Will add.

> > > > +				# remove .la files only
> > > > when .pc files provide the libs
> > > > +				# already or they don't give
> > > > any information
> > > > +				! has $(basename "${f}")
> > > > ${pc_libs} \
> > > > +						&& [[ -n
> > > > "$(sed -n \
> > > 
> > > The comment says "or" but I see an "and" here.
> > 
> > Because everything's negated here. Boolean magic :D.
> 
> OK, got it. Stop writing confusing logic. =P

It's confusing because of that 'continue', I guess ;P.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

[gentoo-dev] [PATCH autotools-utils 1/9] Fix handling whitespace in filenames when looking for .la files.

[Michał Górny]

---
 eclass/autotools-utils.eclass |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index ad5ffea..8bc365d 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -146,7 +146,7 @@ remove_libtool_files() {
 	debug-print-function ${FUNCNAME} "$@"
 
 	local f
-	for f in $(find "${D}" -type f -name '*.la'); do
+	find "${D}" -type f -name '*.la' -print0 | while read -r -d '' f; do
 		# Keep only .la files with shouldnotlink=yes - likely plugins
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
 		if [[  "$1" == 'all' || -z ${shouldnotlink} ]]; then
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 2/9] Strip ${D} from removal message to shorten it.

[Michał Górny]

---
 eclass/autotools-utils.eclass |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 8bc365d..31d228b 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -151,7 +151,7 @@ remove_libtool_files() {
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
 		if [[  "$1" == 'all' || -z ${shouldnotlink} ]]; then
 			if [[ "$1" != 'none' ]]; then
-				einfo "Removing unnecessary ${f}"
+				einfo "Removing unnecessary ${f#${D%/}}"
 				rm -f "${f}"
 			fi
 		fi
@@ -159,7 +159,7 @@ remove_libtool_files() {
 		if [[ -n ${shouldnotlink} ]]; then
 			local remove=${f/%.la/.a}
 			[[ "${f}" != "${remove}" ]] || die 'regex sanity check failed'
-			einfo "Removing unnecessary ${remove}"
+			einfo "Removing unnecessary ${remove#${D%/}}"
 			rm -f "${remove}"
 		fi
 	done
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 3/9] For .la removal, look for static archives rather than USE=static-libs.

[Michał Górny]

---
 eclass/autotools-utils.eclass |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 31d228b..ab8650f 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -132,13 +132,13 @@ _check_build_dir() {
 }
 
 # @FUNCTION: remove_libtool_files
-# @USAGE: [all|none]
+# @USAGE: [all]
 # @DESCRIPTION:
 # Determines unnecessary libtool files (.la) and libtool static archives (.a)
 # and removes them from installation image.
+#
 # To unconditionally remove all libtool files, pass 'all' as argument.
-# To leave all libtool files alone, pass 'none' as argument.
-# Unnecessary static archives are removed in any case.
+# Otherwise, libtool archives required for static linking will be preserved.
 #
 # In most cases it's not necessary to manually invoke this function.
 # See autotools-utils_src_install for reference.
@@ -147,14 +147,17 @@ remove_libtool_files() {
 
 	local f
 	find "${D}" -type f -name '*.la' -print0 | while read -r -d '' f; do
-		# Keep only .la files with shouldnotlink=yes - likely plugins
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
-		if [[  "$1" == 'all' || -z ${shouldnotlink} ]]; then
-			if [[ "$1" != 'none' ]]; then
-				einfo "Removing unnecessary ${f#${D%/}}"
-				rm -f "${f}"
-			fi
+		local archivefile=${f/%.la/.a}
+
+		# Keep .la files when:
+		# - they have shouldnotlink=yes - likely plugins,
+		# - respective static archive exists.
+		if [[ "$1" == 'all' || ( -z ${shouldnotlink} && ! -f ${archivefile} ) ]]; then
+			einfo "Removing unnecessary ${f#${D%/}}"
+			rm -f "${f}"
 		fi
+
 		# Remove static libs we're not supposed to link against
 		if [[ -n ${shouldnotlink} ]]; then
 			local remove=${f/%.la/.a}
@@ -245,9 +248,7 @@ autotools-utils_src_install() {
 	popd > /dev/null
 
 	# Remove libtool files and unnecessary static libs
-	local args
-	has static-libs ${IUSE//+} && ! use static-libs || args='none'
-	remove_libtool_files ${args}
+	remove_libtool_files
 }
 
 # @FUNCTION: autotools-utils_src_test
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 4/9] Clean up & simplify la removal code a little.

[Michał Górny]

---
 eclass/autotools-utils.eclass |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index ab8650f..fd644bb 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -149,21 +149,20 @@ remove_libtool_files() {
 	find "${D}" -type f -name '*.la' -print0 | while read -r -d '' f; do
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
 		local archivefile=${f/%.la/.a}
+		[[ "${f}" != "${archivefile}" ]] || die 'regex sanity check failed'
 
 		# Keep .la files when:
 		# - they have shouldnotlink=yes - likely plugins,
 		# - respective static archive exists.
 		if [[ "$1" == 'all' || ( -z ${shouldnotlink} && ! -f ${archivefile} ) ]]; then
 			einfo "Removing unnecessary ${f#${D%/}}"
-			rm -f "${f}"
+			rm -f "${f}" || die
 		fi
 
 		# Remove static libs we're not supposed to link against
-		if [[ -n ${shouldnotlink} ]]; then
-			local remove=${f/%.la/.a}
-			[[ "${f}" != "${remove}" ]] || die 'regex sanity check failed'
-			einfo "Removing unnecessary ${remove#${D%/}}"
-			rm -f "${remove}"
+		if [[ ${shouldnotlink} ]]; then
+			einfo "Removing unnecessary ${archivefile#${D%/}}"
+			rm -f "${archivefile}" || die
 		fi
 	done
 }
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 5/9] Check command-line args completely in remove_libtool_files().

[Michał Górny]

---
 eclass/autotools-utils.eclass |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index fd644bb..84f6cb6 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -144,6 +144,17 @@ _check_build_dir() {
 # See autotools-utils_src_install for reference.
 remove_libtool_files() {
 	debug-print-function ${FUNCNAME} "$@"
+	local removing_all
+	[[ ${#} -le 1 ]] || die "Invalid number of args to ${FUNCNAME}()"
+	if [[ ${#} -eq 1 ]]; then
+		case "${1}" in
+			all)
+				removing_all=1
+				;;
+			*)
+				die "Invalid argument to ${FUNCNAME}(): ${1}"
+		esac
+	fi
 
 	local f
 	find "${D}" -type f -name '*.la' -print0 | while read -r -d '' f; do
@@ -154,7 +165,7 @@ remove_libtool_files() {
 		# Keep .la files when:
 		# - they have shouldnotlink=yes - likely plugins,
 		# - respective static archive exists.
-		if [[ "$1" == 'all' || ( -z ${shouldnotlink} && ! -f ${archivefile} ) ]]; then
+		if [[ ${removing_all} || ( -z ${shouldnotlink} && ! -f ${archivefile} ) ]]; then
 			einfo "Removing unnecessary ${f#${D%/}}"
 			rm -f "${f}" || die
 		fi
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 6/9] Refactor remove_libtool_files() to simplify conditions.

[Michał Górny]

---
 eclass/autotools-utils.eclass |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 84f6cb6..48b39cb 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -162,18 +162,20 @@ remove_libtool_files() {
 		local archivefile=${f/%.la/.a}
 		[[ "${f}" != "${archivefile}" ]] || die 'regex sanity check failed'
 
-		# Keep .la files when:
-		# - they have shouldnotlink=yes - likely plugins,
-		# - respective static archive exists.
-		if [[ ${removing_all} || ( -z ${shouldnotlink} && ! -f ${archivefile} ) ]]; then
-			einfo "Removing unnecessary ${f#${D%/}}"
-			rm -f "${f}" || die
-		fi
-
 		# Remove static libs we're not supposed to link against
 		if [[ ${shouldnotlink} ]]; then
 			einfo "Removing unnecessary ${archivefile#${D%/}}"
 			rm -f "${archivefile}" || die
+			# We're never going to remove the .la file.
+			[[ ${removing_all} ]] || continue
+		fi
+
+		# Keep .la files when:
+		# - they have shouldnotlink=yes - likely plugins (handled above),
+		# - respective static archive exists.
+		if [[ ${removing_all} || ! -f ${archivefile} ]]; then
+			einfo "Removing unnecessary ${f#${D%/}}"
+			rm -f "${f}" || die
 		fi
 	done
 }
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 7/9] Drop 'empty' .la files as well (those lacking libs & flags).

[Michał Górny]

---
 eclass/autotools-utils.eclass |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 48b39cb..9d7e134 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -162,18 +162,28 @@ remove_libtool_files() {
 		local archivefile=${f/%.la/.a}
 		[[ "${f}" != "${archivefile}" ]] || die 'regex sanity check failed'
 
-		# Remove static libs we're not supposed to link against
+		# Remove static libs we're not supposed to link against.
 		if [[ ${shouldnotlink} ]]; then
 			einfo "Removing unnecessary ${archivefile#${D%/}}"
 			rm -f "${archivefile}" || die
-			# We're never going to remove the .la file.
+			# The .la file may be used by a module loader, so avoid removing it
+			# unless explicitly requested.
 			[[ ${removing_all} ]] || continue
 		fi
 
-		# Keep .la files when:
-		# - they have shouldnotlink=yes - likely plugins (handled above),
-		# - respective static archive exists.
-		if [[ ${removing_all} || ! -f ${archivefile} ]]; then
+		# Remove .la files when:
+		# - user explicitly wants us to remove all .la files,
+		# - respective static archive doesn't exist,
+		# - they don't provide any new information (no libs & no flags).
+		local removing
+		if [[ ${removing_all} ]]; then removing=1
+		elif [[ ! -f ${archivefile} ]]; then removing=1
+		elif [[ ! $(sed -n -e \
+			"s/^\(dependency_libs\|inherited_linker_flags\)='\(.*\)'$/\2/p" \
+			"${f}") ]]; then removing=1
+		fi
+
+		if [[ ${removing} ]]; then
 			einfo "Removing unnecessary ${f#${D%/}}"
 			rm -f "${f}" || die
 		fi
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 8/9] Remove static libs covered by .pc files as well.

[Michał Górny]

---
 eclass/autotools-utils.eclass |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 9d7e134..2e01dcc 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -156,6 +156,15 @@ remove_libtool_files() {
 		esac
 	fi
 
+	local pc_libs=()
+	if [[ ! ${removing_all} ]]; then
+		local arg
+		for arg in $(find "${D}" -name '*.pc' -exec \
+					sed -n -e 's;^Libs:;;p' {} +); do
+			[[ ${arg} == -l* ]] && pc_libs+=(lib${arg#-l}.la)
+		done
+	fi
+
 	local f
 	find "${D}" -type f -name '*.la' -print0 | while read -r -d '' f; do
 		local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
@@ -174,10 +183,12 @@ remove_libtool_files() {
 		# Remove .la files when:
 		# - user explicitly wants us to remove all .la files,
 		# - respective static archive doesn't exist,
+		# - they are covered by a .pc file already,
 		# - they don't provide any new information (no libs & no flags).
 		local removing
 		if [[ ${removing_all} ]]; then removing=1
 		elif [[ ! -f ${archivefile} ]]; then removing=1
+		elif has "$(basename "${f}")" "${pc_libs[@]}"; then removing=1
 		elif [[ ! $(sed -n -e \
 			"s/^\(dependency_libs\|inherited_linker_flags\)='\(.*\)'$/\2/p" \
 			"${f}") ]]; then removing=1
-- 
1.7.6.1

[gentoo-dev] [PATCH autotools-utils 9/9] Explain .la removal reasons in output.

[Michał Górny]

---
 eclass/autotools-utils.eclass |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/eclass/autotools-utils.eclass b/eclass/autotools-utils.eclass
index 2e01dcc..495244b 100644
--- a/eclass/autotools-utils.eclass
+++ b/eclass/autotools-utils.eclass
@@ -186,16 +186,17 @@ remove_libtool_files() {
 		# - they are covered by a .pc file already,
 		# - they don't provide any new information (no libs & no flags).
 		local removing
-		if [[ ${removing_all} ]]; then removing=1
-		elif [[ ! -f ${archivefile} ]]; then removing=1
-		elif has "$(basename "${f}")" "${pc_libs[@]}"; then removing=1
+		if [[ ${removing_all} ]]; then removing='forced'
+		elif [[ ! -f ${archivefile} ]]; then removing='no static archive'
+		elif has "$(basename "${f}")" "${pc_libs[@]}"; then
+			removing='covered by .pc'
 		elif [[ ! $(sed -n -e \
 			"s/^\(dependency_libs\|inherited_linker_flags\)='\(.*\)'$/\2/p" \
-			"${f}") ]]; then removing=1
+			"${f}") ]]; then removing='no libs & flags'
 		fi
 
 		if [[ ${removing} ]]; then
-			einfo "Removing unnecessary ${f#${D%/}}"
+			einfo "Removing unnecessary ${f#${D%/}} (${removing})"
 			rm -f "${f}" || die
 		fi
 	done
-- 
1.7.6.1

Re: [gentoo-dev] [PATCH autotools-utils 1/9] Fix handling whitespace in filenames when looking for .la files.

[Dirkjan Ochtman]

2011/9/13 Michał Górny <mgorny@gentoo.org>:
> ---
>  eclass/autotools-utils.eclass |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

I don't think sending 9 patches is very useful for this mailing list.
Next time just sent a link to a git repo or something?

Cheers,

Dirkjan

Re: [gentoo-dev] [PATCH autotools-utils 1/9] Fix handling whitespace in filenames when looking for .la files.

[Nirbheek Chauhan]

On Tue, Sep 13, 2011 at 8:43 PM, Dirkjan Ochtman <djc@gentoo.org> wrote:
> 2011/9/13 Michał Górny <mgorny@gentoo.org>:
>> ---
>>  eclass/autotools-utils.eclass |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> I don't think sending 9 patches is very useful for this mailing list.
> Next time just sent a link to a git repo or something?
>

On the contrary, I like that mgorny sent separate completely
independent patches for review to the list instead of either sending
on huge chunk, or not sending patches at all.

-- 
~Nirbheek Chauhan

Gentoo GNOME+Mozilla Team

Re: [gentoo-dev] [PATCH autotools-utils 1/9] Fix handling whitespace in filenames when looking for .la files.

[Michał Górny]

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

On Tue, 13 Sep 2011 17:13:11 +0200
Dirkjan Ochtman <djc@gentoo.org> wrote:

> 2011/9/13 Michał Górny <mgorny@gentoo.org>:
> > ---
> >  eclass/autotools-utils.eclass |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> I don't think sending 9 patches is very useful for this mailing list.
> Next time just sent a link to a git repo or something?

Erm, maybe I should've attached a complete diff as well, or complete
updated eclass.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] [PATCH autotools-utils 1/9] Fix handling whitespace in filenames when looking for .la files.

[Donnie Berkholz]

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

On 21:53 Tue 13 Sep     , Nirbheek Chauhan wrote:
> On Tue, Sep 13, 2011 at 8:43 PM, Dirkjan Ochtman <djc@gentoo.org> wrote:
> > 2011/9/13 Michał Górny <mgorny@gentoo.org>:
> >> ---
> >>  eclass/autotools-utils.eclass |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > I don't think sending 9 patches is very useful for this mailing list.
> > Next time just sent a link to a git repo or something?
> >
> 
> On the contrary, I like that mgorny sent separate completely
> independent patches for review to the list instead of either sending
> on huge chunk, or not sending patches at all.

+1, except parts of them were dependent. =) Maybe use some `git rebase 
--interactive` next time..

-- 
Thanks,
Donnie

Donnie Berkholz
Council Member / Sr. Developer
Gentoo Linux
Blog: http://dberkholz.com

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]