[pacman-dev] Provides generation for files (a.k.a. rpm fileattrs for makepkg)

Message ID 20200308200419.38739-2-uhhadd@gmail.com
State Rejected, archived
Headers show
Series [pacman-dev] Provides generation for files (a.k.a. rpm fileattrs for makepkg) | expand

Commit Message

Carson Black March 8, 2020, 8:04 p.m. UTC
Whenever I'm working in a relatively fresh Arch environment, my work
flow looks like this:

- cmake .. -DFLAGS=blahblahblah
- Wait a few seconds
- Package providing KF5whatever not found
- Try and deduce Arch package name from CMake package name

Often, trying to figure out the relation between the build system package names
and the Arch package names of missing dependencies ends up being the longest
part of whatever task I was working on, which isn't very efficient.

Compare this to me working in an RPM distro:

- cmake .. -DFLAGS=blahblahblah
- Wait a few seconds
- Package providing KF5whatever not found
- dnf install cmake(KF5whatever)

The latter workflow is a lot more efficient, and is exactly what this
commit introduces to packages generated by makepkg.

Every file is iterated over at the end of the build process to generate
additional provides without the packager needing to manually specify
them.

The code is architected in a manner designed to make it trivial to add
new provider autogenerators.

So far, there are autogenerated providers for:
- pkgconfig(package)
- cmake(package)
- desktop files
  * app(desktopfilename)
  * can-open-mimetype(mimetype)

While these automatically generated provides can be used for packaging,
this is intended more for interactive usage rather than an attempt to
change how Arch packaging works.

Signed-off-by: Carson Black <uhhadd@gmail.com>
---
 scripts/makepkg.sh.in | 71 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

Comments

Eli Schwartz March 8, 2020, 9:46 p.m. UTC | #1
On 3/8/20 4:04 PM, Carson Black wrote:
> Whenever I'm working in a relatively fresh Arch environment, my work
> flow looks like this:
> 
> - cmake .. -DFLAGS=blahblahblah
> - Wait a few seconds
> - Package providing KF5whatever not found
> - Try and deduce Arch package name from CMake package name
> 
> Often, trying to figure out the relation between the build system package names
> and the Arch package names of missing dependencies ends up being the longest
> part of whatever task I was working on, which isn't very efficient.
> 
> Compare this to me working in an RPM distro:
> 
> - cmake .. -DFLAGS=blahblahblah
> - Wait a few seconds
> - Package providing KF5whatever not found
> - dnf install cmake(KF5whatever)
> 
> The latter workflow is a lot more efficient, and is exactly what this
> commit introduces to packages generated by makepkg.
> 
> Every file is iterated over at the end of the build process to generate
> additional provides without the packager needing to manually specify
> them.
> 
> The code is architected in a manner designed to make it trivial to add
> new provider autogenerators.
> 
> So far, there are autogenerated providers for:
> - pkgconfig(package)
> - cmake(package)
> - desktop files
>   * app(desktopfilename)
>   * can-open-mimetype(mimetype)>
> While these automatically generated provides can be used for packaging,
> this is intended more for interactive usage rather than an attempt to
> change how Arch packaging works.

Did you actually try this?

It's worth noting that makepkg forbids depends/provides names that
contain characters outside of the following permitted range:

[[:alnum:]+_.@-]

So if I added this to an existing PKGBUILD:

depends=("cmake(KF5Foo)")

then run

$ makepkg --printsrcinfo
==> ERROR: depends contains invalid characters: '()'

If the idea is to use these in PKGBUILDs, then we would need an update
to libmakepkg/lint_pkgbuild/pkgname.sh.in

So if this patch were to be accepted as-is, one would still NOT be able
to use the automatically generated provides for packaging.

And it feels very wrong to generate provides metadata that is forbidden
from being used.

