Re: [Rpm-maint] [rpm-software-management/rpm] cmake minimum version requirement for rpm? (Discussion #2248)
Speaking with my [CBL-Mariner](https://github.com/microsoft/CBL-Mariner) distro maintainer hat on- no objections to having a floor of cmake 3.20. The lowest version of cmake we support in Mariner at this time is 3.21.4. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/discussions/2248#discussioncomment-3975727 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@DemiMarie pushed 3 commits. ddb8b20e8e8822045eaf5da33b6ec0cf6361be04 Avoid type confusion when verifying signatures f59638fd2e879494ba57fd9f2dc48dfe9e58a90f Check packet types of signatures and public keys e0c93993fe673f6bc8103fc87a4e2c6f3f5c38f9 Reject multiple PGPTAG_PUBLIC_KEY packets -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2242/files/8ba0903e01c1515b992c2306b360c7f1384a5272..e0c93993fe673f6bc8103fc87a4e2c6f3f5c38f9 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] build breaks on libmagic, but it's installed (Issue #2246)
I've got it to the point of building with `make`, but `make check` has a lot of failures. `build.sh` ``` #!/bin/bash set -ex yum install -y \ audit-libs-devel \ autoconf \ automake \ bzip2 \ cmake \ dbus-devel \ doxygen \ elfutils-devel \ file-devel \ file-libs \ gcc \ gettext-devel \ git \ gzip \ libacl-devel \ libarchive-devel \ libcap-devel \ libgcrypt-devel \ libgpg-error-devel \ libselinux-devel \ libsqlite3x-devel \ libtool \ lua-devel \ make \ patch \ pandoc \ popt-devel \ python3-devel \ tar \ xz \ zlib-devel # TODO: temporary hack cat > /usr/lib64/pkgconfig/libmagic.pc < /usr/lib64/pkgconfig/libgcrypt.pc
Re: [Rpm-maint] [rpm-software-management/rpm] unreproducible `rpmdb.sqlite-shm` (Issue #2219)
> The existence of .sqlite-shm is required for read-only WAL mode to work at > all (a very important use-case being queries by regular users), see > https://www.sqlite.org/wal.html#read_only_database I find this weird - because unprivileged code can't write directly to the database, what practical use is the shared memory? Hmm, I just found https://www.sqlite.org/uri.html#uriimmutable - seems like rpm may be able to use that when it detects it can't write? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2219#issuecomment-1292136069 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
Hmm, so the SPECPARTS directory (whether its empty or not at the end of %install) could be used for determining whether the feature was used during a build, so it could be recorded as an rpmlib() dependnecy on src.rpm files similar to DynamicBuildRequires. If we want to - we obviously do not track every new build feature that way. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291952787 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> Well, the sub directory is mainly there so placing specparts on older > rpmbuild versions will fail and they won't just be silently ignored. Yup, I got that. > What about SPECPARTS as dir name. It's loud, but at least it's to the point. Add a %_specpartsdir macro, point it to that and we can move forward. We still got several months to change it if a better name comes to us in a flash of brilliance :laughing: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291906218 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove duplicated code in doSetupMacro (PR #2243)
Heh, a rare genuine vintage dupe. Who knows, may even be worth something :smile: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2243#issuecomment-1291901310 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> > The patch now uses a SPECS sub dir in the buildsubdir
>
> Names are hard, but this directory is nothing at all like SPECS in
> %{_topdir}, I don't think it should share that name. SPEC, maybe. Except, I
> think we may want to use such a directory for other purposes too, which is
> why the spec snippets have *.specpart suffix.
Well, the sub directory is mainly there so placing specparts on older rpmbuild
versions will fail and they won't just be silently ignored. What about
`SPECPARTS` as dir name.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291897664
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
As for the directory name: add a macro for it, and document *that* instead of the hardcoded value. Otherwise, two weeks from the release of 4.19 we'll have a ticket requesting for one. Been there. Also that gives us at least a fleeting chance to rename it should such a thing be needed someday, no matter what we end up calling it here. Other misc observations: - the last three commits should be merged into one, it's a single feature being added there, the earlier ones are obviously just pre-requisite infra work - there's some excess/trailing whitespace in the "Read in spec pieces" commit - it needs a rebase due to #2243 -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291896568 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
On the positive side, spec parse with and without this produce identical results on texlive.spec and kernel.spec (both fairly complicated beasts), except for the expected mkdir on the SPECS dir. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291881156 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request.
> +# Check if dynamic spec generation works
+AT_SETUP([rpmbuild with dynamic spec generation])
+AT_KEYWORDS([build])
+RPMDB_INIT
+AT_CHECK([
+
+run rpmbuild --noclean\
+ -ba "${abs_srcdir}"/data/SPECS/dynamic.spec
+],
+[0],
+[ignore],
+[ignore])
+
+AT_CHECK([
+
+runroot rpm -qp --qf "%{Summary}\n" /build/RPMS/*/hello-docs-1.0-1.*.rpm
...and it avoids globs like these as well.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1156314815
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
@pmatilai commented on this pull request.
> @@ -2272,3 +2272,27 @@ runroot rpmbuild \
],
[ignore])
AT_CLEANUP
+
+# --
+# Check if dynamic spec generation works
+AT_SETUP([rpmbuild with dynamic spec generation])
+AT_KEYWORDS([build])
+RPMDB_INIT
+AT_CHECK([
+
+run rpmbuild --noclean\
+ -ba "${abs_srcdir}"/data/SPECS/dynamic.spec
Use a simpler, noarch package that you can run under runroot instead, what's
being tested here doesn't require a compiler and make to be present at all. It
makes things simpler when you're always operating inside the root and don't
need these abs_srcdir kind of things at all.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1485#pullrequestreview-1156314094
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Spec generation (#1485)
> The patch now uses a SPECS sub dir in the buildsubdir
Names are hard, but this directory is nothing at all like SPECS in %{_topdir},
I don't think it should use this name. SPEC, maybe. Except, I think we may want
to use such a directory for other purposes too, which is why the spec snippets
have *.specpart suffix.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1485#issuecomment-1291860636
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] cmake minimum version requirement for rpm? (Discussion #2248)
It should be possible to build on Debian stable. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/discussions/2248#discussioncomment-3967934 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] cmake minimum version requirement for rpm? (Discussion #2248)
As we're moving to cmake in the next major release, here's a bit of a community poll: what would be a reasonable minimum version to require? Rpm is conservative in its requirements so it's not like we'd require a cmake from last week or even last year, but AIUI cmake is very different from autotools in that things are not expected to break left and right when you update. I initially tossed in version 3.10 as minimum as there were some important looking changes around that time, but it's starting to seem long in the tooth and overly conservative. Buildability on latest RHEL is one of our unwritten requirements, but RHEL 9 and even 8 have cmake 3.20 already. Thoughts from those more familiar with the cmake ecosystem, from other distros perspective etc? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/discussions/2248 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
Readability/style issues aside, yes we need to care about fixing segfaults as long as the internal parser is there. Which might be a while. Thanks for looking into this! Here again though, it's really the subkey stuff that is even more brittle. As the Sequoia option is now in, we could now proceed with ripping all that out of the internal parser to make it considerably less complicated. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2242#issuecomment-1291581547 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@pmatilai commented on this pull request.
> pgpDigAlg sa = sig->alg;
pgpDigAlg ka = key->alg;
- if (sa && sa->verify) {
+ if (sa && sa->verify &&
+ sig->pubkey_algo == key->pubkey_algo) {
Another broken indentation here. A (continued) conditional on the same
indentation level as the next actual line of code is a cardinal sin with no
excuses or exceptions because it's *so* easy to misread.
`indent -kr` would show/fix these.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2242#pullrequestreview-1155946225
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@pmatilai commented on this pull request.
> @@ -1150,9 +1153,13 @@ rpmRC pgpVerifySignature(pgpDigParams key,
> pgpDigParams sig, DIGEST_CTX hashctx)
* done all we can, return NOKEY to indicate "looks okay but dunno."
*/
if (key && key->alg) {
+ if (key->tag != PGPTAG_PUBLIC_KEY &&
+ key->tag != PGPTAG_PUBLIC_SUBKEY)
+ goto exit; /* not a public key */
Indent level is broken, as in difficult to read. With an isKey() helper, that
goes away. And the comment becomes even more redundant than it is already.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2242#pullrequestreview-1155940479
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@pmatilai commented on this pull request. > @@ -1124,6 +1124,9 @@ rpmRC pgpVerifySignature(pgpDigParams key, pgpDigParams > sig, DIGEST_CTX hashctx) if (sig == NULL || ctx == NULL) goto exit; +if (sig->tag != PGPTAG_SIGNATURE) + goto exit; /* not a signature */ This is another very redundant comment, also misplaced. Just drop. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2242#pullrequestreview-1155938713 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@pmatilai commented on this pull request. > const uint8_t *pend = h + hlen; int curve = 0; +if (keyp->tag != PGPTAG_PUBLIC_KEY && keyp->tag != PGPTAG_PUBLIC_SUBKEY) + return rc; /* Not a public key */ +if (keyp->alg) + return rc; /* We can't handle more than one key at a time */ This does deserve the comment, but just leave it above the if, like it originally was. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2242#pullrequestreview-1155937439 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Fix some type confusion bugs in the internal OpenPGP implementation (PR #2242)
@pmatilai commented on this pull request. > const uint8_t *pend = h + hlen; int curve = 0; +if (keyp->tag != PGPTAG_PUBLIC_KEY && keyp->tag != PGPTAG_PUBLIC_SUBKEY) + return rc; /* Not a public key */ This isn't in rpm style of commenting: it's stating the obvious, and code comments should be on their own lines unless there's a very, very good reason. This doesn't. Variable declarations are the one common exception to the rule. As this test repeats elsewhere, I'd suggest adding a (private) helper function/macro, something like isKey(). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2242#pullrequestreview-1155936274 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Lua detection relies on downstream pkg-config patches (Issue #2247)
Lua upstream does not provide a pkg-config file, but we rely on one being present for building rpm. Apparently all relevant distros patch Lua to add that .pc and using one when present is fine, but we should not *rely* on downstream patches. This is the case in both the new cmake build and autotools build in existing releases, but I don't see this getting fixed in current stable versions unless somebody turns up with patches. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2247 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] unreproducible `rpmdb.sqlite-shm` (Issue #2219)
> It'd be nicer of course if rpm had a supported procedure to "park" databases > for this kind of thing. --rebuilddb with some special flag maybe. `--rebuilddb` is much heavier than just a single SQL command. Perhaps `--parkdb`, along with a corresponding C API function? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2219#issuecomment-1291556871 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
