Re: [pacman-dev] [PATCH 3/3] meson: use hidden symbol visiblity by default

2020-12-28 Thread Eli Schwartz

On 12/28/20 10:15 PM, Allan McRae wrote:

On 25/12/20 2:05 am, Emil Velikov wrote:

On Thu, 24 Dec 2020 at 01:38, Eli Schwartz  wrote:


On 12/23/20 5:42 PM, Emil Velikov wrote:

All the required public API is annotated with SYMEXPORT, so we can just
add the meson notation, to hide all the symbols by default.

Thus we no longer spill all the internal API into the global namespace.

Thanks for noticing this... it's a regression from autotools, which
contained in lib/libalpm/Makefile.am:

if ENABLE_VISIBILITY_CC
if DARWIN
AM_CFLAGS += -fvisibility=hidden
else
AM_CFLAGS += -fvisibility=internal
endif
endif


I wonder if we had a good reason to use "internal" and if we should
continue to do so? IIUC it makes it slightly more optimized at the cost
of allowing pointers into private functions (e.g. callbacks) used by
other programs to segfault.


If the output of size is any indication - there is little-to-no
optimisation happening.
The former produces the same numbers, while the latter claims that
binaries built with "internal" are larger by 8 bytes (always).

Fwiw the above snippet is the first time I've seen anyone using
"internal", after staring at various projects for 5+ years.
Can bring it back, simply not sure it brings much.

Thanks
Emil

$ ls
812336 build-internal/libalpm.so.12.0.1
812328 build-hidden/libalpm.so.12.0.1
337176 build-internal/pacman
337168 build-hidden/pacman

$ size
3167083080 592  320380   4e37c build-internal/libalpm.so.12.0.1
3167083080 592  320380   4e37c build-hidden/libalpm.so.12.0.1
15528850405808  166136   288f8 build-internal/pacman
15528850405808  166136   288f8 build-hidden/pacman



It turns out, we have this:
#define SYMHIDDEN __attribute__((visibility("internal")))

But we never flag any functions with this.  That would enable a compiler
to optimize for speed and not compatibility, in which case using
-fvisibility=internal would make a difference.

I'm not sure if we removed any usage of that from the codebase, or if it
was never there...


Hmm... only ever used on alpm_add_target (then _alpm_add_loadtarget) and 
removed in commit 7f7da2b5fc01f46d28236384540c7ecfdac16a63 which added 
the AM_CFLAGS instead and marked a bunch of symbols as SYMEXPORT.


But that define used to be for visibility("hidden"), then in commit 
920b0d2049deb148efe89bfebda03d172b68c1f5 both the (unused) define and 
the AM_CFLAGS got switched to internal: "this allows for slightly better 
optimization".


Which is about as vague as our knowledge today. "Supposedly better and 
probably works fine, but no one put out numbers on it".


idk, "hidden" is probably just fine.

--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


[pacman-dev] [PATCH] doc: make doxygen build from any directory

2020-12-28 Thread Eli Schwartz
In the autotools build, it only built in-tree, from cwd = doc/ and
resolving doc/../lib/libalpm

In the meson build, this accidentally worked if cwd =
pacman/builddir/ and resolved to builddir/../lib/libalpm/

But... this should always have been configured with the actual path to
the inputs. So, we will now proceed to do so.

Fixes building man3 if your out of tree builddir doesn't happen to be a
direct subdirectory of the source root.

Signed-off-by: Eli Schwartz 
---
 doc/Doxyfile.in | 4 ++--
 doc/meson.build | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/doc/Doxyfile.in b/doc/Doxyfile.in
index 776318da7..6744e7659 100644
--- a/doc/Doxyfile.in
+++ b/doc/Doxyfile.in
@@ -117,8 +117,8 @@ WARN_LOGFILE   =
 #---
 # Configuration options related to the input files
 #---
-INPUT  = ../lib/libalpm/alpm.h \
- ../lib/libalpm/alpm_list.h
+INPUT  = @INPUT_DIRECTORY@/../lib/libalpm/alpm.h \
+ @INPUT_DIRECTORY@/../lib/libalpm/alpm_list.h
 INPUT_ENCODING = UTF-8
 FILE_PATTERNS  =
 RECURSIVE  = NO
diff --git a/doc/meson.build b/doc/meson.build
index 570dc7654..4aaac5540 100644
--- a/doc/meson.build
+++ b/doc/meson.build
@@ -135,6 +135,7 @@ meson.add_install_script(MESON_MAKE_SYMLINK,
 doxygen = find_program('doxygen', required : get_option('doxygen'))
 if doxygen.found() and not get_option('doxygen').disabled()
   doxyconf = configuration_data()
+  doxyconf.set('INPUT_DIRECTORY', meson.current_source_dir())
   doxyconf.set('OUTPUT_DIRECTORY', meson.current_build_dir())
   doxyfile = configure_file(
 input : 'Doxyfile.in',
-- 
2.29.2


Re: [pacman-dev] [PATCH 3/3] meson: use hidden symbol visiblity by default

2020-12-23 Thread Eli Schwartz

On 12/23/20 5:42 PM, Emil Velikov wrote:

All the required public API is annotated with SYMEXPORT, so we can just
add the meson notation, to hide all the symbols by default.

Thus we no longer spill all the internal API into the global namespace.
Thanks for noticing this... it's a regression from autotools, which 
contained in lib/libalpm/Makefile.am:


if ENABLE_VISIBILITY_CC
if DARWIN
AM_CFLAGS += -fvisibility=hidden
else
AM_CFLAGS += -fvisibility=internal
endif
endif


I wonder if we had a good reason to use "internal" and if we should 
continue to do so? IIUC it makes it slightly more optimized at the cost 
of allowing pointers into private functions (e.g. callbacks) used by 
other programs to segfault.


--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 1/3] meson: replace objects with link_whole

2020-12-23 Thread Eli Schwartz

On 12/23/20 5:42 PM, Emil Velikov wrote:

Instead extracting/listing objects from existing static libraries,
simply add those to the link list. Note that link_whole is needed
otherwise, unused code will be pruned.

This is intentional! link_whole is broken, just like the comment says.

It is fixed in newer versions of meson than the current minimum. The fix 
is that link_whole does precisely what extract_all_objects() does. In 
other words, link_whole is syntactic sugar for extract_all_objects().


I have intended to switch to link_whole, but *only* once we bump the 
minimum version of meson to something that guarantees the result is 
functional.


I didn't want to bump the minimum version of meson just for this.

--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Add `build/` to gitignore

2020-12-12 Thread Eli Schwartz

On 12/10/20 9:12 PM, Colin Woodbury wrote:

Right, having `meson` do it automatically would work well. I put
forth the patch in the first place because the instructions on the
Pacman website (master branch), if followed, place the artifacts in
`build/`. Other newcomers to the project like me would hit the same
thing, so some automated solution would be nice.

Is there anything we can do to move that `meson` proposal forward,
other than complaining loudly? ;)


https://github.com/mesonbuild/meson/pull/8092

The proposal is getting traction. :D

--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Add `build/` to gitignore

2020-12-10 Thread Eli Schwartz

On 12/10/20 9:40 PM, Ivy Foster wrote:

On 10 Dec 2020, at  6:12 pm -0800, Colin Woodbury wrote:

Right, having `meson` do it automatically would work well. I put
forth the patch in the first place because the instructions on the
Pacman website (master branch), if followed, place the artifacts in
`build/`. Other newcomers to the project like me would hit the same
thing, so some automated solution would be nice.


If I may propose an alternative, this seems like an ideal use for
.../pacman/.git/info/exclude.

For those who don't know, .git/info/exclude is a secondary gitignore
file not under version control. It is specifically for files that
shouldn't be ignored at the project level, but that an individual
developer or user might want to ignore--such as for build artifacts in
non-standardized places, or your personal wrapper scripts for testing
whatever you're working on.


I do this in $HOME/.config/git/ignore since this pattern is shared by 
multiple meson-using projects and I can't imagine "builddir/" being 
actually used for non build artifacts.


Still would be nice to autogenerate this on meson's side. I'm discussing 
it with them...


--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Add `build/` to gitignore

2020-12-10 Thread Eli Schwartz

On 12/9/20 2:14 PM, Colin Woodbury wrote:

Since it's a transient directory for build artifacts, nothing in there will ever
been committed. Ignoring it also makes IDEs happier during file search.

Signed-off-by: Colin Woodbury 
---
  .gitignore | 1 +
  1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 01975fd2..638dfe39 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
  *~
  *.o
  pacman-*.tar.gz
+build/*


I use builddir/ (and upstream usually recommends that these days since 
it's less likely to confuse "meson builddir" as a subcommand, compared 
to "meson build").


This is kind of the problem with trying to ignore a directory that isn't 
canonical.


Instead, I'd prefer to argue upstream in favor of 
https://github.com/mesonbuild/meson/issues/6509


--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] Location of the Pacman Home Page source?

2020-12-04 Thread Eli Schwartz

On 12/4/20 11:43 AM, Colin Woodbury wrote:

Thanks Eli!

It looks like I had been grepping for `meson compile` and not `meson
build`, hence I couldn't find it. It seems like the asciidoc was
already updated some time ago, just not uploaded to the site itself.


Should be a simple matter of bumping the version in 
https://github.com/archlinux/infrastructure/blob/master/playbooks/tasks/pacman-website.yml


The 6.0 release will need the playbook to be updated to use meson 
instead of configure/make.



Also, in both `index.asciidoc` and `NEWS`, 5.2.2 was never given
release notes. Should we add those, or just wait for the impending
6.0?


It got updated, appropriately, on the release/5.2.x branch. That will 
probably get synced once I write the release notes for 6.0, as happened 
last time...


--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] Location of the Pacman Home Page source?

2020-12-04 Thread Eli Schwartz

On 12/4/20 11:20 AM, Colin Woodbury wrote:

Hi folks, I see that while a number of Pacman-related pages like the
HACKING document are rendered into nice HTML and hosted officially,
does anyone know where the source for the Pacman homepage itself is
hosted (https://www.archlinux.org/pacman/)? There are some
documentation lines I'd like to fix, in particular the ones that
still mention configure/make as the way to compile Pacman. Meson
should probably also be mentioned in the Pacman README itself, since
it is otherwise cited nowhere in that repo. I'd be happy to put forth
a PR.


In the doc/ directory, which contains asciidoc sources for both mapages 
and the website.


You can generate the html files using `ninja html`, and tar it up for 
copying onto the website using `ninja doc/website.tar.gz`.


--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [GIT] The official pacman repository tag, last-deltapkgs-commit, created. v5.1.1-144-g377d4714

2020-12-03 Thread Eli Schwartz

On 12/3/20 11:01 PM, Allan McRae wrote:

On 4/12/20 1:54 pm, Allan McRae wrote:

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The official pacman repository".

The tag, last-deltapkgs-commit has been created
 at  377d47142f7aaa01ca782e6587f2d4caf663865b (commit)

- Log -
commit 377d47142f7aaa01ca782e6587f2d4caf663865b
Author: Jelle van der Waa 
Date:   Thu Feb 21 13:20:19 2019 +0100

 doc: add man page for pacman-conf
 
 Signed-off-by: Jelle van der Waa 

 Signed-off-by: Allan McRae 
---


hooks/post-receive



Hrm...  I made the v6.0.0alpha1 tag and pushed the tags and this came
along for the ride.  Not sure where it came from!  Anyone know?


I think you might have fetched it from 
https://git.archlinux.org/users/eschwartz/pacman.git/tag/?h=last-deltapkgs-commit


But I'm no longer sure why I created this (I think) either.

--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v5] Add fossil scm support to makepkg

2020-11-25 Thread Eli Schwartz
exit 1
+   esac
+   fi
+
+   if ! fossil update "$ref"; then


Does this need to be silenced too? If I add #tag=0.3.1 to your PKGBUILD...

  -> Creating working copy of ddate.fossil fossil repo...
Autosync:  https://iff.ink/fossil/ddate
Round-trips: 1   Artifacts sent: 0  received: 0
Pull done, sent: 316  received: 1763  ip: 198.58.102.79
REMOVE .fossil-settings/ignore-glob
ADD .gitignore
ADD Makefile
UPDATE README.md
ADD ddata/meta/version
ADD ddata/wisdom/yearlength
UPDATE doc/ddate.1
REMOVE meson.build
---
updated-to:   eb8e917743ce4f3d212e67a0f3f09df0eee9b4d1 2020-05-05 
05:35:21 UTC

tags: trunk, 0.3.1, release
comment:  Implement actually printing BS dates (user: escond...@iff.ink)
changes:  8 files modified.
 "fossil undo" is available to undo changes to the working checkout.
==> Starting pkgver()...

It would be quite nice if fossil had a flag to only summarize but not 
print info about every single file. >/dev/null is kind of a blunt axe.


Then again, idk how important this status is to begin with.


+   error "$(gettext "Failure while creating working copy of %s %s repo")" 
"${repo}" "fossil"
+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   fi
+
+   popd &>/dev/null
+}
diff --git a/scripts/libmakepkg/source/meson.build 
b/scripts/libmakepkg/source/meson.build
index 59326133..41b18c37 100644
--- a/scripts/libmakepkg/source/meson.build
+++ b/scripts/libmakepkg/source/meson.build
@@ -3,6 +3,7 @@ libmakepkg_module = 'source'
  sources = [
'bzr.sh.in',
'file.sh.in',
+  'fossil.sh.in',
'git.sh.in',
'hg.sh.in',
'local.sh.in',
diff --git a/scripts/libmakepkg/util/source.sh.in 
b/scripts/libmakepkg/util/source.sh.in
index be7c15c2..029bf8ed 100644
--- a/scripts/libmakepkg/util/source.sh.in
+++ b/scripts/libmakepkg/util/source.sh.in
@@ -65,7 +65,7 @@ get_filename() {
local proto=$(get_protocol "$netfile")

case $proto in
-   bzr|git|hg|svn)
+   bzr|fossil|git|hg|svn)
filename=${netfile%%#*}
filename=${filename%%\?*}
filename=${filename%/}
@@ -73,6 +73,9 @@ get_filename() {
if [[ $proto = bzr ]]; then
filename=${filename#*lp:}
fi
+   if [[ $proto = fossil ]]; then
+   filename=$filename.fossil
+   fi
if [[ $proto = git ]]; then
filename=${filename%%.git*}
fi
--
2.29.2




--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v2] Add fossil scm support to makepkg

2020-11-24 Thread Eli Schwartz
ure $db is quoted (here and elsewhere), there's no reason $SRCDEST / 
$srcdir cannot contain whitespace.



+   error "$(gettext "%s is not a clone of %s")" "$db" 
"$url"
+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   fi
+   msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "fossil"
+   if ! fossil pull -R $db; then
+   # only warn on failure to allow offline builds
+   warning "$(gettext "Failure while updating %s %s repo")" 
"${repo}" "fossil"
+   fi
+   fi
+}
+
+extract_fossil() {
+   local netfile=$1 tagname
+
+   local fragment=$(get_uri_fragment "$netfile")
+   local repo=$(get_filename "$netfile")
+
+   local db=$(get_filepath "$netfile")
+   [[ -z "$db" ]] && db="$SRCDEST/$(get_filename "$netfile")"
+   local dir=${db%%.fossil}
+   dir=${dir##*/}
+
+   msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" 
"fossil"
+   pushd "$srcdir" &>/dev/null
+
+   if [[ -d "$dir" ]]; then
+   if [[ -f "$dir/.fslckout" ]]; then
+   cd_safe "$dir"
+   if ! (fossil info | awk '/^repository:/ {print $2}' | grep 
"$db" >/dev/null); then
+   error "$(gettext "Failure while updating working copy of %s %s 
repo")" "${repo}" "fossil"


This seems to be a check for " is not a working copy of ", you've 
moved up the check from the v1 patch and ensured it correctly checks, 
but the error message is no longer accurate.