> Signed-off-by: Carson Black <uhhadd@gmail.com>
> ---
>  scripts/makepkg.sh.in | 71 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index ca3e7459..65a3648e 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -57,6 +57,7 @@ CLEANUP=0
>  DEP_BIN=0
>  FORCE=0
>  GENINTEG=0
> +GENPROVIDES=1
>  HOLDVER=0
>  IGNOREARCH=0
>  INFAKEROOT=0
> @@ -520,6 +521,70 @@ find_libdepends() {
>  	(( ${#libdepends[@]} )) && printf '%s\n' "${libdepends[@]}"
>  }
>  
> +pkgconfig_prov() {
> +	case $1 in
> +		*.pc)
> +			directory="`dirname ${1}`"

As I mentioned before, it's bad practice to use the deprecated ``
syntax, modern shellscripts should always use $()

> +			export PKG_CONFIG_PATH="$DIR:$DIR/../../share/pkgconfig"
> +			pkg-config --print-provides "$1" 2>/dev/null | while read name _ _; do

What is the "$DIR" variable?

Why are you exporting PKG_CONFIG_PATH instead of defining it only for
the pkg-config execution?

Why is stderr being redirected to /dev/null? pkg-config exits 1 without
output, if a nonexistent dependency is specified, so the while loop will
exit without running the "do" section even once..

pkg-config is part of archlinux's base-devel group, but pacman isn't
archlinux exclusive. If you want to add a dependency on an external
program, it should at least be documented in the beginning of makepkg.sh.in:

# makepkg uses quite a few external programs during its execution. You
# need to have at least the following installed for makepkg to function:

And it might be a good idea to add a check in libmakepkg/executable/ as
well.

> +				[ -n "$name" ] || continue

When will this be empty?

> +				echo "pkgconfig($name)"
> +			done
> +			;;
> +	esac
> +}
> +
> +cmake_prov() {
> +	case $1 in
> +		*.cmake)
> +			base="$(basename $1)"
> +			case $1 in
> +				*Config.cmake)
> +					echo "cmake(${base%Config.cmake})"
> +					;;
> +				*-config.cmake)
> +					echo "cmake(${base%-config.cmake})"
> +					;;
> +			esac
> +			;;
> +	esac
> +}
> +
> +desktop_prov() {
> +	case "$1" in
> +		*.desktop)
> +			grep -q "Type=Application" "$1" || return
> +			grep -q "Exec=" "$1" || return

Why do either of these conditions matter if the file provides a
mimetype? Surely that check alone is sufficient?

> +			base="$(basename $1)"
> +			echo "app(${base%.desktop})"
> +
> +			mapfile -t -d ";" mimetypes < <(grep MimeType= $1 | cut -d '=' -f 2)
> +			for mimetype in "${mimetypes[@]}"; do
> +				cleaned=$(echo $mimetype | xargs)

What is "cleaned", and how is the xargs program "cleaning" it?

> +
> +				[[ -z "$cleaned" ]] && continue

When will this be set but empty?

