Re: [Rpm-maint] [rpm-software-management/rpm] Drop architecture from %builddir path (PR #3069)

[Panu Matilainen]

@pmatilai pushed 1 commit.

658bc4f097fd0dd050e10b3ee31a31d25fa35be1  Drop architecture from %builddir path

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3069/files/60ca0113b7a935f3e670e390b4720def7fbadd62..658bc4f097fd0dd050e10b3ee31a31d25fa35be1
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] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[David Disseldorp]

Makes sense, thanks for the clarification.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9270632
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] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[Panu Matilainen]

The compressed and uncompressed digests are *alternatives*, both cannot be 
valid at the same time. Rpm calculates both but uses the one that matches (if 
any). This is to allow freely changing between uncompressed and compressed 
format of the same content, which "obviously" must be a legit case.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9270613
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] Drop architecture from %builddir path (PR #3069)

[Panu Matilainen]

This causes more issues than it solves, at least presently. For one, when 
BuildArch is used it typically causes the path to disagree with the actual arch 
(eg on noarch packages). Which looks weird and causes yet other issues in turn. 
The other issue, raised by Neal Gompa, is that it can cause superfluous path 
differences in noarch subpackages, which sharing the noarch package across 
multiple architectures in at least koji.

Suggested-by: Neal Gompa n...@gompa.dev
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/3069

-- Commit Summary --

  * Drop architecture from %builddir path

-- File Changes --

M build/parsePreamble.c (2)
M tests/rpmbuild.at (34)
M tests/rpmspec.at (6)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/3069.patch
https://github.com/rpm-software-management/rpm/pull/3069.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3069
You are receiving this because you are subscribed to this thread.

Message ID: rpm-software-management/rpm/pull/3...@github.com
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[David Disseldorp]

One other thing I noticed is that the rpm header carries the digest of the 
compressed payload in addition to the uncompressed payload digest. Verification 
of the compressed payload alongside `BTRFS_IOC_ENCODED_WRITE` is relatively 
straightforward, but verifying the uncompressed payload would require 
decompression, which we're obviously trying to avoid with this optimization.
I think it's safe to only perform compressed payload verification - zstd frames 
can carry an optional xxHash64 hash, which should be suitable for correctness 
checking if desired.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9270483
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] Code cleanups in tests/rpmpgppubkeyfingerprint.c (PR #3068)

[Panu Matilainen]

> I took the liberty to make this whole source look better.

Better is subjective. The indentation was mostly following rpm coding style, 
now it's not. See Coding style / Indentation in CONTRIBUTING.md. Never, ever 
make formatting changes at the same time as other changes, and in particular, 
don't break what wasn't broken. 

It's impossible to review a change like this. In the light of recent events, 
"small test stuff" certainly needs reviewing too. One thing that did catch my 
eye despite that is pointless NULL checks before freeing. Don't do that, free() 
will do it for you anyhow.

In the meanwhile, there *are* real cleanups/simplifications to be made. Like 
manually calculating string sizes when it could use rasprintf(), and manual 
getcwd() when we have rpmGetCwd() and malloc() with OOM checking for what's a 
static buffer that wouldn't need freeing. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068#issuecomment-2084396253
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] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[David Disseldorp]

If what you mean is that the payload shouldn't be used before it's been checked 
against a digest in the cryptographically verified header, then yes. The 
question is what quantifies as "used".
The payload data needs to go somewhere while we're calculating the digest, in 
which case we could "stream" it to Btrfs in it's compressed, yet-to-be-verified 
form via `BTRFS_IOC_ENCODED_WRITE`. However, doing so would allow any 
concurrent (pre-verification) reads to effectively fuzz the btrfs/zstd 
decompression code path.
One protection mechanism might be to write out the payload in its regular 
(compressed) form before attempting to *switch* those regular extents to 
compressed extents on btrfs via some new `copy_file_range()` flag. The 
regular->compressed extent switch would only be attempted following successful 
payload verification.
I'm very much open to any other suggestions :-)

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9270320
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] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[Demi Marie Obenour]