Similar issue to above w.r.t. grep -q, () subshell, and [[


+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   fi
+   cd_safe "$srcdir"
+   else
+   error "$(gettext "Failure while updating working copy of %s %s repo")" 
"${repo}" "fossil"
+   plainerr "$(gettext "Aborting...")"
+   exit 1


This seems to be correct now... the working copy doesn't strictly need 
to be updated like in git, but this line here will run whenever the 
working copy is not, in fact, a fossil repo at all. Which means we won't 
be able to update it down below since we fail early.


lgtm


+   fi
+   elif ! fossil open "$db" --workdir "$dir"; then
+   error "$(gettext "Failure while creating working copy of %s %s repo")" 
"${repo}" "fossil"
+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   fi
+
+   cd_safe "${dir##*/}"
+
+   ref=tip
+   if [[ -n $fragment ]]; then
+   case ${fragment%%=*} in
+   branch|commit|tag)
+   ref=${fragment##*=}
+   ;;
+   *)
+   error "$(gettext "Unrecognized reference: %s")" 
"${fragment}"
+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   esac
+   fi
+
+   if ! fossil update "$ref"; then
+   error "$(gettext "Failure while creating working copy of %s %s repo")" 
"${repo}" "fossil"
+   plainerr "$(gettext "Aborting...")"
+   exit 1
+   fi
+
+   popd &>/dev/null
+}
diff --git a/scripts/libmakepkg/source/meson.build 
b/scripts/libmakepkg/source/meson.build
index 59326133..41b18c37 100644
--- a/scripts/libmakepkg/source/meson.build
+++ b/scripts/libmakepkg/source/meson.build
@@ -3,6 +3,7 @@ libmakepkg_module = 'source'
  sources = [
'bzr.sh.in',
'file.sh.in',
+  'fossil.sh.in',
'git.sh.in',
'hg.sh.in',
'local.sh.in',
diff --git a/scripts/libmakepkg/util/source.sh.in 
b/scripts/libmakepkg/util/source.sh.in
index be7c15c2..029bf8ed 100644
--- a/scripts/libmakepkg/util/source.sh.in
+++ b/scripts/libmakepkg/util/source.sh.in
@@ -65,7 +65,7 @@ get_filename() {
local proto=$(get_protocol "$netfile")
  
  	case $proto in

-   bzr|git|hg|svn)
+   bzr|fossil|git|hg|svn)
filename=${netfile%%#*}
filename=${filename%%\?*}
filename=${filename%/}
@@ -73,6 +73,9 @@ get_filename() {
if [[ $proto = bzr ]]; then
filename=${filename#*lp:}
fi
+   if [[ $proto = fossil ]]; then
+   filename=$filename.fossil
+   fi
if [[ $proto = git ]]; then
filename=${filename%%.git*}
fi




--
Eli Schwartz
Bug Wrangler and Trusted User



OpenPGP_signature
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Add fossil scm support to makepkg

2020-11-04 Thread Eli Schwartz
fragment=$(get_uri_fragment "$netfile")
> + local repo=$(get_filename "$netfile")
> +
> + local db=$(get_filepath "$netfile")
> + [[ -z "$db" ]] && db="$SRCDEST/$(get_filename "$netfile")"
> + local dir=${db%%.fossil}
> + dir=${dir##*/}
> +
> + msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" 
> "fossil"
> + pushd "$srcdir" &>/dev/null
> +
> + if [[ -f "$dir/.fslckout" ]]; then
> + cd_safe "$dir"
> + if ! (fossil revert && fossil clean --verily); then
> + error "$(gettext "Failure while updating working copy 
> of %s %s repo")" "${repo}" "fossil"
> + plainerr "$(gettext "Aborting...")"
> + exit 1
> + fi

This entire block feels terribly wrong. In git, we fetch the $SRCDEST
version into the $srcdir one, since they're different git repos wired
together as remotes. In fossil, it looks like this is unnecessary and
should be skipped (the metadata is directly referenced and opened in
$srcdir).

Instead, what this is doing is trying to break incremental builds by
deleting all untracked files (we do not do this for other sources, but
let you use makepkg -C to nuke $srcdir for this purpose), but not
modifying the potential revision at all, then declaring this to be the
"updating working copy" step of the extraction.



> + cd_safe "$srcdir"
> + elif [[ -d "$dir" && ! -f "$dir/.fslckout" ]]; then
> + error "$(gettext "%s is not a working copy of %s")" "$dir" "$db"
> + plainerr "$(gettext "Aborting...")"
> + exit 1

This string is new. Shouldn't fossil detect this automatically? How does
checking for the existence of the metadata file tell you which repo it
is a clone of?

> + elif ! fossil open "$db" --workdir "$dir"; then
> + error "$(gettext "Failure while creating working copy of %s %s 
> repo")" "${repo}" "fossil"
> + plainerr "$(gettext "Aborting...")"
> + exit 1
> + fi
> +
> + cd_safe "${dir##*/}"
> +
> + ref=tip
> + if [[ -n $fragment ]]; then
> + case ${fragment%%=*} in
> + branch|checkin|ci|commit|tag)
> + ref=${fragment##*=}
> + ;;
> + *)
> + error "$(gettext "Unrecognized reference: %s")" 
> "${fragment}"
> + plainerr "$(gettext "Aborting...")"
> + exit 1
> + esac
> + fi
> +
> + if ! fossil update "$ref"; then
> + error "$(gettext "Failure while creating working copy of %s %s 
> repo")" "${repo}" "fossil" "$ref"
> + plainerr "$(gettext "Aborting...")"
> + exit 1
> + fi
> +
> + popd &>/dev/null
> +}
> diff --git a/scripts/libmakepkg/util/source.sh.in 
> b/scripts/libmakepkg/util/source.sh.in
> index be7c15c2..029bf8ed 100644
> --- a/scripts/libmakepkg/util/source.sh.in
> +++ b/scripts/libmakepkg/util/source.sh.in
> @@ -65,7 +65,7 @@ get_filename() {
>   local proto=$(get_protocol "$netfile")
>  
>   case $proto in
> - bzr|git|hg|svn)
> + bzr|fossil|git|hg|svn)
>   filename=${netfile%%#*}
>   filename=${filename%%\?*}
>   filename=${filename%/}
> @@ -73,6 +73,9 @@ get_filename() {
>   if [[ $proto = bzr ]]; then
>   filename=${filename#*lp:}
>   fi
> + if [[ $proto = fossil ]]; then
> + filename=$filename.fossil
> + fi
>   if [[ $proto = git ]]; then
>   filename=${filename%%.git*}
>   fi
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] pacman-key: change signing key to ed25519

2020-11-04 Thread Eli Schwartz
On 11/4/20 5:47 PM, Geert Hendrickx via pacman-dev wrote:
> On Wed, Nov 04, 2020 at 16:30:19 -0500, Eli Schwartz wrote:
>> Currently pacman assumes gpgme from >= the year 2010, is that sufficient
>> to read ed25519? (idk, it's shelling out to gpg and thus likely doesn't
>> care?) Maybe we should bump this anyway in the expectation that requiring
>> a ~2015 version of gpgme will naturally lead to gpg versions that support
>> generating such keys.
> 
> 
> This change only affects new installations, existing ones will continue
> using their rsa2048 (or recently rsa4096) master keys, until they re-run
> pacman-key --init.

That's really not my point at all. My point is that rerunning --init
does something the project dependencies don't describe as a requirement
to support.

>>> This will also become the default in the next version of GnuPG.
>>
>> I see such a commit on GnuPG's master branch but not on the stable
>> branch. When do you expect this to be released...
> 
> 
> Good question, I don't know.  The point is that the trend is clearly
> towards EdDSA rather than larger RSA.  And GnuPG (as well as openssh
> etc) need to be conservative, as they must be interoperable with other
> or older implementations, pacman doesn't even have that limitation.

Why doesn't pacman have this limitation? Because it is only used in Arch
Linux?

Untrue, there is an active MSYS2 community using it, and we support
running on macOS/BSD and occasionally get people posting compilation
fixes on those platforms. Who knows what version of GnuPG various minor
users might have?

It's not ridiculous to consider *if* we can declare a dependency on the
proposed, updated runtime workflow requirement.

Likewise, I think "GnuPG did not actually change this default and won't
for some time" is a meaningful point of discussion; Jonas pointed out
the key strength actually decreases, while the smaller key size is not
obviously beneficial as it is only used for web of trust signing.

TBH, I'm -1 on any change that is being done without any rationale other
than "because it's the modern way of the future". Changes should be done
because they are better, or safer, or some other practical application.

This may or may not be the case here, I haven't pondered it much yet.
But... it's not being highlighted in the current discussion.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] pacman-key: change signing key to ed25519

2020-11-04 Thread Eli Schwartz
On 11/4/20 3:53 PM, Geert Hendrickx via pacman-dev wrote:
> Larger RSA keys are not the way forward, switch to ed25519 instead.

Currently pacman assumes gpgme from >= the year 2010, is that sufficient
to read ed25519? (idk, it's shelling out to gpg and thus likely doesn't
care?) Maybe we should bump this anyway in the expectation that
requiring a ~2015 version of gpgme will naturally lead to gpg versions
that support generating such keys.

> This will also become the default in the next version of GnuPG.

I see such a commit on GnuPG's master branch but not on the stable
branch. When do you expect this to be released...

> Signed-off-by: Geert Hendrickx 
> ---
>  scripts/pacman-key.sh.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index ccfd1b96..fd9d3793 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -147,8 +147,8 @@ generate_master_key() {
>   # Generate the master key, which will be in both pubring and secring
>   "${GPG_PACMAN[@]}" --gen-key --batch <  %echo Generating pacman keyring master key...
> -Key-Type: RSA
> -Key-Length: 4096
> +Key-Type: EDDSA
> +Key-Curve: ed25519
>  Key-Usage: sign
>  Name-Real: Pacman Keyring Master Key
>  Name-Email: pacman@localhost
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values

2020-10-27 Thread Eli Schwartz
On 10/27/20 9:17 PM, Morgan Adamiec wrote:
> So as it turns out -Qip shows the licenses. Installing the package and
> doing -Qi only shows a.

Confirmed...

In /var/lib/pacman/local/e-1-1/desc I do see:

```
%LICENSE%
1
all
the
extra
licenses

even
in
the
middle
```

$ pacman -Qi e
Licenses: 1  all  the  extra  licenses

This makes a definite case for linting this by accepting your patch.
OTOH maybe we should fix this... this /whatever/.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values

2020-10-27 Thread Eli Schwartz
On 10/27/20 8:59 PM, Morgan Adamiec wrote:
> On 28/10/2020 00:46, Eli Schwartz wrote:
>> It remains unclear to me, why the ambiguity matters.
>>
> 
> I guess if you were to officially declare `Key =` means to set the key
> to none then that would solve the issue of it being ambiguous
> 
> Then again that would make `license=('a' '' 'b')` parse as only as
> licence=(b) seems as it was cleared before.
> 
> But funnily enough, if you build the package then pacman -Qi reports
> only license a. Fun!

I just checked this, but -Qip reported all the licenses, both before and
after the '' license. Is this an observation about the current state of
pacman, or about what the proposed semantics would be?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values

2020-10-27 Thread Eli Schwartz
On 10/27/20 8:25 PM, Morgan Adamiec wrote:
> On 28/10/2020 00:04, Allan McRae wrote:
> 
>> pkgnames/depends/etc where it may be an issue.  So I'm not sure this
>> check finds anything in the "break makepkg/pacman" category.
> 
> I disagree, it actually does break something, the srcinfio file
> 
> Consider the following pkgbuild:
> 
> pkgbase=foo
> pkgname=(a b)
> pkgver=1
> pkgrel=1
> arch=(any)
> 
> license=(1)
> 
> package_a() {
>   license=()
> 
> }
> 
> package_b() {
>   license=('')
> }
> 
> And the srcinfo file:
> 
> pkgbase = foo
>   pkgver = 1
>   pkgrel = 1
>   arch = any
>   license = 1
> 
> pkgname = a
>   license =
> 
> pkgname = b
>   license =
> 
> 
> Now package `a` overrides license to an empty array. The srcinfo
> expresses this by putting an empty license entry.
> 
> Now package b defines a license of `empty string`, yet it generates the
> same output. So it's impossible to tell what the original pkgbuild
> actually meant.

In both cases, it overrides the license=('1') into non-existence.

The question is if "There is no license" or "the license is a
zero-length string of emptiness" is semantically meaningful. Especially
given the author of the PKGBUILD probably intended to put the former.

Given it's a purely display-oriented field, you can absolutely stuff it
with whatever you want, and "a zero-length string of emptiness" is not
technically invalid even if it is stupid, whereas it's useful to catch
makedepends=('') before makepkg -s fails to find a satisfier, root and all.

> This example may seen a little contrived, but i assure you it's not.
> Because stuff like this is done in the wild [1] and as a maintainer of a
> srcinfo parser it's annoying that it creates this ambiguity.

It remains unclear to me, why the ambiguity matters.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 1/2] makepkg: fix signing of source packages

2020-10-11 Thread Eli Schwartz
On 10/11/20 10:22 PM, Eli Schwartz wrote:
> In commit c6b04c04653ba9933fe978829148312e412a9ea7 the signing stage was
> moved out of fakeroot, and thus into the main control flow instead of
> create_{,src}package

Random factoid:

10:47 PM  eschwartz: had I seen those patches before?
10:48 PM  no, I wrote them tonight
10:49 PM  ok - it seemed familiar
10:49 PM  you previously merged:
10:49 PM  makepkg: when signing packages, report package
filename on failure
10:49 PM  libmakepkg/integrity: fix regression that broke
--install
10:49 PM  makepkg: avoid false "Signing package(s)" msg when
signing is disabled
10:50 PM  good grief
10:50 PM  great patch :P
10:50 PM  that commit is the gift that keeps on giving


I must say, this patch really was fun. :D How on earth did it manage to
have so many edge cases

Bets on us discovering a 5th problem?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH 1/2] makepkg: fix signing of source packages

2020-10-11 Thread Eli Schwartz
In commit c6b04c04653ba9933fe978829148312e412a9ea7 the signing stage was
moved out of fakeroot, and thus into the main control flow instead of
create_{,src}package

While the function for signing binary packages has logic to build
and gpg-sign multiple filenames, the source package never got this
special treatment. This would be fine, except it uses the standard
variables to set define the filename... like ${fullver}, which is
usually set beforehand, but in this case is not. We don't define fullver
globally as it's an internal implementation detail, except by sheer
coincidence if PKGVERFUNC is false due to improperly guarded code.

Result: source packages didn't end up signed. Instead, we raised a logic
error:

==> WARNING: Failed to sign package file somepackage-.src.tar.gz.

==> ERROR: An unknown error has occurred. Exiting...

Instead, let's just build the version inline, since we only use it once.

Reported-by: GaKu999 
Signed-off-by: Eli Schwartz 
---
 scripts/makepkg.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index e1e95412..a9e7c691 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1362,7 +1362,7 @@ if (( SOURCEONLY )); then
 
if [[ $SIGNPKG = 'y' ]]; then
msg "$(gettext "Signing package...")"
-   create_signature "$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}"
+   create_signature 
"$SRCPKGDEST/${pkgbase}-$(get_full_version)${SRCEXT}"
fi
 
msg "$(gettext "Source package created: %s")" "$pkgbase ($(date +%c))"
-- 
2.28.0


[pacman-dev] [PATCH 2/2] makepkg: properly localize some internal function variables

2020-10-11 Thread Eli Schwartz
We leaked fullver and pkgarch all over the place, and only conditionally
unset the other variables. Marking them local is a more proactive
solution.

Signed-off-by: Eli Schwartz 
---
 scripts/makepkg.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index a9e7c691..89da3fab 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -869,6 +869,7 @@ install_package() {
 }
 
 check_build_status() {
+   local fullver pkgarch allpkgbuilt somepkgbuilt
if (( ! SPLITPKG )); then
fullver=$(get_full_version)
pkgarch=$(get_pkg_arch)
@@ -911,7 +912,6 @@ check_build_status() {
exit $E_ALREADY_BUILT
fi
fi
-   unset allpkgbuilt somepkgbuilt
fi
 }
 
-- 
2.28.0


Re: [pacman-dev] [PATCH] makepkg: libprovides: don't provide both versioned and unversioned sonames

2020-09-22 Thread Eli Schwartz
On 6/26/20 2:09 AM, Allan McRae wrote:
> This patch is fine, but I don't like "newp" as a variable name.  It will
> not help me understand this in a years time.
> 
> How about versioned_provides=() instead?

Yes, that sounds fine to me. Do you want to do a simple replacement
while pulling the patch, or should I resubmit it?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Replace Pacman blurry icon with vector

2020-09-22 Thread Eli Schwartz
On 9/19/20 9:51 AM, Morten Linderud wrote:
> On Wed, Sep 16, 2020 at 04:34:58PM -0400, Eli Schwartz wrote:
>> pacman doesn't come with any icons. What program are you actually using?
> 
> It's pamac, so no relevance to pacman and complete unrelated.
> 
> https://gitlab.manjaro.org/applications/pamac/-/issues/635

I got an off-list email from the OP a day earlier, fingering the same
third-party program.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] repo-add: add --include-sigs option

2020-09-22 Thread Eli Schwartz
On 9/21/20 3:02 AM, Allan McRae wrote:
> On 21/9/20 3:51 pm, Andrew Gregory wrote:
>> I would suggest just allowing the user to specify either way
>> (--include-sigs/--no-include-sigs, --include-sigs={yes,no}, etc).
>> Then uses can specify whatever they want without having to worry about
>> what we set as a default.
>>
> 
> The problem is more the transition.  I would like the default to be not
> to include the signatures in the repo database.  So does pacman need to
> manage the transition from having signatures in a database to not, or do
> the users need to manage that?
> 
> With my patch (or any variant the does not include signatures by
> default), users upgrading to repo-add v6.0 would need to adjust their
> repo management utilities to add a signature include option immediately,
> as their users may still be using pacman-5.x.
> 
> Thinking of Arch here, a dbscripts update would need launched on the
> server at the same time as updating repo-add.  I am OK with that - some
> updates need done in concert.  But Eli was not.

I'm concerned both about the need to time the adjustment of the option,
and about the desire for what I see as sane defaults.

My preference is to provide both options, but change the default in
pacman 6.0.1.
While we're hacking on repo-add options, we could go ahead and make it
use parseopts, because the current handling is gross. Also I would like
an elephant (usually I would request a pony, but this felt more apropos
if we're talking about repo-add).

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Replace Pacman blurry icon with vector

2020-09-16 Thread Eli Schwartz
On 9/16/20 10:23 AM, Lucas Bustamante wrote:
> Hi,
> 
> First of all, thanks for contributing to this open-source program.
> 
> Can you please update Pacman icon to vector? It's a bit blurry :)
> 
> [image: image.png]

pacman doesn't come with any icons. What program are you actually using?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 0/1] Introduce "AssumeInstalled" option in the config file

2020-09-05 Thread Eli Schwartz
On 9/5/20 3:11 AM, Damjan Georgievski wrote:
> On Fri, 4 Sep 2020 at 03:35, Andrew Gregory  
> wrote:
>> Without a better use case, I'm -1 on this.  --assume-installed was
>> intended for ignoring individual dependencies in exceptional
>> circumstances, not permanently ignore a dependency you don't like.
>> The preferred way of dealing with that, should you feel the need, is
>> with a meta package.
> 
> and how do I plug that into pacstrap? custom repo? that seems like a
> much bigger overkill :/

Do you need to install it during pacstrap itself to override packages in
the base install? If not, you can simply execute pacstrap && pacman -r
/mnt -U /path/to/metapackage.

About a year ago, I added a -U option to pacstrap, which makes it use
pacman -U for all packages; you may therefore pipe a list of packages to
install as such:

mkdir ./fakedb
{ pacman --dbpath ./fakedb -Syp base; echo /path/to/metapackage; } |
pacstrap -U /mnt -

Since pacstrap may be repeatedly run on the same existing root and does
not require that it be the one to create said root, you may also do this
much simpler form (assuming the metapackage does not have dependencies,
which it presumably shouldn't):

pacstrap -U /mnt /path/to/metapackage
pacstrap /mnt [base otherpackages]


Either way, I'm even less convinced of the need for a pacman.conf option
if the only time it is needed is during pacstrap and never again; even
if it were true that you needed it then, I'd argue you could just as
well install as normal, then replace the package you didn't want with a
metapackage after chrooting in.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] makepkg.conf: Reword "Defaults"

2020-09-03 Thread Eli Schwartz
On 9/2/20 10:47 PM, Allan McRae wrote:
> FS#61661 notes that we have a comment "Defaults" value for BUILDENV and 
> OPTIONS
> but that does not necessarily correspond to what the example makepkg.conf 
> sets.
> 
> Change the comment to "Makepkg defaults" to indicate this is what makepkg will
> do unless told otherwise.
ACK.

This seems like a very reasonable clarification to make and I really
don't want to back out 44f3a157983e903f926b4f11ddb3f57d111e60f9 :D

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] repo-add: add --include-sigs option

2020-09-03 Thread Eli Schwartz
On 9/2/20 11:02 PM, Allan McRae wrote:
> Pacman now downloads the signature files for all packages when present in a
> repository.  That makes distributing signatures within repository databases
> redundant and costly.
> 
> Do not distribute the package signature files within the repo databases by
> default and add an --include-sigs to revert to the old behaviour.

As I've mentioned on the list before, I would like an --ignore-sigs
option and continue to distribute sigs by default for pacman 6.0

In pacman 6.1 we'll switch by default to ignoring them, and let people
use --include-sigs to revert to the old behavior.

Ignoring sigs right out of the gate means the default behavior of
repo-add is to be unusable for people upgrading from pacman N-1. For
example, Arch Linux would most certainly need to use the option to
provide backwards compat while upgrading. So do third-party repositories.

Also: this option cannot be added to scripts ahead of time, since
repo-add will error on an unknown option, and it cannot be added after
the fact, since some packages will be broken in the meantime.

I don't see what the rush is here to add behavior that no one will want
to use.
- It makes sense to make this configurable now that it's useful to be
  able to ignore them.
- At the same time, defaults should be based on what is more likely for
  people to want.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Add pacman-conf zsh completions

2020-09-03 Thread Eli Schwartz
On 9/3/20 9:38 PM, Andrew Gregory wrote:
> On 08/21/20 at 08:20pm, Ronan Pigott wrote:
>> From: Ronan Pigott 
>>
>> Signed-off-by: Ronan Pigott 
> 
> Sure, I guess.  pacman-conf is meant for use in scripts; who on Earth
> is running it in a terminal?

It can be pretty useful for running without options in order to
copy-paste your pacman.conf for help, but that's again not really a use
case for completions.

...

I guess we want the same completions in bash_completion.in, but given I
don't really have a good use for it myself, I'm not sure I'll bother
writing it myself...

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] NO_MSGSIGNAL and SIGPOLL don't exist on darwin

2020-08-31 Thread Eli Schwartz
On 8/31/20 10:52 PM, Cameron Katri wrote:
> On darwin NO_MSGSIGNAL and SIGPOLL, don't exist, so SO_NOSIGPIPE will be used 
> instead, which serves the same function.

IIUC this isn't usable in send(), but only in setsockopt(), so I think
you'll need a more comprehensive solution to ensure it does more than
compile.

Also why are you hardcoding the value rather than using the existing
SO_NOSIGPIPE define wherever it is found?

> From a61c9634831e5aff6cd020148516337dd4f8090f Mon Sep 17 00:00:00 2001
> From: Cameron Katri 
> Date: Mon, 31 Aug 2020 09:50:09 -0400
> Subject: [PATCH] Fix compilation on Darwin
> 
> ---
>  lib/libalpm/util.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index b70a8192..f7d40b78 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -34,6 +34,11 @@
>  #include 
>  #include 
>  
> +/* no MSG_NOSIGNAL support */
> +#ifndef MSG_NOSIGNAL
> +#define MSG_NOSIGNAL 0x1022
> +#endif
> +
>  /* libarchive */
>  #include 
>  #include 
> @@ -557,8 +562,11 @@ static void _alpm_reset_signals(void)
>   int *i, signals[] = {
>   SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, 
> SIGILL,
>   SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, 
> SIGTSTP,
> - SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, 
> SIGTRAP,
> + SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
>   SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
> +#ifndef __APPLE__
> + SIGPOLL,
> +#endif
>   0
>   };
>   struct sigaction def;
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Disable embedded signatures by default

2020-08-30 Thread Eli Schwartz
On 8/26/20 8:26 PM, Anatol Pomozov wrote:
> The purpose of the change is to start using the detached signatures
> codepath. The detached signatures are shipped with repos for a long
> time and pacman can handle it. Now it is time to actually enable it by
> default.
> 
>  "UseEmbeddedSignatures" option has been added as a fallback plan in
> case we find that the detached signatures codepath is broken. Do you
> think this is too much hassle and we should just start using detached
> signatures by default without any fallback config option?

I already stated my opinion.

Firstly, that we should NOT add a configuration option for this, since
it is a burden on the manpage and is completely useless except for
development testing.

Secondly, I believe we should continue to check both, because I see no
compelling reason to reject perfectly working functionality for no
reason, plus you have no way of knowing if there are thirdparty
repositories which locally generate databases with sigs, but then only
upload the packages and databases, but not the sigs (on the grounds that
they won't be used so why bother).

We don't just care about Arch Linux. Also, we don't just care about the
official repos, even for Arch Linux use. Before instituting a breaking
change, we need a better reason than "this is a convenient way for a
pacman developer to test whether or not pacman is broken".

>> This is the right approach, yeah. I was thinking we'd wait until pacman
>> 6.1 before stopping the signature embedding, to provide a transition
>> period for people depending on SigLevel = Required (which should be
>> everyone, and certainly includes Arch!) to upgrade to 6.x before
>> repo-add starts generating databases useless to pacman 5.x
> 
> There are 2 sets of changes that need to be done:
> 1) make pacman to use detached signatures instead of embedded ones
> 2) change "repo-add" to avoid adding embedded signatures
> 
> We should release changes for #1 first, test it, make sure that
> detached signatures fully work (while dbs still have pacman
> 5.x-compatible embedded sigs). And only then release #2 to get smaller
> databases compatible with pacman version >= 6.0.
> 
> I was thinking #1 can be released with 6.0 and #2 with 6.1.