> +				echo "can-open-mimetype($cleaned)"
> +			done
> +			;;
> +	esac
> +}
> +
> +providers=(
> +	pkgconfig cmake desktop
> +)
> +
> +find_fileattrs_provides() {
> +	local files
> +
> +	mapfile -t files < <(find "$pkgdir" -type f)

Instead of storing it in a "files" variable first, I'd actually
recommend using a while loop...

> +
> +	for file in "${files[@]}"; do
> +		for function in "${providers[@]}"; do
> +			"$function"_prov "$file"
> +		done
> +	done
> +}
>  
>  find_libprovides() {
>  	local p libprovides missing
> @@ -612,12 +677,14 @@ write_pkginfo() {
>  
>  	mapfile -t provides < <(find_libprovides)
>  	mapfile -t depends < <(find_libdepends)
> +	(( "$GENPROVIDES" != 0 )) && mapfile -t generated_provides < <(find_fileattrs_provides)
>  
>  	write_kv_pair "license"     "${license[@]}"
>  	write_kv_pair "replaces"    "${replaces[@]}"
>  	write_kv_pair "group"       "${groups[@]}"
>  	write_kv_pair "conflict"    "${conflicts[@]}"
>  	write_kv_pair "provides"    "${provides[@]}"
> +	write_kv_pair "provides"    "${generated_provides[@]}"
>  	write_kv_pair "backup"      "${backup[@]}"
>  	write_kv_pair "depend"      "${depends[@]}"
>  	write_kv_pair "optdepend"   "${optdepends[@]//+([[:space:]])/ }"
> @@ -980,6 +1047,7 @@ usage() {
>  	printf -- "$(gettext "  --nocheck        Do not run the %s function in the %s")\n" "check()" "$BUILDSCRIPT"
>  	printf -- "$(gettext "  --noprepare      Do not run the %s function in the %s")\n" "prepare()" "$BUILDSCRIPT"
>  	printf -- "$(gettext "  --nosign         Do not create a signature for the package")\n"
> +	printf -- "$(gettext "  --noprovidesgen  Do not autogenerate provides")\n"
>  	printf -- "$(gettext "  --packagelist    Only list package filepaths that would be produced")\n"
>  	printf -- "$(gettext "  --printsrcinfo   Print the generated SRCINFO and exit")\n"
>  	printf -- "$(gettext "  --sign           Sign the resulting package with %s")\n" "gpg"
> @@ -1029,7 +1097,7 @@ OPT_LONG=('allsource' 'check' 'clean' 'cleanbuild' 'config:' 'force' 'geninteg'
>            'help' 'holdver' 'ignorearch' 'install' 'key:' 'log' 'noarchive' 'nobuild'
>            'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare' 'nosign' 'packagelist'
>            'printsrcinfo' 'repackage' 'rmdeps' 'sign' 'skipchecksums' 'skipinteg'
> -          'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version')
> +          'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version' 'noprovidesgen')
>  
>  # Pacman Options
>  OPT_LONG+=('asdeps' 'noconfirm' 'needed' 'noprogressbar')
> @@ -1070,6 +1138,7 @@ while true; do
>  		--nocheck)        RUN_CHECK='n' ;;
>  		--noprepare)      RUN_PREPARE='n' ;;
>  		--nosign)         SIGNPKG='n' ;;
> +		--noprovidesgen)  GENPROVIDES=0 ;;

This is unreproducible, the public metadata of the package now depends
on command-line options.

Also, no official repository packages would be built with manual
command-line options, so you'd never be able to use the results. How
does this help you if it only applies to packages you personally build?

>  		-o|--nobuild)     BUILDPKG=0 NOBUILD=1 ;;
>  		-p)               shift; BUILDFILE=$1 ;;
>  		--packagelist)    BUILDPKG=0 PACKAGELIST=1 IGNOREARCH=1;;
>
Carson Black March 8, 2020, 10:39 p.m. UTC | #2
> Seems more or less "efficient" to me, but I'm not even sure why that's a
> goal. package dependencies are created once, so the focus should be less
> on how *fast* you can pacman -S 'the-dependency' and more on the
> technical merits of conveying the information that way. And I'm
> unconvinced that automatically generating "reasons" to install a package
> is the right way to go here.

Autogenerating provides for packages is a sensible manner of conveying package
independent and filesystem independent information about what a
repository provides
in terms of capabilities rather than raw package and repository names.
And unlike
Appstream metadata which is awkwardly handled across various package managers,
automatically generated provides can piggyback off of the already-existing
provides infrastructure most package managers have.

> > +                     for mimetype in "${mimetypes[@]}"; do
> > +                             cleaned=$(echo $mimetype | xargs)
>
> What is "cleaned", and how is the xargs program "cleaning" it?

Excess whitespace is being trimmed using xargs. "   a string
" will get normalized to "a string" when passed through xargs.

>
> > +
> > +                             [[ -z "$cleaned" ]] && continue
>
> When will this be set but empty?

Trailing semicolon in desktop file will result in a blank entry in the
mimetypes array,
resulting in a blank can-open-mimetype() provide without this code.

> Did you actually try this?

Yes, I've been using the following packages as tests for this and
inspecting their metadata by hand:

- vte3 (pkgconfig)
- kwayland (cmake)
- discover (desktop file)

