Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
On Mon, 12 Sep 2011 17:10:49 -0500
Donnie Berkholz wrote:
> On 23:58 Mon 12 Sep , Michał Górny wrote:
> > On Mon, 12 Sep 2011 16:00:20 -0500
> > Donnie Berkholz 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
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
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.
On 23:58 Mon 12 Sep , Michał Górny wrote:
> On Mon, 12 Sep 2011 16:00:20 -0500
> Donnie Berkholz 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
pgprcOSPCGwcn.pgp
Description: PGP signature
Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
On Mon, 12 Sep 2011 16:00:20 -0500
Donnie Berkholz 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
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
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 pgphwBqOBZT6Q.pgp Description: PGP signature
Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
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.
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
pgpMghPH2v6CX.pgp
Description: PGP signature
[gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
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