My vote is to not do #1 at all. I do not see why you keep insisting it
"needs" to be done. #2 is all we need to generate test repositories.

Here is a script to generate test repositories:

ssh pkgbuild.com
cd public_html/repo
mkdir x86_64-detachedsigs
cd mkdir x86_64-detachedsigs

bsdtar -xOf ../x86_64/eschwartz.db.tar.gz | awk
'/^%FILENAME%/{getline;print}' | while read -r line; do
cp -v ../x86_64/"$line" ./
done

repo-add eschwartz.db.tar.gz *.pkg.tar*

bsdtar -xOf ../x86_64/eschwartz.db.tar.gz | awk
'/^%FILENAME%/{getline;print}' | while read -r line; do
cp -v ../x86_64/"$line".sig ./
done



Here is a test repo you can verify against:

[eschwartz]
Server = https://pkgbuild.com/~eschwartz/repo/x86_64-detachedsigs/

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 0/1] Introduce "AssumeInstalled" option in the config file

2020-08-30 Thread Eli Schwartz
On 8/30/20 1:18 PM, Damjan Georgievski wrote:
> On Sun, 30 Aug 2020 at 19:13, Eli Schwartz  wrote:
>> aside: for perl, see https://bugs.archlinux.org/task/54887
> 
> I know that issue (hint: see the reporter)
> :)

haha :p I did not notice that.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 0/1] Introduce "AssumeInstalled" option in the config file

2020-08-30 Thread Eli Schwartz
On 8/30/20 1:07 PM, Дамјан Георгиевски wrote:
> I have had at least two or three use cases for this option,
> * one has been since I always have to type
>   pacman -Syu --assume-installed noto-fonts when plasma-integration is updated
> * `pacman -Syu --assume-installed perl` since it's needlesly pulled by openssl
>   and I didn't need it for containers and similar
> 
> Having this in the config file allows to use the option together with 
> pacstrap too.
> 
> Please review and if the change is acceptable suggest what else needs to be 
> added

I'm not sure it's a good idea to encourage this in the general case.
Supporting it only as a command-line option is still useful since it can
help resolve certain types of manual itervention.

One alternative would be to create a dummy package providing it.

That being said, I'm not necessarily saying this must be rejected...

aside: for perl, see https://bugs.archlinux.org/task/54887

> Дамјан Георгиевски (1):
>   AssumeInstalled option in the config file
> 
>  etc/pacman.conf.in | 1 +
>  src/pacman/conf.c  | 2 ++
>  2 files changed, 3 insertions(+)
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

2020-08-24 Thread Eli Schwartz
On 8/24/20 4:21 PM, Andrew Gregory wrote:
> I'm still a bit skeptical.  I can't say I'm particularly familiar with
> fonts and their configuration, but is there really a valid use case
> for that kind of per-package configuration that we need to support?

I kind of agree, because while it's true font conf files can add new
search paths it's also not unreasonable to just use one unified path, as
fontconfig already recursively searches there and you can organize fonts
by subdirectory.

Nevertheless, thanks Allan for mentioning it, because I really wanted to
know if this could be suitably generic for non-kernel use and this gives
a much better framework for discussion...

> More abstractly, I'm not fond of how much this ties packages together.
> One of the ideas of hooks was for the package that does the work to
> handle all of the necessary details so the triggering packages
> wouldn't have to worry about it.  With this approach, they not only
> have to know to adjust the hook, they also have to know the name of
> the hook and how the hook works.  Changes to the hook then require an
> adjustment and rebuild of any package modifying it.  This is not an
> entirely new problem, user overrides are also dependant on hook
> naming, but this does exacerbate it in a way that I think will be easy
> for packagers to miss and cause breakage.
> 
> What about adding Include support to hooks?  Then hooks that need this
> type of functionality could explicitly include trigger files from
> a particular directory, insulating the process from simple hook
> renaming and hopefully making it more obvious when changes to the hook
> will require changes to any package that modifies it.

I think you're right and Include support would be a better way to go here.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

2020-08-23 Thread Eli Schwartz
ld system, not
the Arch packager -- so this would force the packagers to maintain an
absolute filelist by hand.

Maybe it would be interesting to look into having NoExtract'ed files not
trigger hooks, instead. Users could prevent installation of dracut files
they don't want to use, and as a bonus, if you suddenly decide to use it
you track this by starting to install the file, rather than just
building the initrd and then forgetting the hook doesn't watch for
rebuilds anymore and having it not rebuild when it should.

> Adding more triggers is mostly useful for hook based packages
> depending on other hook based packages (kernel-install and
> mkinitcpio/dracut is a single example of this) where the nested hook
> based package can add its own hook directory as a trigger to the main
> package pacman hook. Assuming a package stores some stuff in a
> non-standard path and needs another package to run on that
> non-standard place when it's updated, it can add a drop-in to add that
> path to the list of triggers for that package's pacman hook.

You don't need to re-explain to me "what" it does, this is evident in
the initial patch submission. :)

But again -- I don't see how this is useful for anything other than one
arguably wrongly-implemented kernel package (which is not actually
implemented yet).

My argument is you "shouldn't" do this.

...

I'm also not sure what "nested" hooks have to do with this, since pacman
hooks aren't nested and kernel-install would only be one hook that
(re)builds an initramfs as part of its internal functionality.

Do you want to be able to have a hook that is also triggered when
another named hook is run? That's a different proposal, but color me
intrigued.

> I realize that these are mostly niceties and I'm not solving some big
> massive issue packaging issue by adding this. Still, since there's not
> that much added complexity in the implementation and the user facing
> part (imo) so adding this seems worth it to me to make some of the
> mentioned use cases a little easier to handle.

I'm definitely up for improving niceties, there's no problem there. :)

My only concern here is that I don't think this is a nicety -- I think
it's an incorrect approach.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

2020-08-23 Thread Eli Schwartz
On 8/22/20 9:12 AM, Daan De Meyer wrote:
> In some scenarios, instead of adding their own hooks, packages want to
> augment hooks already installed by other packages with extra triggers.
> Currently, this requires modifying the existing hook file which is error
> prone and hard to manage for package managers.
> 
> A concrete example where packages would want to extend an existing hook
> is using systemd's kernel-install script on Arch Linux. An example
> pacman hook for kernel-install could be the following:
> 
> ```
> [Trigger]
> Operation = Install
> Operation = Upgrade
> Type = Path
> Target = usr/lib/modules/*/vmlinuz
> Target = usr/lib/kernel/install.d/*
> 
> [Trigger]
> Operation = Install
> Operation = Upgrade
> Type = Package
> Target = systemd
> 
> [Action]
> Description = Adding kernel and initramfs images to /boot...
> When = PostTransaction
> Exec = /etc/pacman.d/scripts/mkosi-kernel-add
> NeedsTargets
> ```
> 
> This hook would run on installation and upgrades of kernel packages and
> every package that installs kernel-install scripts (which includes
> dracut and mkinitcpio). It's also fairly generic in that it doesn't
> include specifics of other packages except its source package (and
> implicitly the install directory of kernel packages).
> 
> However, both mkinitcpio and dracut use the concept of hooks that
> can be installed by other packages to extend their functionality. In
> mkinitcpio these are stored in /usr/lib/initcpio. When a package is
> installed that installs extra hooks for mkinitcpio, the kernel-install
> hook will not trigger. We can fix this by adding /usr/lib/initcpio/* to
> the kernel-install pacman hook but this requires either modifying the
> kernel-install hook when installing mkinitpcio or adding all possible
> directories for all possible packages that want to add triggers to the
> kernel-install hook in the systemd package itself. Neither of these are
> attractive solutions. The first one isn't because we'd have to modify
> the hook when installing packages, the second isn't because it would be
> a huge maintenance burden and doesn't include for AUR or private
> repository packages.

This all seems fairly specific to the kernel itself. As a matter of
curiosity... why can't a mkinitcpio hook declare it is triggered on
/usr/lib/initcpio and a dracut hook declare it is triggered on
/usr/lib/dracut/modules.d/ ?
In fact, the current mkinitcpio hook does precisely this, and is
provided by the mkinitcpio package.

It's all statically coded ahead of time, AFAICT the only reason to do it
this way is to incorrectly put a kernel-install hook into the systemd
package and make the mkinitcpio/dracut packages dynamically load
themselves into said hook.

There is currently no kernel-install pacman hook, and even if one did
want to declare kernel-install the official way to manage kernels on
Arch Linux, the systemd package is the wrong place to put it as systemd
is installed on many Arch systems that don't have a kernel and should
not run kernel-install hooks.

If two packages provide mutually exclusive services, I think it's
reasonable for them to also provide mutually exclusive pacman hooks
geared to that package specifically. It's not like you can even use
kernel-install with both dracut and mkinitcpio installed, since they
both try to create the same initramfs.

More likely, since kernel-install is only catering to a fairly narrow
use case while being installed as part of the base system due to it
being bundled as part of systemd itself, it will be the responsibility
of people who actually like it to manually (no package) install their
own hook to /etc/pacman.d statically coding the desired directory to
watch, which will never ever ever change after the initial installation
process.

It's entirely reasonable for people to create their own custom hooks for
their own system, augmenting the packaged hooks which are only supposed
to be used for cases where it's unambiguously correct to run those
routines. That is why there is a hookdir in /etc. I have a bunch of
hooks which are totally unfit for general packaging but I wouldn't wish
to live without.

Can I hear an argument for where this functionality is generically
useful, and in a case which isn't simply incorrect packaging?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Disable embedded signatures by default

2020-08-11 Thread Eli Schwartz
On 8/11/20 9:24 AM, Allan McRae wrote:
> On 11/8/20 7:44 am, Eli Schwartz wrote:
>> On 8/10/20 5:34 PM, Anatol Pomozov wrote:
>>> Switching from embedded to detached signatures is a big change. This
>>> feature needs to be thoroughly tested before embedded signatures will be
>>> completely removed from the database.
>>>
>>> To help with detached signatures testing we enable it by default. But in
>>> case if an user needs to go back to embedded signatures we add a config
>>> option to reenable it - "UseEmbeddedSignatures".
>> What is the purpose of this? Either signature source should be
>> equivalent, and you should be able to trivially test this by creating a
>> repo with unsigned packages, then bulk-signing the packages after they
>> were repo-added. I don't believe that pacman should include such an
>> end-user option purely to double-check whether a current feature
>> actually works.
> 
> Agreed - the user should not care where the signatures come from, so
> this option should not exist.
> 
> Also, I see this was proposed on arch-dev-public first.  I am not
> subscribed there, and decisions on what is included in pacman are not
> dictated by Arch Linux.  Proposals should be posted here.

More specifically -- decisions on what is included in pacman are not
dictated by consensus of the Arch Linux team, but by the pacman team
(which is in turn guided, but not dictated, by what is useful for
archlinux).

Making a bad or confusing package manager simply because archlinux wants
it, would be a bad move due to making a bad or confusing package manager.

> Now, thinking out loud here...  Would an alternative be to add an
> "--embed-signatures" option to repo-add?  So two versions of a repo
> could be created and those that want to test without embedded signatures
> can.

This is the right approach, yeah. I was thinking we'd wait until pacman
6.1 before stopping the signature embedding, to provide a transition
period for people depending on SigLevel = Required (which should be
everyone, and certainly includes Arch!) to upgrade to 6.x before
repo-add starts generating databases useless to pacman 5.x

But I'd also be fine with --no-embed-signatures for opting in early, and
switching to --embed-signatures for opting out once we default to --no-*

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] makepkg: --source should download repos with PGP signatures

2020-08-10 Thread Eli Schwartz
We optimize this out for sourceballs since VCS sources don't get their
checksums verified. But this logic is broken ever since we implemented
PGP signature checking for git sources -- if the git source is signed,
we still check it, but we don't make sure to download it first. makepkg
then fails to generate a sourceball unless you previously ran
--verifysource or attempted to build.

Signed-off-by: Eli Schwartz 
---
 scripts/libmakepkg/source.sh.in | 5 -
 scripts/libmakepkg/source/git.sh.in | 9 ++---
 scripts/makepkg.sh.in   | 2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts/libmakepkg/source.sh.in b/scripts/libmakepkg/source.sh.in
index a0c6b662..b95e6be8 100644
--- a/scripts/libmakepkg/source.sh.in
+++ b/scripts/libmakepkg/source.sh.in
@@ -35,7 +35,7 @@ done
 
 download_sources() {
local netfile all_sources
-   local get_source_fn=get_all_sources_for_arch get_vcs=1
+   local get_source_fn=get_all_sources_for_arch get_vcs=1 get_pgp=0
 
msg "$(gettext "Retrieving sources...")"
 
@@ -47,6 +47,9 @@ download_sources() {
novcs)
get_vcs=0
;;
+   getpgp)
+   (( SKIPPGPCHECK )) || get_pgp=1
+   ;;
*)
break
;;
diff --git a/scripts/libmakepkg/source/git.sh.in 
b/scripts/libmakepkg/source/git.sh.in
index 7d191b8d..d090f14e 100644
--- a/scripts/libmakepkg/source/git.sh.in
+++ b/scripts/libmakepkg/source/git.sh.in
@@ -29,13 +29,16 @@ source "$LIBRARY/util/pkgbuild.sh"
 
 
 download_git() {
+   local netfile=$1
+   local query=$(get_uri_query "$netfile")
+
# abort early if parent says not to fetch
if declare -p get_vcs > /dev/null 2>&1; then
-   (( get_vcs )) || return
+   if (( ! get_pgp )) || [[ $query != signed ]]; then
+   (( get_vcs )) || return
+   fi
fi
 
-   local netfile=$1
-
local dir=$(get_filepath "$netfile")
[[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"
 
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 7e8d6805..c9940f0a 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1416,7 +1416,7 @@ if (( SOURCEONLY )); then
download_sources allarch
elif ( (( ! SKIPCHECKSUMS )) || \
( (( ! SKIPPGPCHECK )) && source_has_signatures ) ); 
then
-   download_sources allarch novcs
+   download_sources allarch novcs getpgp
fi
check_source_integrity all
cd_safe "$startdir"
-- 
2.28.0


[pacman-dev] [PATCH] remove more autotools files

2020-08-10 Thread Eli Schwartz
We forgot to remove m4/ in commit 454ea024383eab60295e4c4fdf2c329475887b2c
and now it's tragically reminding me of autotools!

Also take this opportunity to drop some symlinks in lib/libalpm/ for
libcommon source files. In autotools these were built specifically for
libalpm and needed to be available in that directory, but the meson
setup just has libalpm depend on libcommon. So these pseudo source files
aren't needed anymore.

Signed-off-by: Eli Schwartz 
---
 lib/libalpm/ini.c |   1 -
 lib/libalpm/ini.h |   1 -
 lib/libalpm/util-common.c |   1 -
 lib/libalpm/util-common.h |   1 -
 m4/.gitignore |   3 -
 m4/acinclude.m4   | 179 
 m4/gpgme.m4   | 238 --
 7 files changed, 424 deletions(-)
 delete mode 12 lib/libalpm/ini.c
 delete mode 12 lib/libalpm/ini.h
 delete mode 12 lib/libalpm/util-common.c
 delete mode 12 lib/libalpm/util-common.h
 delete mode 100644 m4/.gitignore
 delete mode 100644 m4/acinclude.m4
 delete mode 100644 m4/gpgme.m4

diff --git a/lib/libalpm/ini.c b/lib/libalpm/ini.c
deleted file mode 12
index b16d2f27..
--- a/lib/libalpm/ini.c
+++ /dev/null
@@ -1 +0,0 @@
-../../src/common/ini.c
\ No newline at end of file
diff --git a/lib/libalpm/ini.h b/lib/libalpm/ini.h
deleted file mode 12
index e395fa04..
--- a/lib/libalpm/ini.h
+++ /dev/null
@@ -1 +0,0 @@
-../../src/common/ini.h
\ No newline at end of file
diff --git a/lib/libalpm/util-common.c b/lib/libalpm/util-common.c
deleted file mode 12
index cf965176..
--- a/lib/libalpm/util-common.c
+++ /dev/null
@@ -1 +0,0 @@
-../../src/common/util-common.c
\ No newline at end of file
diff --git a/lib/libalpm/util-common.h b/lib/libalpm/util-common.h
deleted file mode 12
index 988c2acc..
--- a/lib/libalpm/util-common.h
+++ /dev/null
@@ -1 +0,0 @@
-../../src/common/util-common.h
\ No newline at end of file
diff --git a/m4/.gitignore b/m4/.gitignore
deleted file mode 100644
index 4bc6b527..
--- a/m4/.gitignore
+++ /dev/null
@@ -1,3 +0,0 @@
-*.m4
-!acinclude.m4
-!gpgme.m4
diff --git a/m4/acinclude.m4 b/m4/acinclude.m4
deleted file mode 100644
index 845c8286..
--- a/m4/acinclude.m4
+++ /dev/null
@@ -1,179 +0,0 @@
-dnl acinclude.m4 - configure macros used by pacman and libalpm
-dnl Add some custom macros for pacman and libalpm
-
-dnl GCC_STACK_PROTECT_LIB
-dnl adds -lssp to LIBS if it is available
-dnl ssp is usually provided as part of libc, but was previously a separate lib
-dnl It does not hurt to add -lssp even if libc provides SSP - in that case
-dnl libssp will simply be ignored.
-AC_DEFUN([GCC_STACK_PROTECT_LIB],[
-  AC_CACHE_CHECK([whether libssp exists], ssp_cv_lib,
-[ssp_old_libs="$LIBS"
- LIBS="$LIBS -lssp"
- AC_TRY_LINK(,, ssp_cv_lib=yes, ssp_cv_lib=no)
- LIBS="$ssp_old_libs"
-])
-  if test $ssp_cv_lib = yes; then
-LIBS="$LIBS -lssp"
-  fi
-])
-
-dnl GCC_STACK_PROTECT_CC
-dnl checks -fstack-protector-all with the C compiler, if it exists then updates
-dnl CFLAGS and defines ENABLE_SSP_CC
-AC_DEFUN([GCC_STACK_PROTECT_CC],[
-  AC_LANG_ASSERT(C)
-  if test "X$CC" != "X"; then
-AC_CACHE_CHECK([whether ${CC} accepts -fstack-protector-all],
-  ssp_cv_cc,
-  [ssp_old_cflags="$CFLAGS"
-   CFLAGS="$CFLAGS -fstack-protector-all"
-   AC_TRY_COMPILE(,, ssp_cv_cc=yes, ssp_cv_cc=no)
-   CFLAGS="$ssp_old_cflags"
-  ])
-if test $ssp_cv_cc = yes; then
-  CFLAGS="$CFLAGS -fstack-protector-all"
-  AC_DEFINE([ENABLE_SSP_CC], 1, [Define if SSP C support is enabled.])
-fi
-  fi
-])
-
-dnl GCC_STACK_CLASH_PROTECTION
-dnl check -fstack-clash-protection with the C compiler, if it exists then
-dnl updates CFLAGS
-AC_DEFUN([GCC_STACK_CLASH_PROTECTION],[
-  AC_LANG_ASSERT(C)
-  if test "X$CC" != "X"; then
-AC_CACHE_CHECK([whether ${CC} accepts -fstack-clash-protection],
-  scp_cv_cc,
-  [scp_old_cflags="$CFLAGS"
-   CFLAGS="$CFLAGS -fstack-clash-protection"
-   AC_TRY_COMPILE(,, scp_cv_cc=yes, scp_cv_cc=no)
-   CFLAGS="$scp_old_cflags"
-  ])
-if test $scp_cv_cc = yes; then
-  CFLAGS="$CFLAGS -fstack-clash-protection"
-fi
-  fi
-])
-
-dnl GCC_FORTIFY_SOURCE_CC
-dnl checks -D_FORTIFY_SOURCE with the C compiler, if it exists then updates
-dnl CPPFLAGS
-AC_DEFUN([GCC_FORTIFY_SOURCE_CC],[
-  AC_LANG_ASSERT(C)
-  if test "X$CC" != "X"; then
-AC_MSG_CHECKING(for FORTIFY_SOURCE support)
-fs_old_cppflags="$CPPFLAGS"
-fs_old_cflags="$CFLAGS"
-CPPFLAGS="$CPPFLAGS -D_FORTIFY_SOURCE=2"
-CFLAGS="$CFLAGS -Werror"
-AC_TRY_COMPILE([#include ], [
-  int main() {
-  #if !(__GNUC_PREREQ (4, 1) )
-  #error No FORTIFY_SOURCE support
-  #endif

Re: [pacman-dev] [PATCH] Disable embedded signatures by default

2020-08-10 Thread Eli Schwartz
On 8/10/20 5:34 PM, Anatol Pomozov wrote:
> Switching from embedded to detached signatures is a big change. This
> feature needs to be thoroughly tested before embedded signatures will be
> completely removed from the database.
> 
> To help with detached signatures testing we enable it by default. But in
> case if an user needs to go back to embedded signatures we add a config
> option to reenable it - "UseEmbeddedSignatures".
What is the purpose of this? Either signature source should be
equivalent, and you should be able to trivially test this by creating a
repo with unsigned packages, then bulk-signing the packages after they
were repo-added. I don't believe that pacman should include such an
end-user option purely to double-check whether a current feature
actually works.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH v3 2/2] repo-add: use more libmakepkg to handle common compression routines

2020-08-05 Thread Eli Schwartz
Currently the list of supported formats for an archive, is maintained in
two places. And repo-add does not actually get updated. :(

In the process, remove some of the logical duplication when calling
bsdtar/compress_as.

Signed-off-by: Eli Schwartz 
---

v3: use get_compression_command to do an early check if we can compress,
thus preserving the old behavior of failing on invalid compression
types.

 scripts/repo-add.sh.in | 32 
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index 160fd93a..7182d1b8 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -44,6 +44,7 @@ USE_COLOR='y'
 PREVENT_DOWNGRADE=0
 
 # Import libmakepkg
+source "$LIBRARY"/util/compress.sh
 source "$LIBRARY"/util/message.sh
 
 # ensure we have a sane umask set
@@ -189,21 +190,13 @@ verify_signature() {
 }
 
 verify_repo_extension() {
-   local repofile=$1
-
-   case $repofile in
-   *.db.tar.gz)  TAR_OPT="-z" ;;
-   *.db.tar.bz2) TAR_OPT="-j" ;;
-   *.db.tar.xz)  TAR_OPT="-J" ;;
-   *.db.tar.zst) TAR_OPT="--zstd" ;;
-   *.db.tar.Z)   TAR_OPT="-Z" ;;
-   *.db.tar) TAR_OPT="" ;;
-   *) error "$(gettext "'%s' does not have a valid database 
archive extension.")" \
-   "$repofile"
-   exit 1 ;;
-   esac
+   local junk=()
+   if [[ $1 = *.db.tar* ]] && get_compression_command "$1" junk; then
+   return 0
+   fi
 
-   printf '%s' "$TAR_OPT"
+   error "$(gettext "'%s' does not have a valid database archive 
extension.")" "$1"
+   exit 1
 }
 
 # write an entry to the pacman database
@@ -522,7 +515,6 @@ rotate_db() {
 }
 
 create_db() {
-   TAR_OPT=$(verify_repo_extension "$REPO_DB_FILE")
# $LOCKFILE is already guaranteed to be absolute so this is safe
dirname=${LOCKFILE%/*}
 
@@ -532,13 +524,13 @@ create_db() {
tempname=$dirname/.tmp.$filename
 
pushd "$tmpdir/$repo" >/dev/null
-   if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then
-   bsdtar -c ${TAR_OPT} -f "$tempname" *
-   else
+   local files=(*)
+   if [[ ${files[*]} = '*' ]]; then
# we have no packages remaining? zip up some emptyness
warning "$(gettext "No packages remain, creating empty 
database.")"
-   bsdtar -c ${TAR_OPT} -f "$tempname" -T /dev/null
+   files=(-T /dev/null)
fi
+   bsdtar -cf - "${files[@]}" | compress_as "$filename" > 
"$tempname"
popd >/dev/null
 
create_signature "$tempname"
@@ -656,7 +648,7 @@ else
LOCKFILE=$PWD/$REPO_DB_FILE.lck
 fi
 
-verify_repo_extension "$REPO_DB_FILE" >/dev/null
+verify_repo_extension "$REPO_DB_FILE"
 
 REPO_DB_PREFIX=${REPO_DB_FILE##*/}
 REPO_DB_PREFIX=${REPO_DB_PREFIX%.db.*}