I know pacman can already install packages given a provides capability,
so I didn't bother setting up a local test repository to confirm the
fact that pacman
can install these packages given their capabilities.
Eli Schwartz March 8, 2020, 10:49 p.m. UTC | #3
On 3/8/20 6:39 PM, Carson Black wrote:
>> What is "cleaned", and how is the xargs program "cleaning" it?
> 
> Excess whitespace is being trimmed using xargs. "   a string
> " will get normalized to "a string" when passed through xargs.

Examples of desktop files with this issue? I didn't find one like it on
my system.

>>
>>> +
>>> +                             [[ -z "$cleaned" ]] && continue
>>
>> When will this be set but empty?
> 
> Trailing semicolon in desktop file will result in a blank entry in the
> mimetypes array,
> resulting in a blank can-open-mimetype() provide without this code.

$ mapfile -t -d ";" mimetypes < <(grep MimeType=
/usr/share/applications/qemu.desktop | cut -d '=' -f 2)
$ declare -p mimetypes
declare -a mimetypes=()

You're wrong.

>> Did you actually try this?
> 
> Yes, I've been using the following packages as tests for this and
> inspecting their metadata by hand:
> 
> - vte3 (pkgconfig)
> - kwayland (cmake)
> - discover (desktop file)
> 
> I know pacman can already install packages given a provides capability,
> so I didn't bother setting up a local test repository to confirm the
> fact that pacman
> can install these packages given their capabilities.

You're not reading what I said.
Carson Black March 8, 2020, 10:54 p.m. UTC | #4
> > > Did you actually try this?
> >
> > Yes, I've been using the following packages as tests for this and
> > inspecting their metadata by hand:
> >
> > - vte3 (pkgconfig)
> > - kwayland (cmake)
> > - discover (desktop file)
> >
> > I know pacman can already install packages given a provides capability,
> > so I didn't bother setting up a local test repository to confirm the
> > fact that pacman
> > can install these packages given their capabilities.
>
> You're not reading what I said.

Pardon, what did you mean by "Did you actually try this"?

The large quote of the patch's description immediately beforehand had
me assuming that you were asking about
if I had tested the patch.
Eli Schwartz March 9, 2020, 12:07 a.m. UTC | #5
On 3/8/20 6:54 PM, Carson Black wrote:
>>>> Did you actually try this?
>>>
>>> Yes, I've been using the following packages as tests for this and
>>> inspecting their metadata by hand:
>>>
>>> - vte3 (pkgconfig)
>>> - kwayland (cmake)
>>> - discover (desktop file)
>>>
>>> I know pacman can already install packages given a provides capability,
>>> so I didn't bother setting up a local test repository to confirm the
>>> fact that pacman
>>> can install these packages given their capabilities.
>>
>> You're not reading what I said.
> 
> Pardon, what did you mean by "Did you actually try this"?
> 
> The large quote of the patch's description immediately beforehand had
> me assuming that you were asking about
> if I had tested the patch.

I meant "look at the next few paragraphs, where I will explain why I
don't think you tested this".

And I explicitly ended off with "one would still NOT be able
to use the automatically generated provides for packaging".

But you know what, given you've said

> Yes, I've been using the following packages as tests for this and
> inspecting their metadata by hand:

> I know pacman can already install packages given a provides capability,
> so I didn't bother setting up a local test repository to confirm the
> fact that pacman
> can install these packages given their capabilities.
I guess instead of not trying "use this as a packaging dependency in
PKGBUILDs", you simply didn't test it at all, which is worse than I
initially thought.

So in the interest of proving that "it got successfully written to the
.PKGINFO file" is not sufficient proof that the entire toolchain works,
I invite you to add this repo to pacman.conf (I hacked up write_pkginfo
to verbatim write the contents of any PKGBUILD-supplied function by a
given name, PKGBUILDs are in this http directory):

[junk]
Server = https://pkgbuild.com/~eschwartz/repo/junk/

And run:

$ printf '%s\n' '-`.^&=>=<: 1.0.0.0-:0  $""🤢' | sudo pacman -Syu -
$ printf '%s\n' '-`.^&=>=<: 1.0.0.0-:0/  $""🤢' | sudo pacman -Syu -


