Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Hi,
On Wed, 3 Jun 2020, Eli Schwartz wrote:
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.
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/,/^$/ {...}'`).
For more details see https://github.com/eli-schwartz/aurpublish/#hooks
--
Eli Schwartz
Bug Wrangler and Trusted User
regards,
Erich
-BEGIN PGP SIGNATURE-
iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl7Yf/QACgkQCu7JB1Xa
e1oQuhAArWd0GHhWTk6F4bNS+Vw6PFR7UyJmgUzeipnwBFpXhntUwv4SiygGCaY4
XQFcEByjlhMz6xfL7t2ZfBfxpudH0TlypFdHUV9H2ujgJHi82+1XM13H9R/KMY56
I+YLO5iahPCVu/54licMWiSeb8Vu9ZBOgK5M4bm2YPfcADFaS0FiqZ8bwp3LCe3y
JLiXZ3NOJj4U94OLXA6K1gMUkbn1ZX4TExS+qLGPqLo7W0rPCfZo+FyushNHGjv1
MUUKzDVgXMF06pgN1vCkHWuEmV9HKPibCD1265jbHcgXACzl9qv8wB/AJx21oN6G
UgfzkTxu4LfaoVHsGA0be2vHHcdYYDbQLYJNEVm5DfNj7zeYwp1GFsGIarLz6BQT
FnTpXkni2RFNvVJskDnEAXBsgv0u1Ml5CBJegrPBPhuaFXZ8sk9HVwkb+26DtSQr
/5NyouwErASPDU2kDWnXJ8coJGfqGTmTzAAg1Y2S9/1/uVC9xx9Trya83MBxJ7eF
0bIDWngQNtCuEceEugXtM8W4DqkEFGrWGWiNwX632XhqPmLDuEZBz9HppgNYioHK
ae+rklION7jOCHgu+kHH0DQKwL/V6QViLtL8FYn6M0hHe6ncjxUxta+3FYpmdnTR
Xr98++0JRG9FtWIVOPCGnGU0Qfpig8IJk+dYYv0FNhKvahGn5q4=
=vQdo
-END PGP SIGNATURE-
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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
On 4/6/20 9:26 am, 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."
>
> 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.
> ---
> 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
> }
This is a very strange approach to implementing your idea. Now the
function is useless and should have been removed completely. And now
there is zero separate between sections - not just the last line was
removed.
Anyway, the justification for this patch is missing. Why does makepkg
care about a minor warning from "git diff --check"?
>
> srcinfo_write_attr() {
>
Re: [pacman-dev] [PATCH v3] Fallback to detached signatures during keyring check
On 06/01/20 at 11:45pm, Anatol Pomozov wrote:
> Pacman has a 'key in keyring' verification step that makes sure the signatures
> have a valid keyid. Currently pacman parses embedded package signatures only.
>
> Add a fallback to detached signatures. If embedded signature is missing then
> it
> tries to read corresponding *.sig file and get keyid from there.
>
> Verification:
> debug: found cached pkg:
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
> debug: found detached signature
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig with
> size 310
> debug: found signature key: A5E9288C4FA415FA
> debug: looking up key A5E9288C4FA415FA locally
> debug: key lookup success, key exists
>
> Signed-off-by: Anatol Pomozov
> ---
> lib/libalpm/alpm.h| 11 +++
> lib/libalpm/package.c | 34 ++
> lib/libalpm/sync.c| 18 +-
> lib/libalpm/util.c| 37 +
> lib/libalpm/util.h| 1 +
> 5 files changed, 92 insertions(+), 9 deletions(-)
>
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index c6a13273..0ba57109 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -1403,6 +1403,17 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
> */
> const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
>
> +/** Extracts package signature either from embedded package signature
> + * or if it is absent then reads data from detached signature file.
> + * @param pkg a pointer to package.
> + * @param sig output parameter for signature data. Callee function allocates
> + * a buffer needed for the signature data. Caller is responsible for
> + * freeing this buffer.
> + * @param sig_len output parameter for the signature data length.
> + * @return 0 on success, negative number on error.
alpm_errno_t values are positive.
> + */
> +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, size_t *sig_len);
> +
> /** Returns the method used to validate a package during install.
> * @param pkg a pointer to package
> * @return an enum member giving the validation method
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 5c5fa073..1335b70e 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -268,6 +268,40 @@ const char SYMEXPORT *alpm_pkg_get_base64_sig(alpm_pkg_t
> *pkg)
> return pkg->base64_sig;
> }
>
> +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, size_t
> *sig_len)
> +{
> + ASSERT(pkg != NULL, return -1);
> +
> + if(pkg->base64_sig) {
> + return alpm_decode_signature(pkg->base64_sig, sig, sig_len);
You're returning an error without setting pm_errno.
> + } else {
> + char *pkgpath, *sigpath;
> + alpm_errno_t err;
> + int ret = -1;
> +
> + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> + if(!pkgpath) {
> + RET_ERR(pkg->handle, ALPM_ERR_PKG_NOT_FOUND, -1);
> + }
> + sigpath = _alpm_sigpath(pkg->handle, pkgpath);
> + if(!sigpath || _alpm_access(pkg->handle, NULL, sigpath, R_OK)) {
> + GOTO_ERR(pkg->handle, ALPM_ERR_SIG_MISSING, exit);
> + }
> + err = _alpm_read_file(sigpath, sig, sig_len);
> + if(err == ALPM_ERR_OK) {
> + _alpm_log(pkg->handle, ALPM_LOG_DEBUG, "found detached
> signature %s with size %ld\n",
> + sigpath, *sig_len);
> + } else {
> + GOTO_ERR(pkg->handle, err, exit);
> + }
> + ret = 0;
> +exit:
Change exit to cleanup; exit makes it look like you are terminating
the program.
> + FREE(pkgpath);
> + FREE(sigpath);
> + return ret;
> + }
> +}
> +
> const char SYMEXPORT *alpm_pkg_get_arch(alpm_pkg_t *pkg)
> {
> ASSERT(pkg != NULL, return NULL);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 8c01ad95..9350793a 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -880,18 +880,18 @@ static int check_keyring(alpm_handle_t *handle)
> }
>
> level = alpm_db_get_siglevel(alpm_pkg_get_db(pkg));
> - if((level & ALPM_SIG_PACKAGE) && pkg->base64_sig) {
> - unsigned char *decoded_sigdata = NULL;
> - size_t data_len;
> - int decode_ret = alpm_decode_signature(pkg->base64_sig,
> - _sigdata, _len);
> - if(decode_ret == 0) {
> + if((level & ALPM_SIG_PACKAGE)) {
> + unsigned char *sig = NULL;
> + size_t sig_len;
> + int ret = alpm_pkg_get_sig(pkg, , _len);
> + if(ret == 0) {
> alpm_list_t *keys = NULL;
> -
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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] srcinfo.sh: remove trailing newline
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."
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.
---
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() {
--
2.27.0.rc1.69.g3229fa6e15
