| Message ID | 20211018041148.1343731-1-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] Makefile: Add build time and compiler info string | expand |
On Mon, 2021-10-18 at 09:41 +0530, Anup Patel wrote: > From: Wei Fu <wefu@redhat.com> > > When we are doing opensbi development, we want to know the build time > and > compiler info for debug purpose. > To enable this message, please add "BUILD_INFO=y", like: > > ``` > make BUILD_INFO=y > ``` > > NOTE: `BUILD_INFO=y` will violate "reproducible builds". > So it's ONLY for development and debug purpose, and should NOT be used > in a product which follows "reproducible builds". > > Signed-off-by: Wei Fu <wefu@redhat.com> > Reviewed-by: Anup Patel <anup.patel@wdc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > Changes since v4: > - Updated build date format to +%Y-%m-%d %H:%M:%S %z > - Rename SOURCE_DATE_EPOCH to BUILD_DATE_EPOCH > --- > Makefile | 31 +++++++++++++++++++++++++++++++ > README.md | 23 +++++++++++++++++++++++ > lib/sbi/sbi_init.c | 8 ++++++++ > 3 files changed, 62 insertions(+) > > diff --git a/Makefile b/Makefile > index 16d9dca..9548afd 100644 > --- a/Makefile > +++ b/Makefile > @@ -150,6 +150,25 @@ endif > # Check whether the linker supports creating PIEs > OPENSBI_LD_PIE := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) > $(USE_LD_FLAG) -fPIE -nostdlib -Wl,-pie -x c /dev/null -o /dev/null > >/dev/null 2>&1 && echo y || echo n) > > +# Build Info: > +# OPENSBI_BUILD_TIME_STAMP -- the compilation time stamp > +# OPENSBI_BUILD_COMPILER_VERSION -- the compiler version info > +BUILD_INFO ?= n > +ifeq ($(BUILD_INFO),y) > +OPENSBI_BUILD_DATE_FMT = +%Y-%m-%d %H:%M:%S %z > +ifdef BUILD_DATE_EPOCH > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date -u -d > "@$(BUILD_DATE_EPOCH)" \ > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > + date -u -r "$(BUILD_DATE_EPOCH)" \ > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > + date -u "$(OPENSBI_BUILD_DATE_FMT)") > +else > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date > "$(OPENSBI_BUILD_DATE_FMT)") > +endif > +OPENSBI_BUILD_COMPILER_VERSION=$(shell $(CC) -v 2>&1 | grep ' version > ' | \ > + sed 's/[[:space:]]*$$//') > +endif > + > # Setup list of objects.mk files > ifdef PLATFORM > platform-object-mks=$(shell if [ -d $(platform_src_dir)/ ]; then find > $(platform_src_dir) -iname "objects.mk" | sort -r; fi) > @@ -247,6 +266,10 @@ GENFLAGS += -I$(include_dir) > ifneq ($(OPENSBI_VERSION_GIT),) > GENFLAGS += - > DOPENSBI_VERSION_GIT="\"$(OPENSBI_VERSION_GIT)\"" > endif > +ifeq ($(BUILD_INFO),y) > +GENFLAGS += - > DOPENSBI_BUILD_TIME_STAMP="\"$(OPENSBI_BUILD_TIME_STAMP)\"" > +GENFLAGS += - > DOPENSBI_BUILD_COMPILER_VERSION="\"$(OPENSBI_BUILD_COMPILER_VERSION)\"" > +endif > GENFLAGS += $(libsbiutils-genflags-y) > GENFLAGS += $(platform-genflags-y) > GENFLAGS += $(firmware-genflags-y) > @@ -398,6 +421,11 @@ $(build_dir)/%.dep: $(src_dir)/%.c > $(build_dir)/%.o: $(src_dir)/%.c > $(call compile_cc,$@,$<) > > +ifeq ($(BUILD_INFO),y) > +$(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE > + $(call compile_cc,$@,$<) > +endif > + > $(build_dir)/%.dep: $(src_dir)/%.S > $(call compile_as_dep,$@,$<) > > @@ -551,3 +579,6 @@ ifeq > ($(install_root_dir),$(install_root_dir_default)/usr) > $(if $(V), @echo " RM $(install_root_dir_default)") > $(CMD_PREFIX)rm -rf $(install_root_dir_default) > endif > + > +.PHONY: FORCE > +FORCE: > diff --git a/README.md b/README.md > index 9fdf097..c498af0 100644 > --- a/README.md > +++ b/README.md > @@ -254,6 +254,28 @@ to produce broken binaries with missing > relocations; it is therefore currently > recommended that this combination be avoided or *FW_PIC=n* be used to > disable > building OpenSBI as a position-independent binary. > > +Building with timestamp and compiler info > +----------------------------------------- > + > +When doing development, we may want to know the build time and > compiler info > +for debug purpose. OpenSBI can also be built with timestamp and > compiler info. > +To build with those info and print it out at boot time, we can just > simply add > +`BUILD_INFO=y`, like: > +``` > +make BUILD_INFO=y > +``` > + > +But if you have used `BUILD_INFO=y`, and want to switch back to > `BUILD_INFO=n`, > +you must do > +``` > +make clean > +``` > +before the next build. > + > +NOTE: Using `BUILD_INFO=y` will violate [reproducible builds]. This > definition > +is ONLY for development and debug purpose, and should NOT be used in a > product > +which follows "reproducible builds". > + > Contributing to OpenSBI > ----------------------- > > @@ -336,3 +358,4 @@ make I=<install_directory> install_docs > [Doxygen manual]: http://www.doxygen.nl/manual/index.html > [Kendryte standalone SDK]: > https://github.com/kendryte/kendryte-standalone-sdk > [third party notices]: ThirdPartyNotices.md > +[reproducible builds]: https://reproducible-builds.org > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c > index 843659e..b1c7cf0 100644 > --- a/lib/sbi/sbi_init.c > +++ b/lib/sbi/sbi_init.c > @@ -48,6 +48,14 @@ static void sbi_boot_print_banner(struct sbi_scratch > *scratch) > OPENSBI_VERSION_MINOR); > #endif > > +#ifdef OPENSBI_BUILD_TIME_STAMP > + sbi_printf("Build time: %s\n", OPENSBI_BUILD_TIME_STAMP); > +#endif > + > +#ifdef OPENSBI_BUILD_COMPILER_VERSION > + sbi_printf("Build compiler: %s\n", > OPENSBI_BUILD_COMPILER_VERSION); > +#endif > + > sbi_printf(BANNER); > } >
Hi Anup, On Mon, Oct 18, 2021 at 12:13 PM Anup Patel <anup.patel@wdc.com> wrote: > > From: Wei Fu <wefu@redhat.com> > > When we are doing opensbi development, we want to know the build time and > compiler info for debug purpose. > To enable this message, please add "BUILD_INFO=y", like: > > ``` > make BUILD_INFO=y > ``` > > NOTE: `BUILD_INFO=y` will violate "reproducible builds". > So it's ONLY for development and debug purpose, and should NOT be used > in a product which follows "reproducible builds". As I pointed out in v4, with BUILD_INFO=y we can still achieve reproducible builds with BUILD_DATE_EPOCH, so this statement is not entirely true. The question is whether we should just drop introducing BUILD_INFO and turn this on for all builds. If reproducible builds is required, define BUILD_DATE_EPOCH. > > Signed-off-by: Wei Fu <wefu@redhat.com> > Reviewed-by: Anup Patel <anup.patel@wdc.com> > --- > Changes since v4: > - Updated build date format to +%Y-%m-%d %H:%M:%S %z > - Rename SOURCE_DATE_EPOCH to BUILD_DATE_EPOCH > --- > Makefile | 31 +++++++++++++++++++++++++++++++ > README.md | 23 +++++++++++++++++++++++ > lib/sbi/sbi_init.c | 8 ++++++++ > 3 files changed, 62 insertions(+) > > diff --git a/Makefile b/Makefile > index 16d9dca..9548afd 100644 > --- a/Makefile > +++ b/Makefile > @@ -150,6 +150,25 @@ endif > # Check whether the linker supports creating PIEs > OPENSBI_LD_PIE := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) $(USE_LD_FLAG) -fPIE -nostdlib -Wl,-pie -x c /dev/null -o /dev/null >/dev/null 2>&1 && echo y || echo n) > > +# Build Info: > +# OPENSBI_BUILD_TIME_STAMP -- the compilation time stamp > +# OPENSBI_BUILD_COMPILER_VERSION -- the compiler version info > +BUILD_INFO ?= n > +ifeq ($(BUILD_INFO),y) > +OPENSBI_BUILD_DATE_FMT = +%Y-%m-%d %H:%M:%S %z > +ifdef BUILD_DATE_EPOCH > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date -u -d "@$(BUILD_DATE_EPOCH)" \ > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > + date -u -r "$(BUILD_DATE_EPOCH)" \ > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > + date -u "$(OPENSBI_BUILD_DATE_FMT)") > +else > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date "$(OPENSBI_BUILD_DATE_FMT)") > +endif > +OPENSBI_BUILD_COMPILER_VERSION=$(shell $(CC) -v 2>&1 | grep ' version ' | \ > + sed 's/[[:space:]]*$$//') > +endif > + > # Setup list of objects.mk files > ifdef PLATFORM > platform-object-mks=$(shell if [ -d $(platform_src_dir)/ ]; then find $(platform_src_dir) -iname "objects.mk" | sort -r; fi) > @@ -247,6 +266,10 @@ GENFLAGS += -I$(include_dir) > ifneq ($(OPENSBI_VERSION_GIT),) > GENFLAGS += -DOPENSBI_VERSION_GIT="\"$(OPENSBI_VERSION_GIT)\"" > endif > +ifeq ($(BUILD_INFO),y) > +GENFLAGS += -DOPENSBI_BUILD_TIME_STAMP="\"$(OPENSBI_BUILD_TIME_STAMP)\"" > +GENFLAGS += -DOPENSBI_BUILD_COMPILER_VERSION="\"$(OPENSBI_BUILD_COMPILER_VERSION)\"" > +endif > GENFLAGS += $(libsbiutils-genflags-y) > GENFLAGS += $(platform-genflags-y) > GENFLAGS += $(firmware-genflags-y) > @@ -398,6 +421,11 @@ $(build_dir)/%.dep: $(src_dir)/%.c > $(build_dir)/%.o: $(src_dir)/%.c > $(call compile_cc,$@,$<) > > +ifeq ($(BUILD_INFO),y) > +$(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE > + $(call compile_cc,$@,$<) > +endif > + > $(build_dir)/%.dep: $(src_dir)/%.S > $(call compile_as_dep,$@,$<) > > @@ -551,3 +579,6 @@ ifeq ($(install_root_dir),$(install_root_dir_default)/usr) > $(if $(V), @echo " RM $(install_root_dir_default)") > $(CMD_PREFIX)rm -rf $(install_root_dir_default) > endif > + > +.PHONY: FORCE > +FORCE: > diff --git a/README.md b/README.md > index 9fdf097..c498af0 100644 > --- a/README.md > +++ b/README.md > @@ -254,6 +254,28 @@ to produce broken binaries with missing relocations; it is therefore currently > recommended that this combination be avoided or *FW_PIC=n* be used to disable > building OpenSBI as a position-independent binary. > > +Building with timestamp and compiler info > +----------------------------------------- > + > +When doing development, we may want to know the build time and compiler info > +for debug purpose. OpenSBI can also be built with timestamp and compiler info. > +To build with those info and print it out at boot time, we can just simply add > +`BUILD_INFO=y`, like: > +``` > +make BUILD_INFO=y > +``` > + > +But if you have used `BUILD_INFO=y`, and want to switch back to `BUILD_INFO=n`, > +you must do > +``` > +make clean > +``` > +before the next build. > + > +NOTE: Using `BUILD_INFO=y` will violate [reproducible builds]. This definition > +is ONLY for development and debug purpose, and should NOT be used in a product > +which follows "reproducible builds". > + > Contributing to OpenSBI > ----------------------- > > @@ -336,3 +358,4 @@ make I=<install_directory> install_docs > [Doxygen manual]: http://www.doxygen.nl/manual/index.html > [Kendryte standalone SDK]: https://github.com/kendryte/kendryte-standalone-sdk > [third party notices]: ThirdPartyNotices.md > +[reproducible builds]: https://reproducible-builds.org > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c > index 843659e..b1c7cf0 100644 > --- a/lib/sbi/sbi_init.c > +++ b/lib/sbi/sbi_init.c > @@ -48,6 +48,14 @@ static void sbi_boot_print_banner(struct sbi_scratch *scratch) > OPENSBI_VERSION_MINOR); > #endif > > +#ifdef OPENSBI_BUILD_TIME_STAMP > + sbi_printf("Build time: %s\n", OPENSBI_BUILD_TIME_STAMP); > +#endif > + > +#ifdef OPENSBI_BUILD_COMPILER_VERSION > + sbi_printf("Build compiler: %s\n", OPENSBI_BUILD_COMPILER_VERSION); > +#endif > + > sbi_printf(BANNER); > } > Regards, Bin
On Mon, Oct 18, 2021 at 11:27 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Oct 18, 2021 at 12:13 PM Anup Patel <anup.patel@wdc.com> wrote: > > > > From: Wei Fu <wefu@redhat.com> > > > > When we are doing opensbi development, we want to know the build time and > > compiler info for debug purpose. > > To enable this message, please add "BUILD_INFO=y", like: > > > > ``` > > make BUILD_INFO=y > > ``` > > > > NOTE: `BUILD_INFO=y` will violate "reproducible builds". > > So it's ONLY for development and debug purpose, and should NOT be used > > in a product which follows "reproducible builds". > > As I pointed out in v4, with BUILD_INFO=y we can still achieve > reproducible builds with BUILD_DATE_EPOCH, so this statement is not > entirely true. Yes, but BUILD_DATE_EPOCH is optional and generally people will not specify BUILD_DATE_EPOCH unless they want a reproducible build. I will certainly update the "NOTE:" in the commit description to clearly state when reproducible builds are violated. > > The question is whether we should just drop introducing BUILD_INFO and > turn this on for all builds. If reproducible builds is required, > define BUILD_DATE_EPOCH. I think we should keep both BUILD_INFO and BUILD_DATA_EPOCH because of the reason I mentioned above. Regards, Anup > > > > > Signed-off-by: Wei Fu <wefu@redhat.com> > > Reviewed-by: Anup Patel <anup.patel@wdc.com> > > --- > > Changes since v4: > > - Updated build date format to +%Y-%m-%d %H:%M:%S %z > > - Rename SOURCE_DATE_EPOCH to BUILD_DATE_EPOCH > > --- > > Makefile | 31 +++++++++++++++++++++++++++++++ > > README.md | 23 +++++++++++++++++++++++ > > lib/sbi/sbi_init.c | 8 ++++++++ > > 3 files changed, 62 insertions(+) > > > > diff --git a/Makefile b/Makefile > > index 16d9dca..9548afd 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -150,6 +150,25 @@ endif > > # Check whether the linker supports creating PIEs > > OPENSBI_LD_PIE := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) $(USE_LD_FLAG) -fPIE -nostdlib -Wl,-pie -x c /dev/null -o /dev/null >/dev/null 2>&1 && echo y || echo n) > > > > +# Build Info: > > +# OPENSBI_BUILD_TIME_STAMP -- the compilation time stamp > > +# OPENSBI_BUILD_COMPILER_VERSION -- the compiler version info > > +BUILD_INFO ?= n > > +ifeq ($(BUILD_INFO),y) > > +OPENSBI_BUILD_DATE_FMT = +%Y-%m-%d %H:%M:%S %z > > +ifdef BUILD_DATE_EPOCH > > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date -u -d "@$(BUILD_DATE_EPOCH)" \ > > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > > + date -u -r "$(BUILD_DATE_EPOCH)" \ > > + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ > > + date -u "$(OPENSBI_BUILD_DATE_FMT)") > > +else > > + OPENSBI_BUILD_TIME_STAMP ?= $(shell date "$(OPENSBI_BUILD_DATE_FMT)") > > +endif > > +OPENSBI_BUILD_COMPILER_VERSION=$(shell $(CC) -v 2>&1 | grep ' version ' | \ > > + sed 's/[[:space:]]*$$//') > > +endif > > + > > # Setup list of objects.mk files > > ifdef PLATFORM > > platform-object-mks=$(shell if [ -d $(platform_src_dir)/ ]; then find $(platform_src_dir) -iname "objects.mk" | sort -r; fi) > > @@ -247,6 +266,10 @@ GENFLAGS += -I$(include_dir) > > ifneq ($(OPENSBI_VERSION_GIT),) > > GENFLAGS += -DOPENSBI_VERSION_GIT="\"$(OPENSBI_VERSION_GIT)\"" > > endif > > +ifeq ($(BUILD_INFO),y) > > +GENFLAGS += -DOPENSBI_BUILD_TIME_STAMP="\"$(OPENSBI_BUILD_TIME_STAMP)\"" > > +GENFLAGS += -DOPENSBI_BUILD_COMPILER_VERSION="\"$(OPENSBI_BUILD_COMPILER_VERSION)\"" > > +endif > > GENFLAGS += $(libsbiutils-genflags-y) > > GENFLAGS += $(platform-genflags-y) > > GENFLAGS += $(firmware-genflags-y) > > @@ -398,6 +421,11 @@ $(build_dir)/%.dep: $(src_dir)/%.c > > $(build_dir)/%.o: $(src_dir)/%.c > > $(call compile_cc,$@,$<) > > > > +ifeq ($(BUILD_INFO),y) > > +$(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE > > + $(call compile_cc,$@,$<) > > +endif > > + > > $(build_dir)/%.dep: $(src_dir)/%.S > > $(call compile_as_dep,$@,$<) > > > > @@ -551,3 +579,6 @@ ifeq ($(install_root_dir),$(install_root_dir_default)/usr) > > $(if $(V), @echo " RM $(install_root_dir_default)") > > $(CMD_PREFIX)rm -rf $(install_root_dir_default) > > endif > > + > > +.PHONY: FORCE > > +FORCE: > > diff --git a/README.md b/README.md > > index 9fdf097..c498af0 100644 > > --- a/README.md > > +++ b/README.md > > @@ -254,6 +254,28 @@ to produce broken binaries with missing relocations; it is therefore currently > > recommended that this combination be avoided or *FW_PIC=n* be used to disable > > building OpenSBI as a position-independent binary. > > > > +Building with timestamp and compiler info > > +----------------------------------------- > > + > > +When doing development, we may want to know the build time and compiler info > > +for debug purpose. OpenSBI can also be built with timestamp and compiler info. > > +To build with those info and print it out at boot time, we can just simply add > > +`BUILD_INFO=y`, like: > > +``` > > +make BUILD_INFO=y > > +``` > > + > > +But if you have used `BUILD_INFO=y`, and want to switch back to `BUILD_INFO=n`, > > +you must do > > +``` > > +make clean > > +``` > > +before the next build. > > + > > +NOTE: Using `BUILD_INFO=y` will violate [reproducible builds]. This definition > > +is ONLY for development and debug purpose, and should NOT be used in a product > > +which follows "reproducible builds". > > + > > Contributing to OpenSBI > > ----------------------- > > > > @@ -336,3 +358,4 @@ make I=<install_directory> install_docs > > [Doxygen manual]: http://www.doxygen.nl/manual/index.html > > [Kendryte standalone SDK]: https://github.com/kendryte/kendryte-standalone-sdk > > [third party notices]: ThirdPartyNotices.md > > +[reproducible builds]: https://reproducible-builds.org > > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c > > index 843659e..b1c7cf0 100644 > > --- a/lib/sbi/sbi_init.c > > +++ b/lib/sbi/sbi_init.c > > @@ -48,6 +48,14 @@ static void sbi_boot_print_banner(struct sbi_scratch *scratch) > > OPENSBI_VERSION_MINOR); > > #endif > > > > +#ifdef OPENSBI_BUILD_TIME_STAMP > > + sbi_printf("Build time: %s\n", OPENSBI_BUILD_TIME_STAMP); > > +#endif > > + > > +#ifdef OPENSBI_BUILD_COMPILER_VERSION > > + sbi_printf("Build compiler: %s\n", OPENSBI_BUILD_COMPILER_VERSION); > > +#endif > > + > > sbi_printf(BANNER); > > } > > > > Regards, > Bin
>>>>> "Bin" == Bin Meng <bmeng.cn@gmail.com> writes: > Hi Anup, > On Mon, Oct 18, 2021 at 12:13 PM Anup Patel <anup.patel@wdc.com> wrote: >> >> From: Wei Fu <wefu@redhat.com> >> >> When we are doing opensbi development, we want to know the build time and >> compiler info for debug purpose. >> To enable this message, please add "BUILD_INFO=y", like: >> >> ``` >> make BUILD_INFO=y >> ``` >> >> NOTE: `BUILD_INFO=y` will violate "reproducible builds". >> So it's ONLY for development and debug purpose, and should NOT be used >> in a product which follows "reproducible builds". > As I pointed out in v4, with BUILD_INFO=y we can still achieve > reproducible builds with BUILD_DATE_EPOCH, so this statement is not > entirely true. > The question is whether we should just drop introducing BUILD_INFO and > turn this on for all builds. If reproducible builds is required, > define BUILD_DATE_EPOCH. Agreed, though BUILD_DATE_EPOCH seems to be specific to opensbi. Why was this changed from SOURCE_DATE_EPOCH? In Buildroot at least, we only set SOURCE_DATE_EPOCH when we want reproducible builds and the spec has explicit examples where it is used for the build timestamp: https://reproducible-builds.org/docs/source-date-epoch/
On Mon, Oct 18, 2021 at 2:17 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Oct 18, 2021 at 11:27 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Oct 18, 2021 at 12:13 PM Anup Patel <anup.patel@wdc.com> wrote: > > > > > > From: Wei Fu <wefu@redhat.com> > > > > > > When we are doing opensbi development, we want to know the build time and > > > compiler info for debug purpose. > > > To enable this message, please add "BUILD_INFO=y", like: > > > > > > ``` > > > make BUILD_INFO=y > > > ``` > > > > > > NOTE: `BUILD_INFO=y` will violate "reproducible builds". > > > So it's ONLY for development and debug purpose, and should NOT be used > > > in a product which follows "reproducible builds". > > > > As I pointed out in v4, with BUILD_INFO=y we can still achieve > > reproducible builds with BUILD_DATE_EPOCH, so this statement is not > > entirely true. > > Yes, but BUILD_DATE_EPOCH is optional and generally people will not > specify BUILD_DATE_EPOCH unless they want a reproducible build. > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we still keep ifdef BUILD_DATE_EPOCH OPENSBI_BUILD_TIME_STAMP ?= $(shell date -u -d "@$(BUILD_DATE_EPOCH)" \ "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ date -u -r "$(BUILD_DATE_EPOCH)" \ "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ date -u "$(OPENSBI_BUILD_DATE_FMT)") else > I will certainly update the "NOTE:" in the commit description to clearly > state when reproducible builds are violated. > > > > > The question is whether we should just drop introducing BUILD_INFO and > > turn this on for all builds. If reproducible builds is required, > > define BUILD_DATE_EPOCH. > > I think we should keep both BUILD_INFO and BUILD_DATA_EPOCH > because of the reason I mentioned above. > Regards, Bin
On Okt 18 2021, Bin Meng wrote: > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > still keep BUILD_DATE_EPOCH is also non-standard. The standard variable is SOURCE_DATE_EPOCH. Andreas.
On Mon, Oct 18, 2021 at 3:44 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Okt 18 2021, Bin Meng wrote: > > > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > > still keep > > BUILD_DATE_EPOCH is also non-standard. The standard variable is > SOURCE_DATE_EPOCH. > Yep, it's SOURCE_DATE_EPOCH in v4, not sure why Anup changed the name in v5. SOURCE_DATE_EPOCH is well documented and is the way for distros to generate "reproducible builds". I would vote to completely drop the "BUILD_INFO=y" as it is not necessary. Regards, Bin
On Mon, Oct 18, 2021 at 1:42 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Mon, Oct 18, 2021 at 3:44 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > On Okt 18 2021, Bin Meng wrote: > > > > > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > > > still keep > > > > BUILD_DATE_EPOCH is also non-standard. The standard variable is > > SOURCE_DATE_EPOCH. > > > > Yep, it's SOURCE_DATE_EPOCH in v4, not sure why Anup changed the name in v5. Yes, I changed it to BUILD_DATE_EPOCH to imply that it is related to BUILD_INFO. I was not aware that SOURCE_DATE_EPOCH is a standard name. I will rename it to SOURCE_DATE_EPOCH in v6. > > SOURCE_DATE_EPOCH is well documented and is the way for distros to > generate "reproducible builds". > > I would vote to completely drop the "BUILD_INFO=y" as it is not necessary. I don't agree with this. The BUILD_INFO helps us totally disable build information whereas SOURCE_DATE_EPOCH helps us override the build date/time when BUILD_INFO=y. Regards, Anup
On Mon, Oct 18, 2021 at 6:08 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Oct 18, 2021 at 1:42 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Mon, Oct 18, 2021 at 3:44 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > On Okt 18 2021, Bin Meng wrote: > > > > > > > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > > > > still keep > > > > > > BUILD_DATE_EPOCH is also non-standard. The standard variable is > > > SOURCE_DATE_EPOCH. > > > > > > > Yep, it's SOURCE_DATE_EPOCH in v4, not sure why Anup changed the name in v5. > > Yes, I changed it to BUILD_DATE_EPOCH to imply that it is related to > BUILD_INFO. I was not aware that SOURCE_DATE_EPOCH is a > standard name. I will rename it to SOURCE_DATE_EPOCH in v6. > > > > > SOURCE_DATE_EPOCH is well documented and is the way for distros to > > generate "reproducible builds". > > > > I would vote to completely drop the "BUILD_INFO=y" as it is not necessary. > > I don't agree with this. The BUILD_INFO helps us totally disable build > information whereas SOURCE_DATE_EPOCH helps us override the > build date/time when BUILD_INFO=y. But what benefits us to provide the capability to conditionally disable the build information? I think that is useful and can be turned on unconditionally, and we can still generate reproducible builds with SOURCE_DATE_EPOCH. Regards, Bin
On Mon, Oct 18, 2021 at 3:41 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Mon, Oct 18, 2021 at 6:08 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Oct 18, 2021 at 1:42 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > On Mon, Oct 18, 2021 at 3:44 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > > > On Okt 18 2021, Bin Meng wrote: > > > > > > > > > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > > > > > still keep > > > > > > > > BUILD_DATE_EPOCH is also non-standard. The standard variable is > > > > SOURCE_DATE_EPOCH. > > > > > > > > > > Yep, it's SOURCE_DATE_EPOCH in v4, not sure why Anup changed the name in v5. > > > > Yes, I changed it to BUILD_DATE_EPOCH to imply that it is related to > > BUILD_INFO. I was not aware that SOURCE_DATE_EPOCH is a > > standard name. I will rename it to SOURCE_DATE_EPOCH in v6. > > > > > > > > SOURCE_DATE_EPOCH is well documented and is the way for distros to > > > generate "reproducible builds". > > > > > > I would vote to completely drop the "BUILD_INFO=y" as it is not necessary. > > > > I don't agree with this. The BUILD_INFO helps us totally disable build > > information whereas SOURCE_DATE_EPOCH helps us override the > > build date/time when BUILD_INFO=y. > > But what benefits us to provide the capability to conditionally > disable the build information? I think that is useful and can be > turned on unconditionally, and we can still generate reproducible > builds with SOURCE_DATE_EPOCH. Users will have to figure-out/remember a fixed build date when using SOURCE_DATA_EPOCH for reproducible builds whereas users don't need to do anything when BUILD_INFO is not defined. BUILD_INFO being not defined is easy/convenient for users creating reproducible builds by default. This is also reflected by the "BUILD_INFO ?= n" line. Regards, Anup
On Okt 18 2021, Anup Patel wrote: > Users will have to figure-out/remember a fixed build date when > using SOURCE_DATA_EPOCH for reproducible builds whereas > users don't need to do anything when BUILD_INFO is not defined. Users who care about reproducible builds already know how to set SOURCE_DATA_EPOCH. Andreas.
On Mon, Oct 18, 2021 at 6:19 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Oct 18, 2021 at 3:41 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Mon, Oct 18, 2021 at 6:08 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Mon, Oct 18, 2021 at 1:42 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > On Mon, Oct 18, 2021 at 3:44 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > > > > > On Okt 18 2021, Bin Meng wrote: > > > > > > > > > > > If this BUILD_DATE_EPOCH is not needed at all, I am not sure why do we > > > > > > still keep > > > > > > > > > > BUILD_DATE_EPOCH is also non-standard. The standard variable is > > > > > SOURCE_DATE_EPOCH. > > > > > > > > > > > > > Yep, it's SOURCE_DATE_EPOCH in v4, not sure why Anup changed the name in v5. > > > > > > Yes, I changed it to BUILD_DATE_EPOCH to imply that it is related to > > > BUILD_INFO. I was not aware that SOURCE_DATE_EPOCH is a > > > standard name. I will rename it to SOURCE_DATE_EPOCH in v6. > > > > > > > > > > > SOURCE_DATE_EPOCH is well documented and is the way for distros to > > > > generate "reproducible builds". > > > > > > > > I would vote to completely drop the "BUILD_INFO=y" as it is not necessary. > > > > > > I don't agree with this. The BUILD_INFO helps us totally disable build > > > information whereas SOURCE_DATE_EPOCH helps us override the > > > build date/time when BUILD_INFO=y. > > > > But what benefits us to provide the capability to conditionally > > disable the build information? I think that is useful and can be > > turned on unconditionally, and we can still generate reproducible > > builds with SOURCE_DATE_EPOCH. > > Users will have to figure-out/remember a fixed build date when > using SOURCE_DATA_EPOCH for reproducible builds whereas > users don't need to do anything when BUILD_INFO is not defined. > This is not 100% true as with current OpenSBI we never claim its build is fully reproducible. For example, one can easily add __DATE__ to the source codes which will alter unless we provide SOURCE_DATA_EPOCH. > BUILD_INFO being not defined is easy/convenient for users > creating reproducible builds by default. This is also reflected by > the "BUILD_INFO ?= n" line. Regards, Bin
On Mon, Oct 18, 2021 at 3:58 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Okt 18 2021, Anup Patel wrote: > > > Users will have to figure-out/remember a fixed build date when > > using SOURCE_DATA_EPOCH for reproducible builds whereas > > users don't need to do anything when BUILD_INFO is not defined. > > Users who care about reproducible builds already know how to set > SOURCE_DATA_EPOCH. > @Alistair Francis Are you okay removing BUILD_INFO and having it always enabled ? Regards, Anup
>>>>> "Bin" == Bin Meng <bmeng.cn@gmail.com> writes: Hi, >> Users will have to figure-out/remember a fixed build date when >> using SOURCE_DATA_EPOCH for reproducible builds whereas >> users don't need to do anything when BUILD_INFO is not defined. > This is not 100% true as with current OpenSBI we never claim its build > is fully reproducible. For example, one can easily add __DATE__ to the > source codes which will alter unless we provide SOURCE_DATA_EPOCH. Notice that GCC >= 7 does in fact look for SOURCE_DATE_EPOCH in the environment and change the behaviour of the __DATE__ / __TIME__ macros: https://gcc.gnu.org/onlinedocs/cpp/Environment-Variables.html
On Mon, 2021-10-18 at 16:06 +0530, Anup Patel wrote: > On Mon, Oct 18, 2021 at 3:58 PM Andreas Schwab <schwab@linux-m68k.org> > wrote: > > > > On Okt 18 2021, Anup Patel wrote: > > > > > Users will have to figure-out/remember a fixed build date when > > > using SOURCE_DATA_EPOCH for reproducible builds whereas > > > users don't need to do anything when BUILD_INFO is not defined. > > > > Users who care about reproducible builds already know how to set > > SOURCE_DATA_EPOCH. > > > > @Alistair Francis Are you okay removing BUILD_INFO and having it > always enabled ? I think it should be disabled by default. We have not had a timestamp for awhile and it hasn't impacted people too much. The reproducible build documentation specifically says that timestamps are best avoided (https://reproducible-builds.org/docs/timestamps/). I think they sum it up well that it doesn't add much value, except for a few specific circumstances. So I don't see a good justification for enabling it by default, even with SOURCE_DATE_EPOCH (good catch on the name chage, I missed that). Alistair > > Regards, > Anup
On Tue, Oct 19, 2021 at 8:23 AM Alistair Francis <Alistair.Francis@wdc.com> wrote: > > On Mon, 2021-10-18 at 16:06 +0530, Anup Patel wrote: > > On Mon, Oct 18, 2021 at 3:58 PM Andreas Schwab <schwab@linux-m68k.org> > > wrote: > > > > > > On Okt 18 2021, Anup Patel wrote: > > > > > > > Users will have to figure-out/remember a fixed build date when > > > > using SOURCE_DATA_EPOCH for reproducible builds whereas > > > > users don't need to do anything when BUILD_INFO is not defined. > > > > > > Users who care about reproducible builds already know how to set > > > SOURCE_DATA_EPOCH. > > > > > > > @Alistair Francis Are you okay removing BUILD_INFO and having it > > always enabled ? > > I think it should be disabled by default. > > We have not had a timestamp for awhile and it hasn't impacted people > too much. > > The reproducible build documentation specifically says that timestamps > are best avoided (https://reproducible-builds.org/docs/timestamps/). I > think they sum it up well that it doesn't add much value, except for a > few specific circumstances. > > So I don't see a good justification for enabling it by default, even > with SOURCE_DATE_EPOCH (good catch on the name chage, I missed that). > If that, please reword the following part of the commit message as well as updating the docs regarding "reproducible builds". Having BUILD_INFO=n does not mean we are reproducible builds by default. NOTE: `BUILD_INFO=y` will violate "reproducible builds". So it's ONLY for development and debug purpose, and should NOT be used in a product which follows "reproducible builds". Regards, Bin
diff --git a/Makefile b/Makefile index 16d9dca..9548afd 100644 --- a/Makefile +++ b/Makefile @@ -150,6 +150,25 @@ endif # Check whether the linker supports creating PIEs OPENSBI_LD_PIE := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) $(USE_LD_FLAG) -fPIE -nostdlib -Wl,-pie -x c /dev/null -o /dev/null >/dev/null 2>&1 && echo y || echo n) +# Build Info: +# OPENSBI_BUILD_TIME_STAMP -- the compilation time stamp +# OPENSBI_BUILD_COMPILER_VERSION -- the compiler version info +BUILD_INFO ?= n +ifeq ($(BUILD_INFO),y) +OPENSBI_BUILD_DATE_FMT = +%Y-%m-%d %H:%M:%S %z +ifdef BUILD_DATE_EPOCH + OPENSBI_BUILD_TIME_STAMP ?= $(shell date -u -d "@$(BUILD_DATE_EPOCH)" \ + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ + date -u -r "$(BUILD_DATE_EPOCH)" \ + "$(OPENSBI_BUILD_DATE_FMT)" 2>/dev/null || \ + date -u "$(OPENSBI_BUILD_DATE_FMT)") +else + OPENSBI_BUILD_TIME_STAMP ?= $(shell date "$(OPENSBI_BUILD_DATE_FMT)") +endif +OPENSBI_BUILD_COMPILER_VERSION=$(shell $(CC) -v 2>&1 | grep ' version ' | \ + sed 's/[[:space:]]*$$//') +endif + # Setup list of objects.mk files ifdef PLATFORM platform-object-mks=$(shell if [ -d $(platform_src_dir)/ ]; then find $(platform_src_dir) -iname "objects.mk" | sort -r; fi) @@ -247,6 +266,10 @@ GENFLAGS += -I$(include_dir) ifneq ($(OPENSBI_VERSION_GIT),) GENFLAGS += -DOPENSBI_VERSION_GIT="\"$(OPENSBI_VERSION_GIT)\"" endif +ifeq ($(BUILD_INFO),y) +GENFLAGS += -DOPENSBI_BUILD_TIME_STAMP="\"$(OPENSBI_BUILD_TIME_STAMP)\"" +GENFLAGS += -DOPENSBI_BUILD_COMPILER_VERSION="\"$(OPENSBI_BUILD_COMPILER_VERSION)\"" +endif GENFLAGS += $(libsbiutils-genflags-y) GENFLAGS += $(platform-genflags-y) GENFLAGS += $(firmware-genflags-y) @@ -398,6 +421,11 @@ $(build_dir)/%.dep: $(src_dir)/%.c $(build_dir)/%.o: $(src_dir)/%.c $(call compile_cc,$@,$<) +ifeq ($(BUILD_INFO),y) +$(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE + $(call compile_cc,$@,$<) +endif + $(build_dir)/%.dep: $(src_dir)/%.S $(call compile_as_dep,$@,$<) @@ -551,3 +579,6 @@ ifeq ($(install_root_dir),$(install_root_dir_default)/usr) $(if $(V), @echo " RM $(install_root_dir_default)") $(CMD_PREFIX)rm -rf $(install_root_dir_default) endif + +.PHONY: FORCE +FORCE: diff --git a/README.md b/README.md index 9fdf097..c498af0 100644 --- a/README.md +++ b/README.md @@ -254,6 +254,28 @@ to produce broken binaries with missing relocations; it is therefore currently recommended that this combination be avoided or *FW_PIC=n* be used to disable building OpenSBI as a position-independent binary. +Building with timestamp and compiler info +----------------------------------------- + +When doing development, we may want to know the build time and compiler info +for debug purpose. OpenSBI can also be built with timestamp and compiler info. +To build with those info and print it out at boot time, we can just simply add +`BUILD_INFO=y`, like: +``` +make BUILD_INFO=y +``` + +But if you have used `BUILD_INFO=y`, and want to switch back to `BUILD_INFO=n`, +you must do +``` +make clean +``` +before the next build. + +NOTE: Using `BUILD_INFO=y` will violate [reproducible builds]. This definition +is ONLY for development and debug purpose, and should NOT be used in a product +which follows "reproducible builds". + Contributing to OpenSBI ----------------------- @@ -336,3 +358,4 @@ make I=<install_directory> install_docs [Doxygen manual]: http://www.doxygen.nl/manual/index.html [Kendryte standalone SDK]: https://github.com/kendryte/kendryte-standalone-sdk [third party notices]: ThirdPartyNotices.md +[reproducible builds]: https://reproducible-builds.org diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c index 843659e..b1c7cf0 100644 --- a/lib/sbi/sbi_init.c +++ b/lib/sbi/sbi_init.c @@ -48,6 +48,14 @@ static void sbi_boot_print_banner(struct sbi_scratch *scratch) OPENSBI_VERSION_MINOR); #endif +#ifdef OPENSBI_BUILD_TIME_STAMP + sbi_printf("Build time: %s\n", OPENSBI_BUILD_TIME_STAMP); +#endif + +#ifdef OPENSBI_BUILD_COMPILER_VERSION + sbi_printf("Build compiler: %s\n", OPENSBI_BUILD_COMPILER_VERSION); +#endif + sbi_printf(BANNER); }