It's provided here, so it should work, right? :

$ bsdtar -xOf test-junk2-1-1-any.pkg.tar.zst .PKGINFO | grep provides
provides = -`.^&=>=<: 1.0.0.0-:0  $""🤢
$ bsdtar -xOf test-junk-1-1-any.pkg.tar.zst .PKGINFO | grep provides
provides = -`.^&=>=<: 1.0.0.0-:0/  $""🤢



...

Conclusion: just because it is in the .PKGINFO, doesn't mean pacman
itself can read it. You need to actually look at the changes you make,
and verify the parser will accept it, and document that  it works.

P.S. pacman *can* read 'pkgconfig(libcurl)' provided by the same
database, so that at least will be accepted by *pacman*'s parser. Even
though you didn't test it. Even though makepkg won't let you depend on
it in another package.
Allan McRae March 9, 2020, 1:45 a.m. UTC | #6
On 9/3/20 6:04 am, Carson Black wrote:
> The code is architected in a manner designed to make it trivial to add
> new provider autogenerators.
> 
> So far, there are autogenerated providers for:
> - pkgconfig(package)
> - cmake(package)
> - desktop files
>   * app(desktopfilename)
>   * can-open-mimetype(mimetype)
> 
> While these automatically generated provides can be used for packaging,
> this is intended more for interactive usage rather than an attempt to
> change how Arch packaging works.

Lets take a step back from even looking at the patch and whether it
works, and focus on whether this should be a feature of makepkg/pacman
at all.

Firstly, I am very against makepkg performing action that are not
obvious from reading a PKGBUILD.  This is why our templating system
pulls the code into the PKGBUILD instead of just providing a library of
functions to use.  That way we can read PKGBUILDs to see the
build/package commands, unlike ebuilds.  It is also why libprovides &
libdepends require specifying the library name, and only the version is
automatically added.

People have previously suggested automatically adding all dependencies
on libraries to depends, and have all packages provide their libraries.
 This was rejected.

So, I can't see me accepting this idea either.