Do you plan on doing streaming cryptographic verification?  See 
.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9269372
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] API improvement to accommodate for RPM CoW (PR#1470) (Discussion #2057)

[David Disseldorp]

I hacked together a proof of concept implementation which uses 
`BTRFS_IOC_ENCODED_WRITE` to write a zstd-compressed cpio payload directly to 
disk as-is, from a carefully aligned rpm. Compressed extents are then reflinked 
to the installation path.
presentation: 
https://www.youtube.com/live/qNGw8IKaqc0?si=vrLkk8Bi9Odfqm4Z=10325
rpm-rs based source: 
https://github.com/ddiss/rpm/tree/poc-btrfs-zstd-encoded-io-reflink-extract

The rpm payload format isn't modified, although there's a slight "bending" of 
the cpio/newc spec to use the filename field for padding. zstandard frames 
making up the compressed rpm payload explicitly carry both compressed and 
uncompressed lengths, to allow detection of filesystem-supported I/O sizes.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/2057#discussioncomment-9269013
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] Code cleanups in tests/rpmpgppubkeyfingerprint.c (PR #3068)

[Shreenidhi Shedi]

@ffesti I took the liberty to make this whole source look better. Please let me 
know if you want me to make changes in incremental commits. This is a small 
test file which is essentially testing a dummy function, so I made all changes 
in one commit.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068#issuecomment-2083523347
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 file handle leaks (PR #3068)

[Shreenidhi Shedi]

@sshedi pushed 1 commit.

ffc49917d14bde347d2281e0784547cffda7f199  rpmpgppubkeyfingerprint.c: code 
cleanups

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068/files/4befafdf0bd6c625d7817c8a020dde4affa71d90..ffc49917d14bde347d2281e0784547cffda7f199
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 file handle leaks (PR #3068)

[Florian Festi]

@ffesti commented on this pull request.



>   return 1;
 }
 
 if (!fpr || strcmp(got, fpr) != 0) {
fprintf(stderr, "%s:\n got '%s'\nexpected '%s'\n",
 filename, got, fpr ? fpr : "");
free(got);
+   fclose(f);

This should probably also free(fp);

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068#pullrequestreview-2028919643
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 file handle leaks (PR #3068)

[Florian Festi]

@ffesti commented on this pull request.



>   return 1;
 }
 
 // We expect success now.
 char *got = rpmhex(fp, fplen);
 if (! got) {
fprintf(stderr, "%s: rpmhex failed\n", filename);
+   fclose(f);

This should probably also free(fp);

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068#pullrequestreview-2028918055
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] Fix file handle leaks (PR #3068)

[Shreenidhi Shedi]

You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/3068

-- Commit Summary --

  * Fix file handle leaks

-- File Changes --

M tests/rpmpgppubkeyfingerprint.c (9)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/3068.patch
https://github.com/rpm-software-management/rpm/pull/3068.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3068
You are receiving this because you are subscribed to this thread.

Message ID: rpm-software-management/rpm/pull/3...@github.com
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Add error messages for url helper calls (PR #3041)

[Panu Matilainen]

Hmm, and of course we have entirely different way of reporting the error on 
install, eg:

> [root@localhost _build]# ./tools/rpm --define "_urlhelper /bad" -U --nodeps 
> --root /srv/test 
> https://ftp.funet.fi/pub/mirrors/fedora.redhat.com/pub/fedora/linux/development/rawhide/Everything/x86_64/os/Packages/0/0x-0.10-7.fc40.x86_64.rpm
error: Could not find url helper: "/bad": No such file or directory
error: skipping 
https://ftp.funet.fi/pub/mirrors/fedora.redhat.com/pub/fedora/linux/development/rawhide/Everything/x86_64/os/Packages/0/0x-0.10-7.fc40.x86_64.rpm
 - transfer failed

Which isn't so bad. But probably --import and all have their own more cryptic 
variants like the query above. Anyway, if we accept that we call rpmlog() from 
that location, it is an improvement for the user. Which is what matters in the 
end I guess.

Besides tests there are some improvements you can make here:
The exec() can fail for other reasons besides not found, so I'd use a more 
generic error message there. Maybe just "failed to execute url helper". The 
exact error code is available in the child if we move the log there (ignoring 
the wrath of rpmlog after fork), or we could extend our use of the bash scheme 
started in faaa0305f5. From bash(1): "If a command is not found, the child 
process created to execute it returns a status of  127.   If  a  command  is 
found but is not executable, the return status is 126."

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3041#issuecomment-2082634102
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] Fix header source/patch names disagreeing with src.rpm contents (PR #3067)

[Panu Matilainen]

If sources or patches in the spec are defined via a macro that does not yet 
exist, itll still work for building if the macro has been defined by that 
time as theres another round of expansion there. But this can leave the 
source/patch names inserted to the header disagreeing with what actually ended 
up in the source package, eg in the testcase youd previously get 
%{somemacro}-2.0.tar.gz in the header whereas the src.rpm had the 
right contents.

While defining sources this way seems mad and brittle, it does actually work 
for building rpms and theres a whole ecosystem of packages relying on it 
in Fedora. So lets at least be consistent about it, and re-expand the source 
paths once more before inserting in the header, because thats what happens 
for them in the actual build as well.

Originally reported at https://bugzilla.redhat.com/show_bug.cgi?id=2233878
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/3067

-- Commit Summary --

  * Fix header source/patch names disagreeing with src.rpm contents

-- File Changes --

M build/parseSpec.c (6)
A tests/data/SPECS/macrosource.spec (15)
M tests/rpmbuild.at (31)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/3067.patch
https://github.com/rpm-software-management/rpm/pull/3067.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3067
You are receiving this because you are subscribed to this thread.

Message ID: rpm-software-management/rpm/pull/3...@github.com
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] how should a test on given argument --noclean (Discussion #3065)

[Panu Matilainen]

You can't.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/discussions/3065#discussioncomment-9261221
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] Convert macro table to STL containers + native strings (PR #3062)

[Panu Matilainen]

The final commit turns the macro entry stack into an STL container too, making 
the macro entries themselves freestanding and since all this is now standard 
library stuff we don't need to manually clean up and deallocate etc.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3062#issuecomment-2082100677
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] Convert a bunch of librpm stuff to native C++ allocation (and containers) (PR #3066)

[Panu Matilainen]

Nothing hugely interesting in here, see commit 
0d1071b99ada2df920ba657d9e015e0c2259d4b6 for rationale.

There are bits and pieces to finish in librpmio side still but moving on to 
librpm as some of this stuff is linked, eg string pool cannot use C++ 
containers before rpmds is updated to clean up after itself, otherwise there 
will be an unfixable indirect leak from rpmlibProvides_s struct on process exit 
:zany_face: 
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/3066

-- Commit Summary --

  * Natively allocate rpmte structs and temporary colors vector
  * Natively allocate rpmts, rpmtsi, rpmtxn and tsMembers
  * Natively allocate rpmds, rpmfiles and rpmfi mains structs
  * Natively allocate rpmps and rpmprob structs, use vector for prob storage
  * Natively allocate main header related structs
  * Natively allocate bunch of miscellaneous librpm structs
  * Natively allocate main rpmdb related structs
  * Use a vector for index iterator offset storage
  * Use an unordered map for the rpmdb verify cache
  * Natively allocate rpmfs structs
  * Natively allocate fingerprint cache
  * Natively allocate plugin structures, use a vector for storage

-- File Changes --

M lib/backend/dbi.h (6)
M lib/cpio.c (4)
M lib/fprint.c (6)
M lib/header.c (14)
M lib/psm.c (5)
M lib/rpmdb.c (75)
M lib/rpmds.c (5)
M lib/rpmfi.c (13)
M lib/rpmfs.c (57)
M lib/rpmgi.c (6)
M lib/rpmlock.c (6)
M lib/rpmplugins.c (67)
M lib/rpmprob.c (4)
M lib/rpmps.c (46)
M lib/rpmscript.c (8)
M lib/rpmtd.c (4)
M lib/rpmte.c (11)
M lib/rpmts.c (19)
M lib/rpmvs.c (4)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/3066.patch
https://github.com/rpm-software-management/rpm/pull/3066.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3066
You are receiving this because you are subscribed to this thread.

Message ID: rpm-software-management/rpm/pull/3...@github.com
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Convert macro table to STL containers + native strings (PR #3062)

[Panu Matilainen]

@pmatilai pushed 3 commits.

9c28251cdb3664839a5a03b6f6b15f096653483b  Add copy control and in particular, 
destructor to the macro context
3ff1f78c49d63d75b30cc46607edfb148cb5e948  Convert macro table to STL containers 
+ native strings
a06877ec1ed1fd5ac8cbaae8100288a2b220fd5c  Untangle the per-name stack from the 
macro entry struct

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3062/files/e551a36f8300347f90a54e25e73e32c53cfb0a37..a06877ec1ed1fd5ac8cbaae8100288a2b220fd5c
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