-- 
2.28.0


[pacman-dev] [PATCH v3 1/2] libmakepkg: extend compress.sh to also permit checking validity

2020-08-05 Thread Eli Schwartz
get_compression_command() can now be used to do upfront checks for
whether a given extension is known to do something successfully. This is
useful when writing tools in which an unknown compression type is a
fatal error.

Signed-off-by: Eli Schwartz 
---

v3: split commit, this first part implements a function useful for
checking if we have a known good compression type

 scripts/libmakepkg/util/compress.sh.in | 53 +++---
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/scripts/libmakepkg/util/compress.sh.in 
b/scripts/libmakepkg/util/compress.sh.in
index 16420beb..d35a01fa 100644
--- a/scripts/libmakepkg/util/compress.sh.in
+++ b/scripts/libmakepkg/util/compress.sh.in
@@ -24,6 +24,7 @@ LIBMAKEPKG_UTIL_COMPRESS_SH=1
 LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
 
 source "$LIBRARY/util/message.sh"
+source "$LIBRARY/util/pkgbuild.sh"
 
 
 # Wrapper around many stream compression formats, for use in the middle of a
@@ -31,20 +32,46 @@ source "$LIBRARY/util/message.sh"
 compress_as() {
# $1: final archive filename extension for compression type detection
 
-   local ext=".tar${1##*.tar}"
+   local cmd ext=${1#${1%.tar*}}
+
+   if ! get_compression_command "$ext" cmd; then
+   warning "$(gettext "'%s' is not a valid archive extension.")" 
"${ext:-${1##*/}}"
+   cat
+   else
+   "${cmd[@]}"
+   fi
+}
+
+# Retrieve the compression command for an archive extension, or cat for .tar,
+# and save it to an existing array name. If the extension cannot be found,
+# clear the array and return failure.
+get_compression_command() {
+   local extarray ext=$1 outputvar=$2
+   local resolvecmd=() fallback=()
 
case "$ext" in
-   *.tar.gz)  ${COMPRESSGZ[@]:-gzip -c -f -n} ;;
-   *.tar.bz2) ${COMPRESSBZ2[@]:-bzip2 -c -f} ;;
-   *.tar.xz)  ${COMPRESSXZ[@]:-xz -c -z -} ;;
-   *.tar.zst) ${COMPRESSZST[@]:-zstd -c -z -q -} ;;
-   *.tar.lrz) ${COMPRESSLRZ[@]:-lrzip -q} ;;
-   *.tar.lzo) ${COMPRESSLZO[@]:-lzop -q} ;;
-   *.tar.Z)   ${COMPRESSZ[@]:-compress -c -f} ;;
-   *.tar.lz4) ${COMPRESSLZ4[@]:-lz4 -q} ;;
-   *.tar.lz)  ${COMPRESSLZ[@]:-lzip -c -f} ;;
-   *.tar) cat ;;
-   *) warning "$(gettext "'%s' is not a valid archive 
extension.")" \
-   "$ext"; cat ;;
+   *.tar.gz)  fallback=(gzip -c -f -n) ;;
+   *.tar.bz2) fallback=(bzip2 -c -f) ;;
+   *.tar.xz)  fallback=(xz -c -z -) ;;
+   *.tar.zst) fallback=(zstd -c -z -q -) ;;
+   *.tar.lrz) fallback=(lrzip -q) ;;
+   *.tar.lzo) fallback=(lzop -q) ;;
+   *.tar.Z)   fallback=(compress -c -f) ;;
+   *.tar.lz4) fallback=(lz4 -q) ;;
+   *.tar.lz)  fallback=(lzip -c -f) ;;
+   *.tar) fallback=(cat) ;;
+   # do not respect unknown COMPRESS* env vars
+   *)array_build "$outputvar" resolvecmd; return 1 ;;
esac
+
+   ext=${ext#*.tar.}
+   if [[ -n $ext ]]; then
+   extarray="COMPRESS${ext^^}[@]"
+   resolvecmd=("${!extarray}")
+   fi
+   if (( ${#resolvecmd[@]} == 0 )); then
+   resolvecmd=("${fallback[@]}")
+   fi
+
+   array_build "$outputvar" resolvecmd
 }
-- 
2.28.0


Re: [pacman-dev] Errors in pacman man pages

2020-07-08 Thread Eli Schwartz
 remote files\\&. "
> "All instances of %u will be replaced with the download URL\\&. If present, "
> "instances of %o will be replaced with the local filename, plus a \\(lq\\&."
> "part\\(rq extension, which allows programs like wget to do file resumes "
> "properly\\&."
> 
> "This option is useful for users who experience problems with built-in HTTP/"
> "FTP support, or need the more advanced proxy support that comes with "
> "utilities like wget\\&."
> --
> Man page: pacman.conf.5
> Issue: Incorrect markup of -U: "" → B<>
> 
> "Set the signature verification level for installing packages using the \"-U"
> "\" operation on a local file\\&. Uses the value from SigLevel as the default"
> "\\&."
> --
> Man page: pacman.conf.5
> Issue: Missing markup of syslog(): syslog() → B()
> 
> "Log action messages through syslog()\\&. This will insert log entries into /"
> "var/log/messages or equivalent\\&."
> --
> Man page: pacman.conf.5
> Issue: Where did "%o" come from? The XferCommand command usage does not have 
> an %o
> 
> "If set, an external program will be used to download all remote files\\&. "
> "All instances of %u will be replaced with the download URL\\&. If present, "
> "instances of %o will be replaced with the local filename, plus a \\(lq\\&."
> "part\\(rq extension, which allows programs like wget to do file resumes "
> "properly\\&."

The rationale here seems to have been that %o is optional.

Currently:

XferCommand = /path/to/command %u

Could be, perhaps:

XferCommand = /path/to/command %u [--out %o]

> --
> Man page: pacman.conf.5
> Issue: [options] should not be translated, wrap it with B<>
> 
> "The I directive is valid in both the [options] and repository "
> "sections\\&. If used in [options], it sets a default value for any "
> "repository that does not provide the setting\\&."
> --
> Man page: pacman-conf.8
> Issue: remote → repository
> 
> "B<-r, --repo> EremoteE"
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Fix srcinfo sometimes printing double newline at the end

2020-06-25 Thread Eli Schwartz
On 6/25/20 7:51 PM, Denton Liu wrote:
> Hi Allan,
> 
> On Fri, Jun 26, 2020 at 09:43:39AM +1000, Allan McRae wrote:
>> Please do not resend patches.  We have a patch tracking system, which
>> means patches are never lost.
> 
> Sorry about that, coming from other mailing list projects, I didn't
> realise that patches here are automatically tracked. Do you have a link
> to the patch tracker?

https://patchwork.archlinux.org/project/pacman/list/

> (Also, if possible, the v2 that I just sent should be used over the old
> v2 as it contains an updated commit message without the typo and with a
> sign-off.)

I've marked your v2 patch as superseded. Today's v3 (mislabeled "v2" :p)
is now the current iteration.

>> I have given this patch a very low priority. That means with limited
>> time available to me, I will likely review other patches first.  But it
>> will get considered at some stage.
> 
> Thanks!
> 
> -Denton

At first glance it seems fine to me.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Adds a check to -Qo, so that pacman warns the user if the file does not exits

2020-06-17 Thread Eli Schwartz
On 6/16/20 9:35 PM, Pedro Victor wrote:
> That's true, thanks. I sent this patch because I mistyped a path in -Qo and
> it only printed "file %mistyped_path is not owned by any package", without
> warning me that the file did not exist.
> So if this check is put somewhere after the file database query, it will
> warn about mistyped paths to nonexistent files, right?

Yes, that seems reasonable. Can you submit an updated patch?

While you are at it, commit messages are usually in the imperative mood,
e.g. "add a check [...]" rather than "adds a check". See e.g.

https://git-scm.com/docs/SubmittingPatches#imperative-mood
https://chris.beams.io/posts/git-commit/#imperative

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Adds a check to -Qo, so that pacman warns the user if the file does not exits

2020-06-16 Thread Eli Schwartz
On 6/16/20 8:05 PM, Pedro Victor wrote:
> From: Pedro Victor 
> 
> ---
>  src/pacman/query.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 513ddb7a..784936b9 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -209,6 +209,11 @@ static int query_fileowner(alpm_list_t *targets)
>   strcat(rpath + rlen, "/");
>   }
>  
> + if (access(rel_path, F_OK) == -1) {
> + pm_printf(ALPM_LOG_ERROR, _("File does not exist: 
> %s\n"), rel_path);
> + goto targcleanup;
> + }
> +

Note that the current behavior is, if a file has been deleted and a
warning is reported by 'pacman -Qkk', pacman -Qo /path/to/file will
still report which package is *supposed* to it.

>   for(i = packages; i && (!found || is_dir); i = 
> alpm_list_next(i)) {
>   if(alpm_filelist_contains(alpm_pkg_get_files(i->data), 
> rel_path)) {
>   print_query_fileowner(rpath, i->data);
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] doc/pacman.8: fix typo

2020-06-14 Thread Eli Schwartz
Fixes FS#67000

Signed-off-by: Eli Schwartz 
---
 doc/pacman.8.asciidoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/pacman.8.asciidoc b/doc/pacman.8.asciidoc
index fee5aa15..476c16f3 100644
--- a/doc/pacman.8.asciidoc
+++ b/doc/pacman.8.asciidoc
@@ -141,7 +141,7 @@ Options
guest system. See '\--sysroot' instead.
 
 *-v, \--verbose*::
-   Output paths such as as the Root, Conf File, DB Path, Cache Dirs, etc.
+   Output paths such as the Root, Conf File, DB Path, Cache Dirs, etc.
 
 *\--arch* ::
Specify an alternate architecture.
-- 
2.27.0


Re: [pacman-dev] [PATCH] makepkg: guard against undefined git pinned sources

2020-06-10 Thread Eli Schwartz
On 6/10/20 8:51 PM, Allan McRae wrote:
> On 26/5/20 1:52 pm, Eli Schwartz wrote:
>> If something like source=(..."#commit=") is used, e.g. due to failed
>> variable expansion, we try to check out an empty refspec as nothing at
>> all, and end up just running "git checkout". This happens because we
>> fail at variable expansion too -- so let's quote our variables properly
>> and make sure git sees this as an empty refspec, so it can error out.
>>
>> Also make sure it is interpreted as a ref instead of a path.
>>
>> Signed-off-by: Eli Schwartz 
>> ---
>>
>> This ensures that something like https://bugs.archlinux.org/task/66729
>> cannot happen again.
>>
> 
> Patch good.
> 
> Worth checking if this can happen with other VCS too.

Ironically (considering git is the primary VCS which we use and test) it
seems like all the other VCSes quote their args.

Except for svn, which runs

svn update -r ${ref}

and the -r requires an argument and fails if it is not provided.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] makepkg/repo-add: do not accept public-only keys for signing

2020-06-08 Thread Eli Schwartz
If it's not listed by --list-secret-key we don't care if it has been
imported into your keyring, it's unusable. And you might not have a
private key at all in the no-keyid-specified case.

Signed-off-by: Eli Schwartz 
---

Overlaps previous patch to fix GPGKEY handling.

 scripts/makepkg.sh.in  | 2 +-
 scripts/repo-add.sh.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index cc8de5aa..9e75ef17 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1293,7 +1293,7 @@ fi
 # check if gpg signature is to be created and if signing key is valid
 if { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; } || [[ $SIGNPKG == 'y' 
]]; then
SIGNPKG='y'
-   if ! gpg --list-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; then
+   if ! gpg --list-secret-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key %s does not exist in your 
keyring.")" "${GPGKEY}"
else
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index 272d8d22..160fd93a 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -137,7 +137,7 @@ check_gpg() {
fi
 
if (( ! VERIFY )); then
-   if ! gpg --list-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; then
+   if ! gpg --list-secret-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; 
then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key ${GPGKEY} does not 
exist in your keyring.")"
elif (( ! KEY )); then
-- 
2.27.0


[pacman-dev] [PATCH v2] makepkg/repo-add: handle GPGKEY with spaces

2020-06-08 Thread Eli Schwartz
We pass this to gpg -u and this gpg option can accept a number of
different formats, not just the historical hexadecimal fingerprint we
assumed. We should not barf hard if a format is used which happens to
contain spaces.

This also fixes a validation bug. When we initially check if the desired
key is available, we don't quote spaces, so gpg goes ahead and treats
each space-separated string as a *different key* to search for,
returning partial matches, and returning success if at least one key is
found. But gpg --detach-sign -u will certainly not accept multiple keys!

Fixes FS#66949

Signed-off-by: Eli Schwartz 
---

v2: fix case of GPGKEY="" with signing enabled reporting that no keys
exist in the keyring. Only expand the quoted GPGKEY if it is non-empty.

 scripts/libmakepkg/integrity/generate_signature.sh.in | 6 +++---
 scripts/makepkg.sh.in | 2 +-
 scripts/repo-add.sh.in| 8 
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/libmakepkg/integrity/generate_signature.sh.in 
b/scripts/libmakepkg/integrity/generate_signature.sh.in
index aec96c03..748087c2 100644
--- a/scripts/libmakepkg/integrity/generate_signature.sh.in
+++ b/scripts/libmakepkg/integrity/generate_signature.sh.in
@@ -29,12 +29,12 @@ create_signature() {
local ret=0
local filename="$1"
 
-   local SIGNWITHKEY=""
+   local SIGNWITHKEY=()
if [[ -n $GPGKEY ]]; then
-   SIGNWITHKEY="-u ${GPGKEY}"
+   SIGNWITHKEY=(-u "${GPGKEY}")
fi
 
-   gpg --detach-sign --use-agent ${SIGNWITHKEY} --no-armor "$filename" 
&>/dev/null || ret=$?
+   gpg --detach-sign --use-agent "${SIGNWITHKEY[@]}" --no-armor 
"$filename" &>/dev/null || ret=$?
 
 
if (( ! ret )); then
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 7261fb2c..cc8de5aa 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1293,7 +1293,7 @@ fi
 # check if gpg signature is to be created and if signing key is valid
 if { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; } || [[ $SIGNPKG == 'y' 
]]; then
SIGNPKG='y'
-   if ! gpg --list-key ${GPGKEY} &>/dev/null; then
+   if ! gpg --list-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key %s does not exist in your 
keyring.")" "${GPGKEY}"
else
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index 545c2929..272d8d22 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -137,7 +137,7 @@ check_gpg() {
fi
 
if (( ! VERIFY )); then
-   if ! gpg --list-key ${GPGKEY} &>/dev/null; then
+   if ! gpg --list-key ${GPGKEY:+"$GPGKEY"} &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key ${GPGKEY} does not 
exist in your keyring.")"
elif (( ! KEY )); then
@@ -155,11 +155,11 @@ create_signature() {
local ret=0
msg "$(gettext "Signing database '%s'...")" "${dbfile##*/.tmp.}"
 
-   local SIGNWITHKEY=""
+   local SIGNWITHKEY=()
if [[ -n $GPGKEY ]]; then
-   SIGNWITHKEY="-u ${GPGKEY}"
+   SIGNWITHKEY=(-u "${GPGKEY}")
fi
-   gpg --detach-sign --use-agent --no-armor ${SIGNWITHKEY} "$dbfile" 
&>/dev/null || ret=$?
+   gpg --detach-sign --use-agent --no-armor "${SIGNWITHKEY[@]}" "$dbfile" 
&>/dev/null || ret=$?
 
if (( ! ret )); then
msg2 "$(gettext "Created signature file '%s'")" 
"${dbfile##*/.tmp.}.sig"
-- 
2.27.0


[pacman-dev] [PATCH] makepkg/repo-add: handle GPGKEY with spaces

2020-06-08 Thread Eli Schwartz
We pass this to gpg -u and this gpg option can accept a number of
different formats, not just the historical hexadecimal fingerprint we
assumed. We should not barf hard if a format is used which happens to
contain spaces.

This also fixes a validation bug. When we initially check if the desired
key is available, we don't quote spaces, so gpg goes ahead and treats
each space-separated string as a *different key* to search for,
returning partial matches, and returning success if at least one key is
found. But gpg --detach-sign -u will certainly not accept multiple keys!

Fixes FS#66949

Signed-off-by: Eli Schwartz 
---
 scripts/libmakepkg/integrity/generate_signature.sh.in | 6 +++---
 scripts/makepkg.sh.in | 2 +-
 scripts/repo-add.sh.in| 8 
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/libmakepkg/integrity/generate_signature.sh.in 
b/scripts/libmakepkg/integrity/generate_signature.sh.in
index aec96c03..748087c2 100644
--- a/scripts/libmakepkg/integrity/generate_signature.sh.in
+++ b/scripts/libmakepkg/integrity/generate_signature.sh.in
@@ -29,12 +29,12 @@ create_signature() {
local ret=0
local filename="$1"
 
-   local SIGNWITHKEY=""
+   local SIGNWITHKEY=()
if [[ -n $GPGKEY ]]; then
-   SIGNWITHKEY="-u ${GPGKEY}"
+   SIGNWITHKEY=(-u "${GPGKEY}")
fi
 
-   gpg --detach-sign --use-agent ${SIGNWITHKEY} --no-armor "$filename" 
&>/dev/null || ret=$?
+   gpg --detach-sign --use-agent "${SIGNWITHKEY[@]}" --no-armor 
"$filename" &>/dev/null || ret=$?
 
 
if (( ! ret )); then
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 7261fb2c..84150cff 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1293,7 +1293,7 @@ fi
 # check if gpg signature is to be created and if signing key is valid
 if { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; } || [[ $SIGNPKG == 'y' 
]]; then
SIGNPKG='y'
-   if ! gpg --list-key ${GPGKEY} &>/dev/null; then
+   if ! gpg --list-key "${GPGKEY}" &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key %s does not exist in your 
keyring.")" "${GPGKEY}"
else
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index 545c2929..b1b1b2b7 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -137,7 +137,7 @@ check_gpg() {
fi
 
if (( ! VERIFY )); then
-   if ! gpg --list-key ${GPGKEY} &>/dev/null; then
+   if ! gpg --list-key "${GPGKEY}" &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key ${GPGKEY} does not 
exist in your keyring.")"
elif (( ! KEY )); then
@@ -155,11 +155,11 @@ create_signature() {
local ret=0
msg "$(gettext "Signing database '%s'...")" "${dbfile##*/.tmp.}"
 
-   local SIGNWITHKEY=""
+   local SIGNWITHKEY=()
if [[ -n $GPGKEY ]]; then
-   SIGNWITHKEY="-u ${GPGKEY}"
+   SIGNWITHKEY=(-u "${GPGKEY}")
fi
-   gpg --detach-sign --use-agent --no-armor ${SIGNWITHKEY} "$dbfile" 
&>/dev/null || ret=$?
+   gpg --detach-sign --use-agent --no-armor "${SIGNWITHKEY[@]}" "$dbfile" 
&>/dev/null || ret=$?
 
if (( ! ret )); then
msg2 "$(gettext "Created signature file '%s'")" 
"${dbfile##*/.tmp.}.sig"
-- 
2.27.0


Re: [pacman-dev] makepkg: package_...() can export variables to each other

2020-06-05 Thread Eli Schwartz
On 6/5/20 3:59 AM, Allan McRae wrote:
> On 5/6/20 5:39 pm, Erich Eckner wrote:
>> Hi,
>>
>> I stumbled upon the fact, that variables set in one package_...()
>> function of a split package are accessible by following package_...()
>> functions. Is this by design or may I try to provide a patch to restore
>> env after the call to package_...()?
>> Note: packaging-relevant variables (e.g. depends_x86_64) are cleaned
>> across function calls.
>>
>> Simple example:
>>
>> --8<--8<--8<--
>> arch=(any)
>> pkgbase=test
>> pkgname=(ta tb)
>> pkgver=0
>> pkgrel=1
>>
>> package_ta() {
>>   _test='hi, there!'
>> }
>>
>> package_tb() {
>>   echo "$_test"
>> }
>> -->8-->8-->8--
>>
> 
> PKGBUILDs are bash.  Bash scope rules apply.

... with the caveat that makepkg --nobuild && makepkg --noextract will
run prepare() and build() in separate program invocations, so you cannot
*depend* on variables being persisted. And all package_*() functions are
run -- together -- under fakeroot in a separate bash process, so they
are separated in scope from non-package_*() functions.

The packaging-relevant variables are special, we reset them precisely
because they're documented to only be local to the package that defines
them.

For all other variables, as Allan said it's your job to scope them as
you wish. People usually declare them with the "local" builtin.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline

2020-06-03 Thread Eli Schwartz
On 6/4/20 1:00 AM, Erich Eckner wrote:
> In case my voice counts anything in this discussion: I would prefer the
> empty lines between the sections and at the end to stay. I regularly
> parse .SRCINFOs (or the output of `makepkg --printsrcinfo`) and
> terminate parsing of sections on the empty line (think along the lines
> `sed '/^\S/,/^$/ {...}'`).

I should probably clarify that the empty lines as section separators are
part of the current format specification and as far as I'm concerned,
removing them is simply not up for discussion. Any proposed patch *must*
retain those empty lines.

Your use case is a good example of why. And I have no reason to think
you're the only person in the entire userbase who is parsing srcinfo
content with this assumption.


Removing the *final* trailing newline could in theory be done (any
srcinfo parser would presumably terminate at EOF anyway) and that, I'm
"merely unconvinced" about implementing. I still don't think a
compelling case has been made for it, but I'm willing to be convinced.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline

2020-06-03 Thread Eli Schwartz
On 6/3/20 8:04 PM, Eli Schwartz wrote:
> On 6/3/20 7:26 PM, Denton Liu wrote:
>> When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each
>> section is concluded with an empty line. This means that at the end of
>> the file, an empty line remains. When running `git diff --check`, Git
>> will complain about this as a whitespace error, saying "new blank line
>> at EOF."
> 
> git diff --check isn't necessarily our problem, though...
> 
> But it will only be reported once, the very first time you ever commit
> this .SRCINFO

Also FWIW, I use git diff --check in my pre-commit githook for AUR
packages, but I also generate the .SRCINFO in that very hook, AFTER I
check for whitespace issues. Hence I don't see whitespace issues for the
.SRCINFO file, even for the very first commit. So I'm really not
motivated to change this.

For more details see https://github.com/eli-schwartz/aurpublish/#hooks

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline

2020-06-03 Thread Eli Schwartz
On 6/3/20 7:26 PM, Denton Liu wrote:
> When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each
> section is concluded with an empty line. This means that at the end of
> the file, an empty line remains. When running `git diff --check`, Git
> will complain about this as a whitespace error, saying "new blank line
> at EOF."

git diff --check isn't necessarily our problem, though...

But it will only be reported once, the very first time you ever commit
this .SRCINFO

> Remove the empty line after sections. Replace the empty echo with a
> placeholder `true` call in case in the future, we do want to close the
> section with something.

Now you've also removed the blank line in the middle of the file,
separating the pkgbase section from each pkgname section. That is more
than just the blank line terminating the final pkgname section.

> ---
>  scripts/libmakepkg/srcinfo.sh.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/libmakepkg/srcinfo.sh.in 
> b/scripts/libmakepkg/srcinfo.sh.in
> index 6e783279..e7b5c4be 100644
> --- a/scripts/libmakepkg/srcinfo.sh.in
> +++ b/scripts/libmakepkg/srcinfo.sh.in
> @@ -31,7 +31,7 @@ srcinfo_open_section() {
>  }
>  
>  srcinfo_close_section() {
> - echo
> + true # nothing to be done
>  }
>  
>  srcinfo_write_attr() {
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH v2 1/2] makepkg: correctly handle missing download clients

2020-06-02 Thread Eli Schwartz
This was broken in commit 882e707e40bbade0111cf3bdedbdac4d4b70453b,
which changed 'plain()' messages to go to stdout, which was then
captured as the download client in question: cmdline=("Aborting...").

The result was a very confusing error message e.g.

/usr/share/makepkg/source/file.sh: line 72: $'\E[1m': command not found

or with makepkg --nocolor:

/usr/share/makepkg/source/file.sh: line 72: Aborting...: command not found

The problem here is that we checked to see if an asynchronous subshell,
in our case <(...), failed, by checking if its captured stdout is
non-empty. Which is terrible, and also a limitation of old bash. But
bash 4.4 can use wait $! to retrieve the return value of an asynchronous
subshell. Now we target that as our minimum, we can sanely handle errors
in such functions.

Losing error messages on stdout by capturing them in a variable instead
of printing them, continues to be a problem, but this will be fixed
systematically in a later commit.

Signed-off-by: Eli Schwartz 
---

v2: split out the plain() function redirection into its own patch, which
will be handled more consistently.

 scripts/libmakepkg/source/file.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/source/file.sh.in 
b/scripts/libmakepkg/source/file.sh.in
index 819320c2..2b804564 100644
--- a/scripts/libmakepkg/source/file.sh.in
+++ b/scripts/libmakepkg/source/file.sh.in
@@ -42,7 +42,7 @@ download_file() {
# find the client we should use for this URL
local -a cmdline
IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
-   (( ${#cmdline[@]} )) || exit
+   wait $! || exit
 
local filename=$(get_filename "$netfile")
local url=$(get_url "$netfile")
-- 
2.26.2


[pacman-dev] [PATCH v2 2/2] libmakepkg: fix regression in sending plain() output to stderr

2020-06-02 Thread Eli Schwartz
In commit 882e707e40bbade0111cf3bdedbdac4d4b70453b we changed message
output to go to stdout by default, unless it was an error. The plain()
function doesn't *look* like an error function, but in practice it was
-- it's used to continue multiline messages, and all in-tree uses were
for warning/error.

This is a problem both because we're sending output to the wrong place,
and because in some cases, we were performing error logging from a
function which would otherwise return a value to be captured in a
variable using command substution.

Fix this and straighten out the API by providing two functions: one for
continuing msg output, and one which wraps this by sending output to
stderr, for continuing error output.

Change all callers to use the second function.

Signed-off-by: Eli Schwartz 
---
 scripts/libmakepkg/executable/vcs.sh.in  |  2 +-
 .../libmakepkg/integrity/verify_signature.sh.in  |  2 +-
 scripts/libmakepkg/source/bzr.sh.in  |  8 
 scripts/libmakepkg/source/file.sh.in |  4 ++--
 scripts/libmakepkg/source/git.sh.in  | 14 +++---
 scripts/libmakepkg/source/hg.sh.in   |  8 
 scripts/libmakepkg/source/svn.sh.in  |  4 ++--
 scripts/libmakepkg/util/config.sh.in |  2 +-
 scripts/libmakepkg/util/message.sh.in|  8 
 scripts/libmakepkg/util/source.sh.in |  4 ++--
 scripts/libmakepkg/util/util.sh.in   |  2 +-
 scripts/makepkg.sh.in| 16 
 12 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/scripts/libmakepkg/executable/vcs.sh.in 
b/scripts/libmakepkg/executable/vcs.sh.in
index 6eb93fae..436b82db 100644
--- a/scripts/libmakepkg/executable/vcs.sh.in
+++ b/scripts/libmakepkg/executable/vcs.sh.in
@@ -43,7 +43,7 @@ get_vcsclient() {
# if we didn't find an client, return an error
if [[ -z $client ]]; then
error "$(gettext "Unknown download protocol: %s")" "$proto"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit $E_CONFIG_ERROR
fi
 
diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in 
b/scripts/libmakepkg/integrity/verify_signature.sh.in
index a1bf7f1c..2be5c493 100644
--- a/scripts/libmakepkg/integrity/verify_signature.sh.in
+++ b/scripts/libmakepkg/integrity/verify_signature.sh.in
@@ -112,7 +112,7 @@ check_pgpsigs() {
 
if (( warnings )); then
warning "$(gettext "Warnings have occurred while verifying the 
signatures.")"
-   plain "$(gettext "Please make sure you really trust them.")"
+   plainerr "$(gettext "Please make sure you really trust them.")"
fi
 }
 
diff --git a/scripts/libmakepkg/source/bzr.sh.in 
b/scripts/libmakepkg/source/bzr.sh.in
index 1227c739..d8e47185 100644
--- a/scripts/libmakepkg/source/bzr.sh.in
+++ b/scripts/libmakepkg/source/bzr.sh.in
@@ -52,7 +52,7 @@ download_bzr() {
msg2 "$(gettext "Branching %s...")" "${displaylocation}"
if ! bzr branch "$url" "$dir" --no-tree --use-existing-dir; then
error "$(gettext "Failure while branching %s")" 
"${displaylocation}"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
fi
elif (( ! HOLDVER )); then
@@ -83,7 +83,7 @@ extract_bzr() {
;;
*)
error "$(gettext "Unrecognized reference: %s")" 
"${fragment}"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
esac
fi
@@ -98,12 +98,12 @@ extract_bzr() {
cd_safe "${dir##*/}"
if ! (bzr pull "$dir" -q --overwrite -r "$rev" && bzr 
clean-tree -q --detritus --force); then
error "$(gettext "Failure while updating working copy 
of %s %s repo")" "${repo}" "bzr"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
fi
elif ! bzr checkout "$dir" -r "$rev"; then
error "$(gettext "Failure while creating working copy of %s %s 
repo")" "${repo}" "bzr"
-  

[pacman-dev] [PATCH] makepkg: guard against undefined git pinned sources

2020-05-25 Thread Eli Schwartz
If something like source=(..."#commit=") is used, e.g. due to failed
variable expansion, we try to check out an empty refspec as nothing at
all, and end up just running "git checkout". This happens because we
fail at variable expansion too -- so let's quote our variables properly
and make sure git sees this as an empty refspec, so it can error out.

Also make sure it is interpreted as a ref instead of a path.

Signed-off-by: Eli Schwartz 
---

This ensures that something like https://bugs.archlinux.org/task/66729
cannot happen again.

 scripts/libmakepkg/source/git.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/source/git.sh.in 
b/scripts/libmakepkg/source/git.sh.in
index aee944f7..a29be3c5 100644
--- a/scripts/libmakepkg/source/git.sh.in
+++ b/scripts/libmakepkg/source/git.sh.in
@@ -125,7 +125,7 @@ extract_git() {
fi
 
if [[ $ref != "origin/HEAD" ]] || (( updating )) ; then
-   if ! git checkout --force --no-track -B makepkg $ref; then
+   if ! git checkout --force --no-track -B makepkg "$ref" --; then
error "$(gettext "Failure while creating working copy 
of %s %s repo")" "${repo}" "git"
plain "$(gettext "Aborting...")"
exit 1
-- 
2.26.2


[pacman-dev] [PATCH] build: add libintl dependency to meson and the .pc file

2020-05-18 Thread Eli Schwartz
In order to use gettext on systems where it is not part of libc, the
correct linker flags are needed in libalpm.pc (for static compilation).
This has never been the case.

The new meson build system currently only checks for ngettext in libc,
but does not fall back to searching for the existence of -lintl; add it
to the libalpm dependencies.

Signed-off-by: Eli Schwartz 
---

Fixes build on MSYS2, and possibly others. Part of
https://github.com/msys2/MSYS2-packages/pull/1951

 lib/libalpm/libalpm.pc.in | 2 +-
 meson.build   | 8 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/libalpm.pc.in b/lib/libalpm/libalpm.pc.in
index 126a6e6a..da7f96aa 100644
--- a/lib/libalpm/libalpm.pc.in
+++ b/lib/libalpm/libalpm.pc.in
@@ -10,4 +10,4 @@ Version: @LIB_VERSION@
 Requires.private: libarchive @pc_crypto@ @pc_libcurl@ @pc_gpgme@
 Cflags: -I${includedir} @LFS_CFLAGS@
 Libs: -L${libdir} -lalpm
-Libs.private: @LIBS@ @pc_gpgme_libs@
+Libs.private: @LIBS@ @pc_gpgme_libs@ @LIBINTL@
diff --git a/meson.build b/meson.build
index 331f02b8..aa3bd4d9 100644
--- a/meson.build
+++ b/meson.build
@@ -77,9 +77,13 @@ conf.set_quoted('CACHEDIR', join_paths(LOCALSTATEDIR, 
'cache/pacman/pkg/'))
 conf.set_quoted('HOOKDIR', join_paths(SYSCONFDIR, 'pacman.d/hooks/'))
 conf.set_quoted('ROOTDIR', ROOTDIR)
 
+libintl = dependency('', required: false)
 if get_option('i18n')
   if not cc.has_function('ngettext')
-error('ngettext not found but NLS support requested')
+libintl = cc.find_library('intl', required : false, static: 
get_option('buildstatic'))
+if not libintl.found()
+  error('ngettext not found but NLS support requested')
+endif
   endif
   conf.set('ENABLE_NLS', 1)
 endif
@@ -301,7 +305,7 @@ libcommon = static_library(
   include_directories : includes,
   install : false)
 
-alpm_deps = [crypto_provider, libarchive, libcurl, gpgme]
+alpm_deps = [crypto_provider, libarchive, libcurl, libintl, gpgme]
 
 libalpm_a = static_library(
   'alpm_objlib',
-- 
2.26.2


[pacman-dev] [PATCH] makepkg: correctly handle missing download clients

2020-05-17 Thread Eli Schwartz
This was broken in commit 882e707e40bbade0111cf3bdedbdac4d4b70453b,
which changed 'plain()' messages to go to stdout, which was then
captured as the download client in question: cmdline=("Aborting...").

The result was a very confusing error message e.g.

/usr/share/makepkg/source/file.sh: line 72: $'\E[1m': command not found

or with makepkg --nocolor:

/usr/share/makepkg/source/file.sh: line 72: Aborting...: command not found

Solve this on two levels:
- redirect this error() continuation to stderr so the user sees it.
- catch erroring returns in get_downloadclient and propagate them

bash 4.4 can use wait $! to retrieve the return value of an asynchronous
subshell such as <(...), which means, now we target that as our minimum,
we can sanely handle errors in such functions.

Signed-off-by: Eli Schwartz 
---

Actually, maybe every use of plain() needs to do this. Or else make
plain() do this by default. But it's technically used to "continue the
previous message", and there's no guarantee it merits going to stderr,
even though every time we use plain(), it does.

What to do...

 scripts/libmakepkg/source/file.sh.in | 1 +
 scripts/libmakepkg/util/source.sh.in | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/source/file.sh.in 
b/scripts/libmakepkg/source/file.sh.in
index 819320c2..28f373fb 100644
--- a/scripts/libmakepkg/source/file.sh.in
+++ b/scripts/libmakepkg/source/file.sh.in
@@ -42,6 +42,7 @@ download_file() {
# find the client we should use for this URL
local -a cmdline
IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
+   wait $! || exit
(( ${#cmdline[@]} )) || exit
 
local filename=$(get_filename "$netfile")
diff --git a/scripts/libmakepkg/util/source.sh.in 
b/scripts/libmakepkg/util/source.sh.in
index e0490661..1bf5b814 100644
--- a/scripts/libmakepkg/util/source.sh.in
+++ b/scripts/libmakepkg/util/source.sh.in
@@ -165,7 +165,7 @@ get_downloadclient() {
if [[ ! -x $program ]]; then
local baseprog="${program##*/}"
error "$(gettext "The download program %s is not installed.")" 
"$baseprog"
-   plain "$(gettext "Aborting...")"
+   plain "$(gettext "Aborting...")" >&2
exit 1 # $E_MISSING_PROGRAM
fi
 
-- 
2.26.2


[pacman-dev] [PATCH] makepkg: libprovides: don't provide both versioned and unversioned sonames

2020-05-14 Thread Eli Schwartz
If multiple files match the pattern libfoo.so*, we want to check each of
them and see if they are shared libraries, and if so, if they have
versions attached.

But some packages can have both shared libraries and random files which
match the filename pattern. This is true at least for files in
/usr/share/gdb/auto-load/, which must match the filename they are paired
with, followed by "-gdb.py" (or some other gdb scripting ext), but
definitely don't contain a shared library. In this case, we don't want
to double-report the library in the generated provides.

It's also possible (probably) for a package to provide a versioned as
well as an unversioned shared library, but in such cases a single
provides entry is sufficient to cover both cases (and the libdepends
for the depending package would contain an unversioned dependency).

Solve this by keeping track of whether we have added a versioned soname
provides already, and then only adding a maximum of one unversioned
provides *iff* there isn't a versioned one yet.

Signed-off-by: Eli Schwartz 
---

Notes:
Currently we only have gdb/auto-load/ scripts for shared libraries in:
- gcc
- glib2
- gstreamer
- efl

In the first 3 cases, libprovides is entirely useless anyway.

libstdc++ is perpetually stable (backwards-compatible ABI) just like glibc.

glib2 actually does use libprovides, despite that it makes no sense for 
glib2
to libprovide anything, as the glib2 developers have this thing where they
always use a soname of "0" and bump ABI versions by changing libglib-2.0.so 
to
libglib-3.0.so -- as a result if you want to track dependencies you need to
change to a different soname manually, and might as well just depend on the
package itself (which already has a version in the pkgname).

(gstreamer is in the same boat as glib2 because it is part of the same 
ecosystem.)

efl doesn't currently libprovide "libeo.so", but if it did, then the issue
would exist and be fixed by this patch. And more generally, any project 
wishing
to implement gdb/auto-load/ scripts could have this issue, so it makes 
sense to
make it work nicely.

 scripts/makepkg.sh.in | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index d1416d15..3f746b77 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -516,9 +516,10 @@ find_libdepends() {
 
 
 find_libprovides() {
-   local p libprovides missing
+   local p newp libprovides missing
for p in "${provides[@]}"; do
missing=0
+   newp=()
case "$p" in
*.so)
mapfile -t filename < <(find "$pkgdir" -type f 
-name $p\*)
@@ -531,7 +532,6 @@ find_libprovides() {
local sofile=$(LC_ALL=C 
readelf -d "$fn" 2>/dev/null | sed -n 's/.*Library soname: \[\(.*\)\].*/\1/p')
if [[ -z "$sofile" ]]; 
then
warning 
"$(gettext "Library listed in %s is not versioned: %s")" "'provides'" "$p"
-   
libprovides+=("$p")
continue
fi
 
@@ -541,25 +541,25 @@ find_libprovides() {
# extract the library 
major version
local 
soversion="${sofile##*\.so\.}"
 
-   
libprovides+=("${p}=${soversion}-${soarch}")
+   
newp+=("${p}=${soversion}-${soarch}")
else
warning "$(gettext 
"Library listed in %s is not a shared object: %s")" "'provides'" "$p"
-   libprovides+=("$p")
fi
done
else
-   libprovides+=("$p")
missing=1
fi
;;
-   *)
-   libprovides+=("$p")
-   ;;
esac
 
if (( missing )); then
warning "$(gettext "Cannot find library list

Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version

2020-05-11 Thread Eli Schwartz
On 5/11/20 10:27 AM, guilla...@manjaro.org wrote:
> Hi Allan, Hi Anatol,
> 
> I was looking to this new multiplexed implementation and I found some
> problems:
> 1) This implementation can lead to download databases from different
> servers, I think it can't be a problem if they are not synced

This should already be the case in the single-download version too, no?

> 2) Differents download payloads are created for databases and its
> signature files, this can this time lead to download the database and
> its signature from differents server, I'm sure it's not wanted.
> 3) Because signature files have their own download payloads, if the
> signature is missing on a the first server (e.g. database signature is
> optional), all next servers will be tried and all will give the same error.

re 2) I cannot remember offhand if this was already the case, but anyway
it's not impossible for one to download the database successfully from
one mirror, then get a sudden network issue on the signature. Falling
back to a second server for optional signatures will just lead to
download errors (which are ignored for sigs), and for required
signatures it will fail the signature check the same way failure to
download the signature at all would fail, because one way or another we
*need* an updated signature.

re both 2) and 3) I guess this could be optimized in the database
download handling to abort on the first 404 but fall through to
additional servers for connection failures. I'm not sure whether this
would be a good thing... how would we detect mirrors that can be
connected to, but no longer host any pacman repos? Checking 5 mirrors
for 404 errors isn't much of an issue.

I guess it could also solve this error I got:

error: failed retrieving file 'core.db.sig' from mirror.epiphyte.network
: Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'testing.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'extra.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'community-testing.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'community.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'multilib.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network
error: failed retrieving file 'multilib-testing.db.sig' from
mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network

But I'd already solved this one by removing the dead mirror from the
bottom of my mirrorlist (it used to be a pretty good fallback one). When
all mirrors fail in the more normal manner at retrieving a signature,
there's no UI results.

> I really wanted to suggest a patch but I didn't find a easy way
> (understand a small patch) to fix it and I would like your opinion on
> point 1) first.
> 
> Regards,
> Guillaume.


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Use absolute path for pacman and pacman-conf in makepkg

2020-05-11 Thread Eli Schwartz
On 5/11/20 9:01 AM, wwijs...@live.nl wrote:
> Right now an unpatched pacman almost works for my use case, but there
> still is one issue with building it. Currently ninja will install the
> bash-completion scripts system wide, regardless of which user runs
> ninja install or what the prefix is. Only if bash-completion is not
> available, will the scripts be installed inside the prefix. The code
> which makes this happen is here: 
> https://git.archlinux.org/pacman.git/tree/meson.build#n46
> 
> This is why I submitted the patch which makes bash-completion optional.
> It may need a different solution, though. Would you take a patch which
> adds an option to install the bash-completion scripts inside the
> prefix? I'd prefer not to maintain any private patches if that's
> possible.

https://git.archlinux.org/users/eschwartz/pacman.git/commit/?h=queue2=b4e3e1c7d93463dffdbb63ff78008df0f57780db

It's simple enough to do, I've just been trying to decide whether I
think it is the right solution before proposing it.


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] libmakepkg/strip: don't re-add the same debug source multiple times

2020-05-10 Thread Eli Schwartz
It's either a waste of work, or triggers edge cases in some packages
(like coreutils-8.31) where the source file is readonly and cp gets a
permission denied error trying to overwrite it with an identical copy of
itself.

Also while we are at it, make the variable names be something readable,
because I could barely tell what this was doing while editing it.

Signed-off-by: Eli Schwartz 
---
 scripts/libmakepkg/tidy/strip.sh.in | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/scripts/libmakepkg/tidy/strip.sh.in 
b/scripts/libmakepkg/tidy/strip.sh.in
index 2b6f732d..868b96f3 100644
--- a/scripts/libmakepkg/tidy/strip.sh.in
+++ b/scripts/libmakepkg/tidy/strip.sh.in
@@ -56,11 +56,14 @@ strip_file() {
fi
 
# copy source files to debug directory
-   local f t
+   local file dest t
while IFS= read -r t; do
-   f=${t/${dbgsrcdir}/"$srcdir"}
-   mkdir -p "${dbgsrc/"$dbgsrcdir"/}${t%/*}"
-   cp -- "$f" "${dbgsrc/"$dbgsrcdir"/}$t"
+   file=${t/${dbgsrcdir}/"$srcdir"}
+   dest="${dbgsrc/"$dbgsrcdir"/}$t"
+   if ! [[ -f $dest ]]; then
+   mkdir -p "${dest%/*}"
+   cp -- "$file" "$dest"
+   fi
done < <(source_files "$binary")
 
# copy debug symbols to debug directory
-- 
2.26.2


Re: [pacman-dev] [PATCH] makepkg: deterministic PKGINFO libprovides for multiple library versions

2020-05-10 Thread Eli Schwartz
On 5/10/20 6:45 PM, anthr...@archlinux.org wrote:
> From: Levente Polyak 
> 
> While iterating over the provides array, the find call for locating a
> shared library may result in listing multiple entries which by itself
> does not produce a stable deterministic order and may vary depending on
> the underlying filesystem.
> To provide a stable listing and a reproducible .PKGINFO file the result
> of find is piped to sort with a static LC_ALL=C localisation.

Wait, what. Do we have packages with libprovides providing multiple
versions of a shared library?

I mean yes, given this possibility it does seem like we'd need to sort
them, but still... why does this exist? :p

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH 3/3] pacman-conf: fix half-baked support for ILoveCandy

2020-05-10 Thread Eli Schwartz
On 5/10/20 4:06 AM, Allan McRae wrote:
> On 10/5/20 2:32 pm, Eli Schwartz wrote:
>> This was only partially implemented in the original implementation.
>> `pacman-conf | grep ILoveCandy` would tell you if it was set, but
>> querying directly by name would not.
>>
> 
> OK.
> 
> Note "half-baked" can have negative connotations, while "incomplete"
> specifies that issue without being accusatory.
Feel free to exchange the commit message with that text, and chalk it up
to late-night coding. :D

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH 3/3] pacman-conf: fix half-baked support for ILoveCandy

2020-05-09 Thread Eli Schwartz
This was only partially implemented in the original implementation.
`pacman-conf | grep ILoveCandy` would tell you if it was set, but
querying directly by name would not.

Signed-off-by: Eli Schwartz 
---
 src/pacman/pacman-conf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index d76c0985..463badf1 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -379,6 +379,8 @@ static int list_directives(void)
show_bool("VerbosePkgLists", config->verbosepkglists);
} else if(strcasecmp(i->data, "DisableDownloadTimeout") == 0) {
show_bool("DisableDownloadTimeout", 
config->disable_dl_timeout);
+   } else if(strcasecmp(i->data, "ILoveCandy") == 0) {
+   show_bool("ILoveCandy", config->chomp);
} else if(strcasecmp(i->data, "NoProgressBar") == 0) {
show_bool("NoProgressBar", config->noprogressbar);
 
-- 
2.26.2


[pacman-dev] [PATCH 2/3] log invalid conf settings as an error

2020-05-09 Thread Eli Schwartz
This is not a warning, _parse_options() returns failure without even
parsing further lines and the attempted pacman/pacman-conf program
execution immediately aborts. Warnings are for when e.g. later on if we
don't recognize a setting at all, we skip over it and have enough
confidence in this to continue executing the program.

The current implementation results in pacman-conf aborting with:

warning: config file /etc/pacman.conf, line 60: invalid value for 
'ParallelDownloads' : '2.5'
error parsing '/etc/pacman.conf'

or pacman -Syu aborting with the entirely more cryptic:

warning: config file /etc/pacman.conf, line 59: invalid value for 
'ParallelDownloads' : '2.5'

and this isn't just a problem for the newly added ParallelDownloads
setting, either, you could get the same problem if you specified a
broken XferCommand, but that's harder as it's more accepting of input
and you probably don't hit this except with unbalanced quotes.

Signed-off-by: Eli Schwartz 
---
 src/pacman/conf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index ac5a5329..3a3ef605 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -665,7 +665,7 @@ static int _parse_options(const char *key, char *value,
} else if(strcmp(key, "XferCommand") == 0) {
char **c;
if((config->xfercommand_argv = wordsplit(value)) == 
NULL) {
-   pm_printf(ALPM_LOG_WARNING,
+   pm_printf(ALPM_LOG_ERROR,
_("config file %s, line %d: 
invalid value for '%s' : '%s'\n"),
file, linenum, "XferCommand", 
value);
return 1;
@@ -717,21 +717,21 @@ static int _parse_options(const char *key, char *value,
 
err = parse_number(value, );
if(err) {
-   pm_printf(ALPM_LOG_WARNING,
+   pm_printf(ALPM_LOG_ERROR,
_("config file %s, line %d: 
invalid value for '%s' : '%s'\n"),
file, linenum, 
"ParallelDownloads", value);
return 1;
}
 
if(number < 1) {
-   pm_printf(ALPM_LOG_WARNING,
+   pm_printf(ALPM_LOG_ERROR,
_("config file %s, line %d: 
value for '%s' has to be positive : '%s'\n"),
file, linenum, 
"ParallelDownloads", value);
return 1;
}
 
if(number > INT_MAX) {
-   pm_printf(ALPM_LOG_WARNING,
+   pm_printf(ALPM_LOG_ERROR,
_("config file %s, line %d: 
value for '%s' is too large : '%s'\n"),
file, linenum, 
"ParallelDownloads", value);
return 1;
-- 
2.26.2


[pacman-dev] [PATCH 1/3] pacman-conf: add support for new ParallelDownloads config option

2020-05-09 Thread Eli Schwartz
This was forgotten in the initial implementation, so it was impossible
to figure out the value from a script, or correctly roundtrip the
config file.

Signed-off-by: Eli Schwartz 
---
 src/pacman/pacman-conf.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index 60739bf6..d76c0985 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -142,6 +142,14 @@ static void show_str(const char *directive, const char 
*val)
printf("%s%c", val, sep);
 }
 
+static void show_int(const char *directive, unsigned int val)
+{
+   if (verbose) {
+   printf("%s = ", directive);
+   }
+   printf("%u%c", val, sep);
+}
+
 static void show_list_str(const char *directive, alpm_list_t *list)
 {
alpm_list_t *i;
@@ -261,6 +269,8 @@ static void dump_config(void)
show_bool("ILoveCandy", config->chomp);
show_bool("NoProgressBar", config->noprogressbar);
 
+   show_int("ParallelDownloads", config->parallel_downloads);
+
show_cleanmethod("CleanMethod", config->cleanmethod);
 
show_siglevel("SigLevel", config->siglevel, 0);
@@ -372,6 +382,9 @@ static int list_directives(void)
} else if(strcasecmp(i->data, "NoProgressBar") == 0) {
show_bool("NoProgressBar", config->noprogressbar);
 
+   } else if(strcasecmp(i->data, "ParallelDownloads") == 0) {
+   show_int("ParallelDownloads", 
config->parallel_downloads);
+
} else if(strcasecmp(i->data, "CleanMethod") == 0) {
show_cleanmethod("CleanMethod", config->cleanmethod);
 
-- 
2.26.2


Re: [pacman-dev] [PATCH] Make bash-completion optional

2020-05-07 Thread Eli Schwartz
On 5/7/20 2:45 PM, Wouter Wijsman wrote:
> The bash completion files were the only reason pacman was not able to
> build as a non-root user. This patch adds the option to not install
> these files. This was needed for my use case, hopefully upstream someone
> else has a use for it as well.

The problem should only happen during install, not during build. I
presume that it is trying to install to e.g.
/usr/share/bash-completion/completions/pacman and raising a PermissionError?

To find the correct install dir for the completions, it first tries to
look at pkg-config to see where pkg-config says they should be
installed. It then falls back on --datadir and installs to
${datadir}/bash-completion/completions/

If you have bash-completion installed and its pkg-config file points at
a root-owned location you'll need root to install pacman. If you don't
have bash-completion installed, then your custom prefix might not
actually be picked up by bash-completion.

I wonder what the right solution is here. In autotools land, ./configure
--help has this to say:

Some influential environment variables:
[...]
  bashcompdir value of completionsdir for bash-completion, overriding
  pkg-config


This is part of the builtin functionality of the pkg.m4 macros which
autotools is using. meson doesn't directly support anything like this.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] doc: remove vim modelines from BUILDINFO(5)

2020-05-05 Thread Eli Schwartz
We (thought we) removed all modelines from the project in commit
860e4c4943ad062bd0eff99f28e7d64804b3c08e, but apparently this one
sneaked in by virtue of this manpage being added to the project after
the "remove all the modelines" patch was submitted, but before it was
applied.

I must have failed to update the patch to remove it from this file also.

Signed-off-by: Eli Schwartz 
---

Found this while pondering to write a pkg.tar(5) manpage.

 doc/BUILDINFO.5.asciidoc | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/doc/BUILDINFO.5.asciidoc b/doc/BUILDINFO.5.asciidoc
index bb300e18..72b297dd 100644
--- a/doc/BUILDINFO.5.asciidoc
+++ b/doc/BUILDINFO.5.asciidoc
@@ -1,6 +1,3 @@
-/
-vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us:
-/
 BUILDINFO(5)
 
 
-- 
2.26.2


Re: [pacman-dev] Setting HOME in build() breaks package signing

2020-05-05 Thread Eli Schwartz
On 5/5/20 4:11 PM, brainpower wrote:
[...]

> This made me wonder:
> Is this something makepkg should take care of (e.g.by restoring $HOME after 
> build() or ensuring gpg will use $OLDHOME/.gnupg)
> or should such a PKGBUILD be considered broken / invalid?

[...]

> $ cat PKGBUILD
> pkgname=broken-home
> pkgver=1
> pkgrel=1
> arch=('any')
> 
> build() {
>   export HOME="${srcdir}/tmphome"
> }
I would argue this should probably be a 'local' variable. e.g.

build() {
local HOME="${srcdir}/tmphome"
some-command-that-needs-fakehome
}

Since HOME is previously marked as exportable, it is still getting
exported, and modifying an exported variable causes the changes to be
picked up. So this is identical:

export somevar
somevar=foo

vs.

export somevar=foo

vs.

somevar=foo
export somevar


And the 'local' attribute coexists with the exported attribute:

$ foo() { declare -p somevar; local somevar=foo; export somevar; env | \
  grep somevar; declare -p somevar; }; foo
bash: declare: somevar: not found
somevar=foo
declare -x somevar="foo"
$ declare -p somevar
bash: declare: somevar: not found


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH 1/2] meson: use better check for debug builds

2020-05-04 Thread Eli Schwartz
meson 0.48 added the 'debug' and 'optimization' builtin options, which
bidirectionally map to the buildtype, but in some cases where debug is
enabled, the builtype may be custom. Checking the 'debug' option lets us
detect every case currently detected, plus a few more, and does so in a
shorter and more concise manner.

Signed-off-by: Eli Schwartz 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 572526b2..680cf62b 100644
--- a/meson.build
+++ b/meson.build
@@ -177,7 +177,7 @@ elif conf.has('HAVE_STRUCT_STATFS_F_FLAGS')
   conf.set('FSSTATSTYPE', 'struct statfs')
 endif
 
-if get_option('buildtype').startswith('debug')
+if get_option('debug')
   extra_cflags = [
 '-Wcast-align',
 '-Wclobbered',
-- 
2.26.2


[pacman-dev] [PATCH 2/2] editorconfig: set meson indentation style

2020-05-04 Thread Eli Schwartz
meson.build gets two-space indents, but our global tabbed default was
overriding this.

Signed-off-by: Eli Schwartz 
---
 .editorconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/.editorconfig b/.editorconfig
index efea87bf..220ffbc8 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -16,5 +16,9 @@ indent_style = tab
 [{NEWS,HACKING}]
 indent_style = space
 
+[meson.build]
+indent_style = space
+indent_size = 2
+
 [*.py]
 indent_style = space
-- 
2.26.2


Re: [pacman-dev] Planning the move to the Arch Linux Gitlab instance

2020-05-04 Thread Eli Schwartz
On 5/2/20 4:29 AM, Allan McRae wrote:
> Hi all,
> 
> Arch Linux is setting up its own Gitlab instance.  I have been playing
> around with it for the day and I think pacman should transition there.
> 
> Note: the current pacman repo on the Arch Gitlab is a playground I have
> been using.  It will be deleted.
> 
> There are some features that are of interest:
>  - CI on merge requests, which will catch build issues automatically.
>  - Review where I can easily see what changed on resubmission
>  - integrated bug tracker, patchwork, ... meaning I spend less time
> updating things on multiple sites.
> 
> We can also do things like provide regular developers a namespace.  e.g.
> I would own allan/* branches, and could prepare work and request merges
> from branches there.
> 
> 
> So how would we proceed?
> 
> Bugs:
> Do we pick a date where we disable new bug submissions on the current
> tracker and point towards gitlab?  Are old bug transferred after some
> ruthless trimming - I don't think this can be automated...  Do we leave
> it open until all bugs get closed?
> 
> Patchwork:
> The got "cleared" somewhat recently, when an update occurred.  At bit of
> effort could followup al the changes needed that never were done and
> clear this out.
> 
> Mailing list:
> New patches would no longer appear here.  I don't know how much we can
> get gitlab to report to the mailing list, or if we even want to.  The
> mailing list would still serve as a good place to discuss feature
> implementation in a more public way.

I know that you're supposed to be able to email patches to a gitlab
address and have it generate a merge request. Not sure how notifications
the other way work, if at all. I'd quite like to have, at a minimum, a
post-receive notification to the mailing list like we currently have...

If we don't allow absolutely anyone to create an account on our gitlab
instance, then they cannot report bugs, or submit patches (even by
email, it's a private email address tied to your account). IIRC devops
were not eager to enable public account creation for various reasons.

> Date:
> What is a good time for all this to happen?  Once Anatol's parallel
> downloads patches are fully committed, I'd like to make a 6.0.0beta1
> release to get some good coverage on these changes.  Could we move at
> the same time?
> 
> Let me have opinions.
-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Bug: using --program-prefix configure option breaks makepkg

2020-05-04 Thread Eli Schwartz
On 5/4/20 5:12 PM, wwijs...@live.nl wrote:
> Thanks for the info! I did manage to fix it for autotools in the way
> gcc does it, for which I've submitted a patch now, but I think you're
> right about Meson. I don't really know much about Meson, though. Are
> you guys planning on keeping autotools supported or is Meson the
> future?

In the long term, we'd like to declare meson feature-compatible and drop
autotools. There is some information here:
https://bugs.archlinux.org/task/64394

I don't know that meson would be adding such a feature, and this was
never explicitly supported (rather, added for free by autotools), so I'm
not sure whether to tell you to do manual moving around or whether to
add an option for it and manually define executable names during
configure time.

>>> I compile with the following commands: ./configure --
>>> prefix=${PSPDEV}
>>> --with-buildscript=PSPBUILD --with-root-dir=${PSPDEV}/psp
>>> --program-prefix="psp-" --disable-doc make make install
>>>
>>> For this to work, I require the following patch:
>>> https://github.com/sharkwouter/psptoolchain/blob/psp-pacman/patches/pacman-5.2.1.patch
>>>
>>>  Is there a way to prevent me from needing this patch? Besides this
>>> patch, the pacman 5.2.1 tarball build and works fine for my
>>> purposes.
>>>
>>> P.S. I did find another small annoyance. Which is that "make
>>> install"
>>> fails if the bash completion files are already installed.
>>
>> Is this because of:
>>
>> for completion in makepkg pacman-key; do \
>> $(LN_S) pacman $(DESTDIR)/$(bashcompdir)/$$completion; \
>> done
>>
>> Because if so, you can probably get it to work by defining LN_S to
>> add
>> the -f flag.
> 
> It seems this can be set in libtool.m4, but that's generated/added by
> autoreconf it seems, so I don't know if that would work. I'll probably
> add a workaround to my build script :/

If it works and covers your use case, then we can simply do this:

$(LN_S) -f pacman $(DESTDIR)/$(bashcompdir)/$$completion

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Showing optional dependencies in pactree output

2020-05-04 Thread Eli Schwartz
On 5/4/20 1:28 PM, Frederick Zhang via pacman-dev wrote:
> Hi all,
> 
> I noticed that there is a patch by Johannes available at [1] which adds
> a new flag to pactree to show optional dependencies as well. I think
> this is a nice feature to have however it seems that the patch was never
> merged.
> 
> I just tried it out locally and managed to compile with a few minor
> changes. If it wasn't rejected for some reason guess I could submit a
> patch if needed?
> 
> [1] https://lists.archlinux.org/pipermail/pacman-dev/2016-February/020922.html

That sounds like a very interesting idea, and I'm not aware of it having
been rejected for any reason. I'd guess it simply got lost.

Please note, however, that in late 2016 pactree was removed from pacman,
along with the other contrib scripts and programs, and refactored into a
separate repo, "pacman-contrib".

The dedicated [pacman-contrib] mailing list collects discussion about
this, and would be an excellent place to submit a patch. :)

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Bug: using --program-prefix configure option breaks makepkg

2020-05-04 Thread Eli Schwartz
On 5/4/20 10:39 AM, Wouter Wijsman wrote:
> Hi Pacman team,
> 
> In the last couple of days I've been trying to build pacman for use
> with a homebrew sdk for the Playstation portable. The idea is for it
> to allow sharing of libraries for the system. To make sure this
> version of pacman doesn't collide with the system's, I use the
> --program-prefix option for the configure script, which adds "psp-"
> in front of all binary names.
> 
> Now when using the --program-prefix option, makepkg is no longer able
> to work. This because it has the name for the pacman executable name
> hardcoded it seems.

I've never really experimented with --program-prefix, but I expect the
general way to do this would be to add those executable names as
replacements to build-aux/edit-script.sh.in (meson) and
scripts/Makefile.am $(edit) (autotools). Is there e.g. a variable set in
the configured Makefile which describes this prefix and can be used to
define a replacement?

Do you know of any examples of other autotools projects which invoke
internal programs via shellscripts and have handled this before?


I think for meson, this would need to be explicitly added as AFAIK meson
doesn't have a comparable feature anyway.

> I compile with the following commands: ./configure --prefix=${PSPDEV}
> --with-buildscript=PSPBUILD --with-root-dir=${PSPDEV}/psp
> --program-prefix="psp-" --disable-doc make make install
> 
> For this to work, I require the following patch:
> https://github.com/sharkwouter/psptoolchain/blob/psp-pacman/patches/pacman-5.2.1.patch
>
>  Is there a way to prevent me from needing this patch? Besides this
> patch, the pacman 5.2.1 tarball build and works fine for my
> purposes.
> 
> P.S. I did find another small annoyance. Which is that "make install"
> fails if the bash completion files are already installed.

Is this because of:

for completion in makepkg pacman-key; do \
$(LN_S) pacman $(DESTDIR)/$(bashcompdir)/$$completion; \
    done

Because if so, you can probably get it to work by defining LN_S to add
the -f flag.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Planning the move to the Arch Linux Gitlab instance

2020-05-04 Thread Eli Schwartz
On 5/4/20 2:20 AM, Anatol Pomozov wrote:
> I've been using review tools like Gerrit and Gitlab at my dayjob for
> almost 10 years now. And personally I find this way of reviewing
> patches more productive than using maillist. Gitlab-like UI provides
> reviewing context, adds syntax colorizing, allows to track what has
> been reviewed and what has not. Plus Gitlab allows to automate some
> boring tasks.

patchwork also provides both syntax coloring and review tracking. Review
context is provided for free. It's really not about these features.

Gitlab is an integrated software forge which provides this all together,
that's all. Better than some in some ways, worse than some in other
ways. But if you value *integration*, then it's a win over having
multiple locations to do everything.

Is there a reason you sound like an infomercial?

>> We can also do things like provide regular developers a namespace.  e.g.
>> I would own allan/* branches, and could prepare work and request merges
>> from branches there.
> 
> That would be great. I know Gerrit tool has $user namespaces where
> people can keep large refactorings before it is ready for a review.

I don't think anyone suggested moving to gerrit... developer namespaces
are not a new concept anyway, you can do them with gitolite too. Gitlab
may provide them as part of the package deal, sure, but we hardly need
gerrit for this...

> We should also invest some efforts into setting up Gitlab automation
> using bots. Bots could run tests, check spelling, run lint, update
> "pacman-git" AUR package etc, etc

How are bots different from CI, which already runs tests? I'm not sure
what you mean by "bots".
(Note that Allan has submitted a patch to add .gitlab-ci.yml, before
this thread even started, so as far as I can tell we don't need to
invest any more efforts as it has already been done. If there are
concerns about it, please address them there.)

What "spelling checks" are you proposing, I don't think I've seen that
before, plus spelling and grammar checkers are not exactly known for
their sterling accuracy.
Are there known problems with spelling in pacman? Prose is pretty
unambiguously right or wrong, so if there are problems it should be a
one-time fix.

No one's updating the pacman-git PKGBUILD, the AUR guidelines forbid
frivolous pkgver bumps on every upstream commit. When changes to e.g the
build() function are needed, I'll be happy to update the PKGBUILD (since
I am the current AUR maintainer), but it will be done by hand, and won't
be because of gitlab...
I already publish and sign binary packages in my custom repo for public
use, and it's my belief that this stage should be manual anyway (I'm not
giving my PGP signing key to a gitlab runner to use whenever it wants).

>> So how would we proceed?
>>
>> Bugs:
>> Do we pick a date where we disable new bug submissions on the current
>> tracker and point towards gitlab?  Are old bug transferred after some
>> ruthless trimming - I don't think this can be automated...
> 
> Do we want to try to move bugs to Gitlab or we start Gitlab bug
> database from scratch? I personally fine with either case but if we
> choose the latter option then bugs.archlinux.org should stay available
> read-only as an archive of the discussions.
> 
> Sven-Hendrik what is the current state of git/flyspray -> gitlab
> migration toolset?

I was under the impression previous flyspray work had been focused on
bugzilla, not gitlab. Anyway, as far as I can tell Sven hasn't been
CC'ed, nor is subscribed to this list, so I'm not sure who you're asking
here.

>>  Do we leave
>> it open until all bugs get closed?
>>
>> Patchwork:
>> The got "cleared" somewhat recently, when an update occurred.  At bit of
>> effort could followup al the changes needed that never were done and
>> clear this out.
>>
>> Mailing list:
>> New patches would no longer appear here.  I don't know how much we can
>> get gitlab to report to the mailing list, or if we even want to.  The
>> mailing list would still serve as a good place to discuss feature
>> implementation in a more public way.
>>
>> Date:
>> What is a good time for all this to happen?  Once Anatol's parallel
>> downloads patches are fully committed, I'd like to make a 6.0.0beta1
>> release to get some good coverage on these changes.  Could we move at
>> the same time?
> 
> I personally would love to try Gitlab for my "parallel-download"
> patchset review. But I guess Gitlab instance is not ready for it
> yet...

Well, if it's not ready it's not ready... so let's just continue the
in-progress review on the mailing list. It's not like there is an urgent
need to see how it works for the sake of learning how it works. We 

Re: [pacman-dev] [PATCH] Adds a "retry?" prompt for when transaction creation fails for user-friendlier fail support.

2020-04-28 Thread Eli Schwartz
On 4/28/20 8:40 PM, Allan McRae wrote:
> On 28/4/20 9:43 am, Pedro wrote:
>> EDIT: Rewrote the prompt message and added a check for the --noconfirm flag
>>
>> >From what I could see, the function trans_init that comes right before the 
>> >prompt has already specific messages
>> for each operation, so I consider that this new implementation fits the 
>> purpose.
>> My motivation to submit this patch is that it's pretty common for 
>> AUR-helpers to automatically invoke pacman after a build,
>> but correct transaction error handling is quite rare. So I thought that to 
>> implement a handling mechanism in the user side
>> would solve the problem not only in that case, but also for other scripts 
>> that invoke pacman.
>>
>> Signed-off-by: Pedro 
>> ---
> 
> I don't see how retrying the same operation with the same input is going
> to help anyone.  If pacman fails in these ways, it is not going to
> succeed the next attempt.
> 
> Allan
Just like I said the first time this patch was submitted -- and now it
just has a slightly longer "retry" message, but it still doesn't answer
my question "what is the user supposed to do about it"?

Pedro, a AUR helper is most likely going to fail because of dependency
checks, which happen later. Do you have examples of a situation where
pacman cannot even create the transaction, let alone populate it with
packages? If so, why is it happening?

These are questions that should be answered before adding a prompt with
no clear purpose.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Adds a "retry?" prompt for when transaction creation fails for user-friendlier fail support.

2020-04-27 Thread Eli Schwartz
What do you propose the user do about this if a transaction fails to be
created? What should the user think if they see the message "Retry?" here?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH v3] Split target output

2020-04-26 Thread Eli Schwartz
   }
> + if (alpm_list_count(upgrade)) {
> + pm_asprintf(_header, _("Upgrading (%lu):"), 
> alpm_list_count(upgrade));
> + list_display(upgrade_header, upgrade, cols);
> + if (alpm_list_count(remove)) {
> + printf("\n");
> + }
> + }
> + if (alpm_list_count(remove)) {
> + pm_asprintf(_header, _("Removing (%lu):"), 
> alpm_list_count(remove));
> + list_display(remove_header, remove, cols);
> + }
>   }
>   printf("\n");
>  
>   table_free(header, rows);
> - FREELIST(names);
> - free(str);
> + if (install) FREELIST(install);
> + if (upgrade) FREELIST(upgrade);
> + if (remove) FREELIST(remove);
> + if (install_header) free(install_header);
> + if (upgrade_header) free(upgrade_header);
> + if (remove_header) free(remove_header);

resolving dependencies...
looking for conflicting packages...
Upgrading (46): bind-tools-9.16.2-2  bluez-libs-5.54-2
checkbashisms-2.20.3-1  containerd-1.3.4-2  cryptsetup-2.3.1-2
dkms-2.8.1-2  fanficfare-3.18.0-1
firefox-developer-edition-76.0b8-1  gedit-3.36.2-1
gnome-terminal-3.36.2-1  gvim-8.2.0510-2  imagemagick-7.0.10.8-1
json-c-0.14-1  lib32-mesa-20.0.5-1
libmfx-20.1.1-1  libslirp-4.3.0-1  lilv-0.24.8-1
linux-firmware-20200421.78c0348-1  man-pages-5.06-2  meson-0.54.1-1
ndctl-68-2
perl-exporter-tiny-1.002002-1  python-distro-1.5.0-1
python-peewee-3.13.3-1  python-sphinx-3.0.3-1  python-wcwidth-0.1.9-1
python2-distro-1.5.0-1
python2-wcwidth-0.1.9-1  qbittorrent-4.2.5-1
rust-1:1.43.0-2  s-nail-14.9.19-1  serd-0.30.4-1  systemd-245.5-2
systemd-libs-245.5-2
systemd-sysvcompat-245.5-2  texinfo-6.7-3  tmux-3.1-1
tzdata-2020a-1  unzip-6.0-14  vim-runtime-8.2.0510-2
vte-common-0.60.2-1  vte3-0.60.2-1
webkit2gtk-2.28.2-1  wine-5.7-1  xapps-1.8.0-1  zip-3.0-9

Total Download Size: 88.22 MiB
Total Installed Size:  1860.56 MiB
Net Upgrade Size: 2.28 MiB
double free or corruption (out)
Aborted

All I have are upgrades.

>   rows = NULL;
>  
>   if(dlsize > 0 || config->op_s_downloadonly) {
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] (no subject)

2020-04-17 Thread Eli Schwartz
On 4/17/20 6:22 PM, Carson Black wrote:
> This asciicast demonstrates the output of this patch: 
> https://asciinema.org/a/321160

Can you use LC_ALL=C for stuff like this? For much the same reason the
patch commit message itself is in English -- if this context helps us
see live what the patch does, and we want to understand it, English is
the generally accepted way to do it.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Touch up proto files

2020-04-13 Thread Eli Schwartz
On 4/12/20 5:01 PM, Daniel M. Capella wrote:
> - split: Fix erroneous $pkgname calls to use $pkgbase

It's not automatically erroneous to use $pkgname in a split package,
${pkgname[0]} is commonly the same value as $pkgbase and there's no
problem using either one. You might need to be more specific for
package_*() alone... but those invocations actually use pkgbase If
this is about making the current inconsistent use of both pkgbase and
pkgname all over the file, then can the commit message mention this?

At the moment it is certainly no *clearer* to the user which one they
might want to use, and why one might recommend avoiding the use of
$pkgname...

> - VCS:
> 
>   - Add missing fields

What is missing about them? These proto files don't need to be a
complete command reference for makepkg variables, the authoritative
documentation which people actually use is PKGBUILD(5). I'd even rather
we remove the unused ones entirely to match how we expect users to write
production PKGBUILDs. :p

At least -vcs and -split probably don't need to repeat everything which
PKGBUILD.proto has prototyped, they only really need to prototype how to
adapt a git source or split packaging function.

>   - Remove unnecessary $srcdir's

This is a stylistic choice, and cd'ing to absolute paths rooted in
$srcdir is useful if you do something like

cd "${srcdir}"/foo
do_something
cd "${srcdir}"/bar
do_something

since cd ../bar can be difficult to follow the more times you do it.

Personally, I always use $srcdir in my cd's.

>   - Move ./autogen.sh to prepare()

Running ./autogen.sh is actually one of my pet peeves, it is flatly
incorrect. GNU autotools documentation considers this to be deprecated
and you should use -- and ensure the build system uses -- autoreconf
when- and where- ever possible (which is nearly always, unless
configure.ac is missing some builtin macro, and that should be fixed).

While I agree that autogen.sh should not be run in PKGBUILD-vcs.proto's
build() function, I would change it to autoreconf -fi at the same time
as moving it to prepare.

> - 80 char line length for comments and make license note stand out

The current comments are already 80 lines, so I'm not sure what this means?

> - Add ./autogen.sh to the other proto's

Continuing on from the previous point about autotools autogen/autoreconf
usage...

For dist tarballs uploaded to ftp sites, autoreconf will have already
been run, there is no need to rerun it except for VCS packages, which is
why PKGBUILD.proto and PKGBUILD-split.proto don't have it at all. I do
not believe we should add this as it violates the expected use of
autotools for stable release tarballs.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Fw: Pacman support for IPFS

2020-04-13 Thread Eli Schwartz
On 4/12/20 9:19 PM, @RubenKelevra wrote:
> Sorry, the mail slipped by - or I had answered sooner.
> 
> Yes, that's possible. The url is indeed ipfs://$CID
> 
> If you got the browser-plugin installed for IPFS, those links will be
> automatically converted to something browser can understand, like
> http://127.0.0.1:8080/ipfs/$CID which then requests the file from the
> local http-gateway of the ipfs-node.
> 
> If you have Opera for Android (which got build in IPFS-Support) those
> ipfs://$CID links will natively work.
> 
> Maybe we could define something like the DLAGENTS in PKGBUILDS, this
> would allow to extend the field in the future:
> 
> ipfs::ipfs://$CID

As far as pacman is concerned, *all* urls will be directly downloaded
with curl. And curl in turn supports many other protocols, though not
AFAICT ipfs. I'd probably recommend running a local http->ipfs proxy for
this. I guess you could also use a custom XferCommand which only
supports ipfs, once you switch your mirrorlist to only use an ipfs server?

How is this content id generated? Is it deterministically generated
based on the file contents, so that repo-add can generate it the same
way it generates the checksum hash, and do so *offline*? Or can
different people end up with different content ids when uploading the
same file?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Use noextract with pacman-conf NoExtract

2020-04-13 Thread Eli Schwartz
On 4/13/20 3:02 AM, Allan McRae wrote:
> On 2/4/20 1:29 am, Eli Schwartz wrote:
>> From: Earnestly 
>>
>> Current code accidently uses noupgrade for the NoExtract directive.
>>
>> https://medium.com/@Code_Analysis/the-last-line-effect-7b1cb7f0d2a1
> 
> Ack.  But we don't need links to irrelevant websites in the commit
> message...

I've directly reposted the patch on Earnestly's behalf, and that is
where the website link came from. Personally, I'm fine with simply
dropping that line. Feel free to do so when pulling the patch.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


[pacman-dev] [PATCH] Use noextract with pacman-conf NoExtract

2020-04-01 Thread Eli Schwartz
From: Earnestly 

Current code accidently uses noupgrade for the NoExtract directive.

https://medium.com/@Code_Analysis/the-last-line-effect-7b1cb7f0d2a1

Signed-off-by: Eli Schwartz 
---

Previously fixed in
https://github.com/andrewgregory/pacutils/commit/64c7d0aca1a3aa3be5b7789f10ee1e5c9e0d7471
but the unfixed version got merged here. :/

 src/pacman/pacman-conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
index 939cbdbd..f67ec21f 100644
--- a/src/pacman/pacman-conf.c
+++ b/src/pacman/pacman-conf.c
@@ -348,7 +348,7 @@ static int list_directives(void)
} else if(strcasecmp(i->data, "NoUpgrade") == 0) {
show_list_str("NoUpgrade", config->noupgrade);
} else if(strcasecmp(i->data, "NoExtract") == 0) {
-   show_list_str("NoExtract", config->noupgrade);
+   show_list_str("NoExtract", config->noextract);
 
 
} else if(strcasecmp(i->data, "Architecture") == 0) {
-- 
2.26.0


Re: [pacman-dev] [PATCH] Use C18 language standard

2020-03-31 Thread Eli Schwartz
On 3/31/20 4:12 PM, Anatol Pomozov wrote:
> C18 is the latest stable specification of the language that is
> supported by modern toolchains.
> It sounds reasonable to me if pacman keeps up with the technology
> stack and uses its advancements.

What advancements are used as a direct result of passing -std=c11? What
generated object code does this patch *on its own*, change?

> Are you saying that C18 requirement introduces limits due to
> requirement of this 2 years old toolchains? Just curious what
> platforms where pacman is used lacks these toolchains?

The limit it introduces is "over-specifying requirements".

> It is not really required to use all the spec features right away. But
> having an option to use it in the future is gonna be useful.
> 
> As of particular use case - if in the future we decide to speed-up the
> installation with more thread-level parallelism (e.g. checking
> signatures/unpacking/... on multiple CPUs) then better C11
> mutlithreadding support is very useful.

If we need it in the future, we can add it in the future. Am I missing
something?

If I add a new pacman feature that relies on libfoo.so, I add a
dependency on libfoo.so -- but does that mean I submit a patch the year
before, to "pass -lfoo when building pacman, to make the libfoo library
an option since it will be useful in the future"? No, I submit a patch
that introduces the use of libfoo functions, and in the same patch, I
add -lfoo to the build flags.

So, it seems reasonable to wait until we want to add c11 features to
pacman, and then submit a patch with the new features, and bump the
minimum-required-language-version argument at the same time.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] meson: use 'pedantic' compiler warning level

2020-03-26 Thread Eli Schwartz
On 3/26/20 4:42 PM, Eli Schwartz wrote:
> On 3/26/20 3:32 PM, Anatol Pomozov wrote:
>> Hello
>>
>> In this case I would like to propose to move forward with my original
>> patch and enable 'pedantic' warning level by default. It will match
>> the flag with the Makefile based build.
> The original patch did not clarify this was about parity with autotools.
> I've looked around a bit and my conclusions are as follows:
> 
> - We always defaulted to -pedantic -D_GNU_SOURCE -Wall in autotools,
>   plus additional flags if --enable-warningflags
> 
> - meson will, with warning_level 3, add -Wall -Wextra -Wpedantic, which
>   means this patch would add the heretofore-unseen -Wextra
>   (warning_level 1 only adds -Wall, 2 adds -Wall -Wextra, no option is
>   available for those who like -Wpedantic but don't want -Wextra)
> 
> Since autotools unconditionally added these flags, parity suggests
> always adding them in meson too. Do we care about -Wextra? We currently
> build with -Wextra without warnings being emitted, anyway.

Allan confirmed on IRC that we do care, and this patch will not be
accepted because we do not want -Wextra.

> If -Wextra is fine then we can just take this current patch, but amend
> it to add -Wextra to configure.ac near
> 
> ```
> if test "x$debug" = "xyes" ; then
> ```
> 
> Anatol, can you also update the commit message to mention that this is
> implementing parity with the default autotools warning level?
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] meson: use 'pedantic' compiler warning level

2020-03-26 Thread Eli Schwartz
On 3/26/20 3:32 PM, Anatol Pomozov wrote:
> Hello
> 
> In this case I would like to propose to move forward with my original
> patch and enable 'pedantic' warning level by default. It will match
> the flag with the Makefile based build.
The original patch did not clarify this was about parity with autotools.
I've looked around a bit and my conclusions are as follows:

- We always defaulted to -pedantic -D_GNU_SOURCE -Wall in autotools,
  plus additional flags if --enable-warningflags

- meson will, with warning_level 3, add -Wall -Wextra -Wpedantic, which
  means this patch would add the heretofore-unseen -Wextra
  (warning_level 1 only adds -Wall, 2 adds -Wall -Wextra, no option is
  available for those who like -Wpedantic but don't want -Wextra)

Since autotools unconditionally added these flags, parity suggests
always adding them in meson too. Do we care about -Wextra? We currently
build with -Wextra without warnings being emitted, anyway.

If -Wextra is fine then we can just take this current patch, but amend
it to add -Wextra to configure.ac near

```
if test "x$debug" = "xyes" ; then
```

Anatol, can you also update the commit message to mention that this is
implementing parity with the default autotools warning level?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] Translation issues

2020-03-17 Thread Eli Schwartz
On 3/17/20 9:53 AM, Luca Weiss wrote:
> On March 17, 2020 5:17:18 AM GMT+01:00, Allan McRae  
> wrote:
>> Looks like this string was fixed a month ago too.
> 
> Great; but that fix is definitely not pulled into git yet as of today ;)

Transifex updates are only checked into git right before a release, to
avoid senseless code churn.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] makepkg: drop duplicate reporting of missing dependencies

2020-03-16 Thread Eli Schwartz
On 3/17/20 12:09 AM, Allan McRae wrote:
> On 6/2/20 10:48 pm, Dave Reisner wrote:
>> When pacman fails to satisfy deps, we might see output like the
>> following:
>>
>> ==> Making package: spiderfoot 3.0-1 (Thu 06 Feb 2020 12:45:10 PM CET)
>> ==> Checking runtime dependencies...
>> ==> Installing missing dependencies...
>> error: target not found: python-pygexf
>> ==> ERROR: 'pacman' failed to install missing dependencies.
>> ==> Missing dependencies:
>>   -> python-dnspython
>>   -> python-exifread
>>   -> python-cherrypy
>>   -> python-beautifulsoup4
>>   -> python-netaddr
>>   -> python-pysocks
>>   -> python-ipwhois
>>   -> python-ipaddress
>>   -> python-phonenumbers
>>   -> python-pypdf2
>>   -> python-stem
>>   -> python-whois
>>   -> python-future
>>   -> python-pyopenssl
>>   -> python-docx
>>   -> python-pptx
>>   -> python-networkx
>>   -> python-cryptography
>>   -> python-secure
>>   -> python-pygexf
>>   -> python-adblockparser
>> ==> Checking buildtime dependencies...
>> ==> ERROR: Could not resolve all dependencies.
>>
>> This is misleading -- the only truly missing package is python-pygexf,
>> but we fail to remove sync-able deps from our deplist and report
>> everything as if it were missing. Simply drop this extra reporting
>> because pacman already tells us exactly what couldn't be resolved.
>> ---
> 
> It seems this went too far:
> 
> Before:
> 
> $ makepkg
> ==> Making package: pacman 5.2.1-4 (Tue 17 Mar 2020 14:05:51)
> ==> Checking runtime dependencies...
> ==> Missing dependencies:
>   -> pacman-mirrorlist
> ==> Checking buildtime dependencies...
> ==> ERROR: Could not resolve all dependencies.
> 
> After:
> 
> $ makepkg
> ==> Making package: pacman 5.2.1-4 (Tue 17 Mar 2020 14:01:13)
> ==> Checking runtime dependencies...
> ==> Checking buildtime dependencies...
> ==> ERROR: Could not resolve all dependencies.
> 
> I have reverted on my patchqueue branch for the time being, but won't
> push to master.  I need more time to look at this...
> 
> Allan
Hrm, the idea here was to rely on handle_deps running pacman -S and
returning pacman's own error messages. But if -s is not invoked, then we
do still need to print it ourselves.

Actually, thinking about this I'm not even sure I understand the patch
in the first place. We're dealing with two errors here: one from pacman,
saying it could not find the sync'able dependency, and one from makepkg,
saying that due to pacman aborting, all of these packages are missing.

Where it might make a difference is if the user reads the error message
from makepkg -s:

==> Installing missing dependencies...
error: target not found: foo
==> ERROR: 'pacman' failed to install missing dependencies.
==> Missing dependencies:
  -> bar
  -> foo

And following that logic, goes and installs pacman -U ./something-which
provides foo, plus pacman -S bar, and then runs makepkg without -s

With this commit, they only install foo, then makepkg fails again due to
bar being missing.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-09 Thread Eli Schwartz
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]

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-09 Thread Eli Schwartz
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
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-08 Thread Eli Schwartz
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".)

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


Re: [pacman-dev] [PATCH] Introduce alpm_dbs_update() function for parallel db updates

2020-03-08 Thread Eli Schwartz
On 3/8/20 4:55 PM, Anatol Pomozov wrote:
>> Note that once review is complete, this patch (and the config one) will
>> sit on a branch until the rest of the work is ready to land in one go.
> 
> It is fine. But please share this branch with me so I can rebase my
> work on top of the reviewed/accepted changes. I do not want to keep
> more than a couple of changes in my development branch. Once review
> for previous changes is done I'll start working on the new changes.
You should in the future be able to find the branch in the same place
Allan publishes other WIP feature branches, which, for future notice, is
here:

https://git.archlinux.org/users/allan/pacman.git/refs/

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-08 Thread Eli Schwartz
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.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-08 Thread Eli Schwartz
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.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-08 Thread Eli Schwartz
 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 "  --nocheckDo 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 "  --packagelistOnly 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;;
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


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

2020-03-08 Thread Eli Schwartz
On 3/8/20 4:04 PM, Carson Black wrote:
> Your feedback on the code should have been addressed.
> 
>> I consider this rpm functionality to be an anti-feature, there are far
>> too many ways to depend on the exact same thing and none of it is
>> opt-in. Also, given there are very simple, intuitive tools like pkgfile
>> or pacman -F, I don't see why such complexity is needed.
> 
> I really don't consider this "too many ways to depend on the exact same
> thing." The entire point is that you're describing what capabilities you 
> want from a package instead of naming the package explicitly.

But that is in fact the exact same thing. You're describing "why" you
want the package, not "what" you want. This is the problem that I have,
because there's an infinite number of "why"s possible.

An rpm can do something like

depends=("/usr/bin/sh")

which seems pretty silly to me! Where does it end?

> And yes, I'm aware of the ability to query packages providing files,
> but looking through filenames is not as efficient a workflow as throwing a
> provides at pacman and letting it find a package that provides it.

pkgfile -q KF5whatever.cmake | pacman -S -

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.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature


  1   2   3   4   5   6   7   >