However, we have started splitting makepkg into smaller parts, and
allowing some parts to be extendable with drop-in files.  I am using
that to do checking on PKGBUILDs and packages from within makepkg
(https://github.com/allanmcrae/pkglint), and people have used it to add
support for more build options (e.g. the following packages in the AUR:
makepkg-optimize, makepkg-tidy-ect, makepkg-tidy-pdfsizeopt,
makepkg-tidy-scripts-git)

One idea could be to add a method for extending a packages metadata
while writing .PKGINFO.  I would not accept any functions adding
metadata into the pacman code base (except maybe some examples, not
enabled by default), but this serves as a point for distributions to
make their choices.

Allan
Eli Schwartz March 9, 2020, 2:34 a.m. UTC | #7
On 3/8/20 9:45 PM, Allan McRae wrote:
> On 9/3/20 6:04 am, Carson Black wrote:
>> The code is architected in a manner designed to make it trivial to add
>> new provider autogenerators.
>>
>> So far, there are autogenerated providers for:
>> - pkgconfig(package)
>> - cmake(package)
>> - desktop files
>>   * app(desktopfilename)
>>   * can-open-mimetype(mimetype)
>>
>> While these automatically generated provides can be used for packaging,
>> this is intended more for interactive usage rather than an attempt to
>> change how Arch packaging works.
> 
> Lets take a step back from even looking at the patch and whether it
> works, and focus on whether this should be a feature of makepkg/pacman
> at all.
> 
> Firstly, I am very against makepkg performing action that are not
> obvious from reading a PKGBUILD.  This is why our templating system
> pulls the code into the PKGBUILD instead of just providing a library of
> functions to use.  That way we can read PKGBUILDs to see the
> build/package commands, unlike ebuilds.  It is also why libprovides &
> libdepends require specifying the library name, and only the version is
> automatically added.

Yep -- like I mentioned before, this should be opt-in via something
explicitly in the PKGBUILD, libprovides is a good example of how to do that.

> People have previously suggested automatically adding all dependencies
> on libraries to depends, and have all packages provide their libraries.
>  This was rejected.
> 
> So, I can't see me accepting this idea either.
> 
> 
> However, we have started splitting makepkg into smaller parts, and
> allowing some parts to be extendable with drop-in files.  I am using
> that to do checking on PKGBUILDs and packages from within makepkg
> (https://github.com/allanmcrae/pkglint), and people have used it to add
> support for more build options (e.g. the following packages in the AUR:
> makepkg-optimize, makepkg-tidy-ect, makepkg-tidy-pdfsizeopt,
> makepkg-tidy-scripts-git)
> 
> One idea could be to add a method for extending a packages metadata
> while writing .PKGINFO.  I would not accept any functions adding
> metadata into the pacman code base (except maybe some examples, not
> enabled by default), but this serves as a point for distributions to
> make their choices.

I kind of also need to extend this for my WIP / languishing attempts at
https://git.archlinux.org/users/eschwartz/pacman.git/log/?h=autoversioned-dependencies,
so there's definitely some interesting stuff you could do with this.
That being said, changes to the metadata generation have a tendency to
become thing that also need lint_pkgbuild changes, so I'm not 100%
convinced thirdparty extensions can be done here *safely*.

(I suppose we could just say "eh, if you modify it, it's your fault if
it breaks".)
Erich Eckner March 9, 2020, 6:16 a.m. UTC | #8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

On Mon, 9 Mar 2020, Allan McRae wrote:

> People have previously suggested automatically adding all dependencies
> on libraries to depends, and have all packages provide their libraries.
> This was rejected.

I had requested something similar once, but not exactly what you wrote (I 
assume, you refer to my request - if not, please ignore):

The request was to add all the provides parts, but the depends part only 
if one likes to (e.g. if one evaluates the stability of linking more 
important than an easy build path for upgrades) - but even that got 
rejected.

So in essence: Already *adding* the metadata is not accepted. Even if it's 
not intended to use it thoroughly in packages.

regards,
Erich

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl5l3yoACgkQCu7JB1Xa
e1p/0BAAmJharWvb4WO/GCC5a5Jqg/DtFmN6TYA7IsB6y+2Pb226ekCdjPlgBvmp
IaSf21y0TAeApvVf6VQvbIztLs/x+GjOtHiVxlNkQMlF3zkux0yBvEADjgso6kfQ
BAMqGNbwI5pG45eyUoi4oKCGdS7tFBcHEwYRdld4QGtmUXZ2SL1Jp0aBdGs8THwh
IETrwLMz+FqooJtBTemtZj01YEyPFlPD+Q2qXSgkTvj9yQFdWI/2JngyAYHCXUyZ
wK951u7ZWt3419qKd6DISwubqAaxjL7l1TZb5AuwMSeHlOLV98/56MhxCdDkj48d
xu69GPpwbV+FqVhto69K8LknAkv0knb/CcE/WDUwx2SrGdSm8u85eBIFvDPYc0dW
bbKmNfkBKUvjTUJFxMSHDpHIPDXZ+CRA4vikgnhfdnOgloG+tsAAgPUKEhfXLKyQ
APLsTNlRxAb+9ULHHifDEidsUCtFo9Tc6Qx/qu1FUKWY1FemDKuYEtymJD+FdL1j
1dM8P2xMnHwgrFYvemrufCcoc0hiURRSvTnb9EXqzRMZJia10vdX33kPElWHYmZL
dKwcIm5GHf5C1GdtebNTFmqWxLe/lyyhAqfCiMILFT/F3RI0n/sKw7RTz4AI1JZf
dezNZqWU3AXTutiWpneFLgwjmFtAuLwIL25AubJmzBhvgzvyYCk=
=Mcz4
-----END PGP SIGNATURE-----
Ralph Corderoy March 9, 2020, 9:55 a.m. UTC | #9
Hi Carsten,

> > > +                     for mimetype in "${mimetypes[@]}"; do
> > > +                             cleaned=$(echo $mimetype | xargs)
> >
> > What is "cleaned", and how is the xargs program "cleaning" it?
>
> Excess whitespace is being trimmed using xargs. "   a string " will
> get normalized to "a string" when passed through xargs.

That's not what actually happens; the whitespace is stripped before
being written down the pipe to xargs.

    $ mimetype='   a string '
    $ echo $mimetype | sed -n l
    a string$
    $

It's also ‘trimming’ internal whitespace, and I don't think that matches
the understood definition of trim.

    $ mimetype='   foo   bar  '
    $ echo $mimetype | sed -n l
    foo bar$
    $

Given this is bash, a built-in method may be more efficient.
Eli Schwartz March 9, 2020, 10:53 a.m. UTC | #10
On 3/9/20 2:16 AM, Erich Eckner wrote:
> Hi,
> 
> On Mon, 9 Mar 2020, Allan McRae wrote:
> 
>> People have previously suggested automatically adding all dependencies
>> on libraries to depends, and have all packages provide their libraries.
>> This was rejected.
> 
> I had requested something similar once, but not exactly what you wrote
> (I assume, you refer to my request - if not, please ignore):
> 
> The request was to add all the provides parts, but the depends part only
> if one likes to (e.g. if one evaluates the stability of linking more
> important than an easy build path for upgrades) - but even that got
> rejected.
> 
> So in essence: Already *adding* the metadata is not accepted. Even if
> it's not intended to use it thoroughly in packages.

Adding provides=(libfoo.so) to all Arch PKGBUILDs wouldn't be a
pacman-dev change.

IIRC people have actually asked that makepkg automatically search
through the $pkgdir and add provides=(libfoo.so=1-64) for every single
installed library, whether the PKGBUILD instructs to do so or not.
Eli Schwartz March 9, 2020, 10:54 a.m. UTC | #11
On 3/8/20 6:49 PM, Eli Schwartz wrote:
> On 3/8/20 6:39 PM, Carson Black wrote:
>>> What is "cleaned", and how is the xargs program "cleaning" it?
>>
>> Excess whitespace is being trimmed using xargs. "   a string
>> " will get normalized to "a string" when passed through xargs.
> 
> Examples of desktop files with this issue? I didn't find one like it on
> my system.
> 
>>>
>>>> +
>>>> +                             [[ -z "$cleaned" ]] && continue
>>>
>>> When will this be set but empty?
>>
>> Trailing semicolon in desktop file will result in a blank entry in the
>> mimetypes array,
>> resulting in a blank can-open-mimetype() provide without this code.
> 
> $ mapfile -t -d ";" mimetypes < <(grep MimeType=
> /usr/share/applications/qemu.desktop | cut -d '=' -f 2)
> $ declare -p mimetypes
> declare -a mimetypes=()

Not sure what I was thinking here, lol, it is in fact a problem when the
Mimetypes= line *does* exist. A good way to solve this would be

[[ -n ${mimetypes[-1]//$'\n'} ]] || unset mimetypes[-1]

Patch

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index ca3e7459..65a3648e 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -57,6 +57,7 @@  CLEANUP=0
 DEP_BIN=0
 FORCE=0
 GENINTEG=0
+GENPROVIDES=1
 HOLDVER=0
 IGNOREARCH=0
 INFAKEROOT=0
@@ -520,6 +521,70 @@  find_libdepends() {
 	(( ${#libdepends[@]} )) && printf '%s\n' "${libdepends[@]}"
 }
 
+pkgconfig_prov() {
+	case $1 in
+		*.pc)
+			directory="`dirname ${1}`"
+			export PKG_CONFIG_PATH="$DIR:$DIR/../../share/pkgconfig"
+			pkg-config --print-provides "$1" 2>/dev/null | while read name _ _; do
+				[ -n "$name" ] || continue
+				echo "pkgconfig($name)"
+			done
+			;;
+	esac
+}
+
+cmake_prov() {
+	case $1 in
+		*.cmake)
+			base="$(basename $1)"
+			case $1 in
+				*Config.cmake)
+					echo "cmake(${base%Config.cmake})"
+					;;
+				*-config.cmake)
+					echo "cmake(${base%-config.cmake})"
+					;;
+			esac
+			;;
+	esac
+}
+
+desktop_prov() {
+	case "$1" in
+		*.desktop)
+			grep -q "Type=Application" "$1" || return
+			grep -q "Exec=" "$1" || return
+			base="$(basename $1)"
+			echo "app(${base%.desktop})"
+
+			mapfile -t -d ";" mimetypes < <(grep MimeType= $1 | cut -d '=' -f 2)
+			for mimetype in "${mimetypes[@]}"; do
+				cleaned=$(echo $mimetype | xargs)
+
+				[[ -z "$cleaned" ]] && continue
+
+				echo "can-open-mimetype($cleaned)"
+			done
+			;;
+	esac
+}
+
+providers=(
+	pkgconfig cmake desktop
+)
+
+find_fileattrs_provides() {
+	local files
+
+	mapfile -t files < <(find "$pkgdir" -type f)
+
+	for file in "${files[@]}"; do
+		for function in "${providers[@]}"; do
+			"$function"_prov "$file"
+		done
+	done
+}
 
 find_libprovides() {
 	local p libprovides missing
@@ -612,12 +677,14 @@  write_pkginfo() {
 
 	mapfile -t provides < <(find_libprovides)
 	mapfile -t depends < <(find_libdepends)
+	(( "$GENPROVIDES" != 0 )) && mapfile -t generated_provides < <(find_fileattrs_provides)
 
 	write_kv_pair "license"     "${license[@]}"
 	write_kv_pair "replaces"    "${replaces[@]}"
 	write_kv_pair "group"       "${groups[@]}"
 	write_kv_pair "conflict"    "${conflicts[@]}"
 	write_kv_pair "provides"    "${provides[@]}"
+	write_kv_pair "provides"    "${generated_provides[@]}"
 	write_kv_pair "backup"      "${backup[@]}"
 	write_kv_pair "depend"      "${depends[@]}"
 	write_kv_pair "optdepend"   "${optdepends[@]//+([[:space:]])/ }"
@@ -980,6 +1047,7 @@  usage() {
 	printf -- "$(gettext "  --nocheck        Do not run the %s function in the %s")\n" "check()" "$BUILDSCRIPT"
 	printf -- "$(gettext "  --noprepare      Do not run the %s function in the %s")\n" "prepare()" "$BUILDSCRIPT"
 	printf -- "$(gettext "  --nosign         Do not create a signature for the package")\n"
+	printf -- "$(gettext "  --noprovidesgen  Do not autogenerate provides")\n"
 	printf -- "$(gettext "  --packagelist    Only list package filepaths that would be produced")\n"
 	printf -- "$(gettext "  --printsrcinfo   Print the generated SRCINFO and exit")\n"
 	printf -- "$(gettext "  --sign           Sign the resulting package with %s")\n" "gpg"
@@ -1029,7 +1097,7 @@  OPT_LONG=('allsource' 'check' 'clean' 'cleanbuild' 'config:' 'force' 'geninteg'
           'help' 'holdver' 'ignorearch' 'install' 'key:' 'log' 'noarchive' 'nobuild'
           'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare' 'nosign' 'packagelist'
           'printsrcinfo' 'repackage' 'rmdeps' 'sign' 'skipchecksums' 'skipinteg'
-          'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version')
+          'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version' 'noprovidesgen')
 
 # Pacman Options
 OPT_LONG+=('asdeps' 'noconfirm' 'needed' 'noprogressbar')
@@ -1070,6 +1138,7 @@  while true; do
 		--nocheck)        RUN_CHECK='n' ;;
 		--noprepare)      RUN_PREPARE='n' ;;
 		--nosign)         SIGNPKG='n' ;;
+		--noprovidesgen)  GENPROVIDES=0 ;;
 		-o|--nobuild)     BUILDPKG=0 NOBUILD=1 ;;
 		-p)               shift; BUILDFILE=$1 ;;
 		--packagelist)    BUILDPKG=0 PACKAGELIST=1 IGNOREARCH=1;;