#2598 F35 Change: Package information on ELF objects
Closed: Rejected 2 years ago by churchyard. Opened 2 years ago by bcotton.

All binaries (executables and shared libraries) are annotated with an ELF note that identifies the rpm distributing this file.


-1

I'm not convinced that this is a good idea, given that it will stamp all binaries with package information, essentially destroying most of the benefits of RPMCoW and messing up the hardlink/reflink stuff done by RPM-OSTree to conserve space and deduplicate content across OSTree revisions.

I share the concerns, but I won't vote yet, as I was not yet able to read the entire discussion about this.

Not in favor - tho not a voting member either.

Primary reason is the churn on the files between releases, eliminating the chance for CoW when we've got a better handle on reproducible compilation long term.
Secondary is that crash report collection implies collation. Existing data includes build-ids which are trivial to index on the collation side and map back to package versions.

Tagging with meeting as it got negative votes.

Metadata Update from @churchyard:
- Issue tagged with: meeting

2 years ago

I haven't been able to get around to this in time for today's meeting (long weekend, yay!!). Please bump it once more until next week.

Sorry for the delay.

Primary reason is the churn on the files between releases, eliminating the chance for CoW when we've got a better handle on reproducible compilation long term.

I looked into this, and I don't see a fundamental conflict. The new ELF section is aligned and padded to a fixed size. So in normal circumstances, the difference in payload between versions is going to be just a few bytes replacement (no length change). We also discussed adding more padding: if it turns out that the section does change size too often, we can safely add more padding without any backwards or forwards compat concerns, based on actual implementation results. And on the consumer side (rmp2extents, etc), rpm2extents already divides the contents into pages and supports reflinking chunks into place [1]. So all that should happen is that one page is always replaced, with no effect on other pages.

Secondary is that crash report collection implies collation. Existing data includes build-ids which are trivial to index on the collation side and map back to package versions.

The change page contains an extensive discussion of when/how build-ids are not trivial to collect. It certainly is possible, but as discussed there, it can be very awkward and require a lot of discipline to do comprehensively. We think that this new approach complements build-ids very nicely: some cases are better handled by build-ids and the existing infra, some cases are handled better by this local/offline stuff.

messing up the hardlink/reflink stuff done by RPM-OSTree to conserve space and deduplicate content across OSTree revisions.

"reflink stuff" — the impact should be small, since reflinking of the unchanged contents will still work as discussed above. "hardlink stuff" — true, maybe. But actually this change only impacts a limited subset of files: the files that most often don't change are data files, translations, artwork, which are out of scope here. And only compiled binaries would change, and my guess is that they would change anyway. So far we haven't been paying attention to build reproducibility too much, but to do some hand waving: most packages are rebuilt at most once every few weeks, and we get updates of glibc/gcc/ld/compressors/major libraries frequently enough so that we don't expect to get bit-identical results. And if we implement "reproducible builds", this does not mean that we get identical builds from different versions, but only that we get an identical build with identical buildroot. So I think it's reasonable to say that even though there will be an impact in certain scenarios, we know that it'll only realistically impact a subset of files from a small subset of packages which build frequently without code changes. (Most packages which build frequently do it precisely because of code changes, like the kernel, systemd, firefox, gcc, etc., and this discussion is moot for them, because the binary text certainly changes.)

I would like to be more concrete, but the RPMCoW change is still undergoing design, so it's impossible to say yet how those two changes will interact in their final form. This proposal seems much closer to being testable (we could push the compilation flags change within a few days, and the systemd consumer side should become available with systemd-249, currently planed for early June), so maybe we can provisionally enable this, to see if it really works out as expected and get some measurable impact statistics. And if it turns out that it breaks CoW significantly, we can reconsider.

[1] https://github.com/rpm-software-management/rpm/pull/1470/commits/82b454d2ff43b39e1a3b38fded0c2e5caffcd336#diff-c68b4ed0c199d4a3de0c0e54682072e05b0d8e4e6337db182ab04170e55870ccR272-R300

You're broadly right: CoW could work with this, but the main simplifying assumption is that every file == one digest. Having a single page change likely requires expanding the model to a list of sub-files and enough logic to identify the common sections. That, or every single binary is a list of page sized digests.

The rebuild speed upstream may be fairly infrequent, and build reproducibility may not be immediate targets, but I've seen much faster continuous integration systems.

The change page contains an extensive discussion of when/how build-ids are not trivial to collect.

I'm not clear on exactly which page you're referencing here. Could you link?

Trying to keep this brief: it'd be nice if coredumpctl told you a package name. My expectation here is that only a small number of people will actually run that. I expect two other use cases to be more prevalent:

  1. Server/container owners: will want to collate crashes across multiple systems.
  2. Most Desktop users likely won't care. Power user might check for an update to a given package version if they see repeated crashes.

The example given[1] that adds a "PACKAGE" column to coredumpctl only lists the top level program's identity. AFAIK the root cause for a given crash may be in a linked library, not the top level program. The only fair way to characterize crashes is to list all binaries like coredumpctl info does. That ends up making the output large and difficult to consume for end users, even with nice package names.

[1] https://fedoraproject.org/wiki/Changes/Package_information_on_ELF_objects#How_To_Test

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/Z7OTP34QOGFU2HI7X4SDIOA6BGQPN4IR/

* #2598 F35 Change: Package information on ELF objects  (mhroncok,
  17:24:31)
  * AGREED: The change in the current form is rejected (+1,3,-2)
    (mhroncok, 17:50:28)
  * The Change can be resubmitted with a more detailed and
    understandable "Benefit to Fedora" section  (mhroncok, 17:50:46)
  * ACTION: zbyszek to resubmit with a more detailed and understandable
    "Benefit to Fedora" section  (mhroncok, 17:51:19) 

Metadata Update from @churchyard:
- Issue close_status updated to: Rejected
- Issue status updated to: Closed (was: Open)

2 years ago

Login to comment on this ticket.

Metadata