Dasharo / dasharo-issues

The Dasharo issue tracker
https://dasharo.com/
24 stars 0 forks source link

Discussion on Enhancing Build Reproducibility for dasharo-pq #889

Open pietrushnic opened 3 months ago

pietrushnic commented 3 months ago

A conversation among team members highlighted an issue with the reproducibility of coreboot builds in the dasharo-pq environment. It was noted that builds are non-reproducible by default due to timestamps embedded within the binaries, which changes the binary output across different builds. Using the BUILD_TIMELESS=1 option improves the situation by removing these timestamps. Still, it removes critical debugging information, such as git commit hashes and line information in asserts, which is sometimes crucial for diagnostics.

The discussion also touched on the practices of building from a git repository versus building from tarballs or applied patches, where the latter requires the BUILD_TIMELESS=1 option to ensure reproducibility. The conversation raised concerns about the balance between reproducibility and the utility of debugging features in development and testing processes. So maybe there should be a change in strategy while using patch queue.

This issue aims to initiate a broader discussion on how to handle these problems in a scalable way, especially considering various build environments and practices like patch queues, container-based builds, and building directly on the host with varying toolchains.

Critical Points for Discussion:

  1. Strategies to enhance build reproducibility without sacrificing useful debug information.
  2. The impact of different building methods (git, tarball, patch) on reproducibility.
  3. Potential guidelines or improvements to the building process to ensure consistent outcomes across different setups and use cases.

Feedback from team members, especially those experienced with similar challenges in projects like Heads, would be invaluable. Considering the current difficulties with binary consistency, this will also help decide the direction for upcoming releases and testing methodologies.

Please contribute your insights or any relevant experiences regarding building practices and reproducibility.

pietrushnic commented 3 months ago

@macpijan @krystian-hebel @SergiiDmytruk @miczyg1 @andyhhp @tlaurion @marmarek cc

pietrushnic commented 3 months ago

For now, I continue the dasharo-pq effort and upcoming Dasharo (coreboot+SeaBIOS) for PC Engines release using BUILD_TIMELESS=1.

SergiiDmytruk commented 3 months ago

line information in asserts

Absolute paths can be different and might be a problem, but line numbers seem harmless. The obvious fix is to replace absolute paths with relative ones when they appear. I think coreboot mostly uses relative ones, while vboot or some other components are build with absolute ones.

git commit hashes

If they can be specified explicitly during build, then one could have the same results when building from a patched tarball.

macpijan commented 3 months ago

Relevant gerrit reference:

andyhhp commented 3 months ago

The usual thing to do here is have the build system set SOURCE_DATE_EPOCH= to the commit date of the patchqueue changeset that you're building from. Most tools know what to do with SOURCE_DATE_EPOCH these days, and that gets you a date/cset pair which is stable when rebuilt later.

pietrushnic commented 3 months ago

Absolute paths can be different and might be a problem, but line numbers seem harmless. The obvious fix is to replace absolute paths with relative ones when they appear. I think coreboot mostly uses relative ones, while vboot or some other components are build with absolute ones.

If they can be specified explicitly during build, then one could have the same results when building from a patched tarball.

Yes, it sounds like a feature request, which could be implemented as part of some future work. Maybe the next release of Dasharo (coreboot+SeaBIOS) for PC Engines: https://github.com/Dasharo/dasharo-issues/issues/890

pietrushnic commented 3 months ago

The usual thing to do here is have the build system set SOURCE_DATE_EPOCH= to the commit date of the patchqueue changeset that you're building from. Most tools know what to do with SOURCE_DATE_EPOCH these days, and that gets you a date/cset pair which is stable when rebuilt later.

When built with BUILD_TIMLESS, it seems to have the same effect as SOURCE_DATE_EPOCH=0 for the coreboot build system, but maybe coreboot should use something used across the ecosystem instead of inventing its own mechanism. How does XenServer deal with the time? If you are using SOURCE_DATE_EPOCH, what value do you set it to?

pietrushnic commented 3 months ago

It breaks our tests because of the weird COREBOOT_VERSION:

% strings $FW_FILE|grep COREBOOT_
#define COREBOOT_VERSION "-TIMELESS--LESSTIME-"
#define COREBOOT_VERSION_TIMESTAMP 0
#define COREBOOT_ORIGIN_GIT_REVISION "Timeless"
#define COREBOOT_EXTRA_VERSION "-v24.05.00.01"
#define COREBOOT_MAJOR_VERSION 0
#define COREBOOT_MINOR_VERSION 0
#define COREBOOT_BUILD "Thu Jan 01 00:00:00 UTC 1970"
#define COREBOOT_BUILD_YEAR_BCD 0x70
#define COREBOOT_BUILD_MONTH_BCD 0x01
#define COREBOOT_BUILD_DAY_BCD 0x01
#define COREBOOT_BUILD_WEEKDAY_BCD 0x4
#define COREBOOT_BUILD_EPOCH "0"
#define COREBOOT_DMI_DATE "01/01/1970"
#define COREBOOT_COMPILE_TIME "00:00:00"
krystian-hebel commented 3 months ago

It breaks our tests because of the weird COREBOOT_VERSION

This is the same as for Talos II releases built by Heads. We had to build Heads which included coreboot.rom that was later discarded for this reason, and coreboot was built separately. It was possible because there was intermediate stage used as coreboot payload, and Heads lived on separate PNOR partition, it would be even more complicated on x86.

pietrushnic commented 3 months ago

Maybe something like this:

diff --git a/Makefile.mk b/Makefile.mk
index 65b827fe3eaf..52b7a92d104d 100644
--- a/Makefile.mk
+++ b/Makefile.mk
@@ -20,7 +20,9 @@ CONFIG_MEMLAYOUT_LD_FILE:=$(call strip_quotes, $(CONFIG_MEMLAYOUT_LD_FILE))
 # misleadingly named, this is the coreboot version
 ifeq ($(KERNELVERSION),)
 ifeq ($(BUILD_TIMELESS),1)
-KERNELVERSION := -TIMELESS--LESSTIME-
+KERNELVERSION := $(strip $(if $(GIT),\
+                $(shell git describe --abbrev=12 --dirty --always || git describe), \
+                -TIMELESS--LESSTIME-))
 else
 KERNELVERSION := $(strip $(if $(GIT),\
        $(shell git describe --abbrev=12 --dirty --always || git describe),\

how harmful would it be for us?

pietrushnic commented 3 months ago

As a consequence PC Engines Sign of Life changed and indicated 19700101