[PATCH v2] Makefile: Pass relative paths to the compiler.

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Vagrant Cascadian]

Upstream commit 12753d22563f7d2d01f2c6644c7b66b06eb5c90f introduced
uses of __FILE__ which may result in the build path getting embedded
into the resulting binary.

https://reproducible-builds.org/docs/build-path/

Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
Cc: Xiang W <wxjstz@126.com>
Cc: Anup Patel <anup@brainfault.org>
---
Changes since v1:

* Pass relative paths to the compiler instead of using
  -ffile-prefix-map to strip out the full paths, as suggested by Xiang
  W.

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8623c1c..d017534 100644
--- a/Makefile
+++ b/Makefile
@@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
 	       -MM $(2) >> $(1) || rm -f $(1)
 compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
 	     echo " CC        $(subst $(build_dir)/,,$(1))"; \
-	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2) -o $(1)
+	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(subst $(src_dir)/,,$(2)) -o $(1)
 compile_as_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
 	     echo " AS-DEP    $(subst $(build_dir)/,,$(1))"; \
 	     printf %s `dirname $(1)`/ > $(1) && \
-- 
2.30.2

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Dong Du]

On Nov 29, 2021, at 9:55 AM, Vagrant Cascadian vagrant at reproducible-builds.org wrote:

> Upstream commit 12753d22563f7d2d01f2c6644c7b66b06eb5c90f introduced
> uses of __FILE__ which may result in the build path getting embedded
> into the resulting binary.
> 
> https://reproducible-builds.org/docs/build-path/
> 
> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Cc: Xiang W <wxjstz@126.com>
> Cc: Anup Patel <anup@brainfault.org>
> ---
> Changes since v1:
> 
> * Pass relative paths to the compiler instead of using
>  -ffile-prefix-map to strip out the full paths, as suggested by Xiang
>  W.
> 
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8623c1c..d017534 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
> 	       -MM $(2) >> $(1) || rm -f $(1)
> compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
> 	     echo " CC        $(subst $(build_dir)/,,$(1))"; \
> -	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2) -o $(1)
> +	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(subst
> $(src_dir)/,,$(2)) -o $(1)

Looks good to me.

Reviewed-by: Dong Du <Dd_nirvana@sjtu.edu.cn>

> compile_as_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
> 	     echo " AS-DEP    $(subst $(build_dir)/,,$(1))"; \
> 	     printf %s `dirname $(1)`/ > $(1) && \
> --
> 2.30.2
> 
> 
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Xiang W]

? 2021-11-28???? 17:55 -0800?Vagrant Cascadian???
> Upstream commit 12753d22563f7d2d01f2c6644c7b66b06eb5c90f introduced
> uses of __FILE__ which may result in the build path getting embedded
> into the resulting binary.
> 
> https://reproducible-builds.org/docs/build-path/
> 
> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Cc: Xiang W <wxjstz@126.com>
> Cc: Anup Patel <anup@brainfault.org>
Looks good to me.

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> Changes since v1:
> 
> * Pass relative paths to the compiler instead of using
> ? -ffile-prefix-map to strip out the full paths, as suggested by
> Xiang
> ? W.
> 
> ?Makefile | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8623c1c..d017534 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname
> $(1)`; \
> ?????????????? -MM $(2) >> $(1) || rm -f $(1)
> ?compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
> ???????????? echo " CC??????? $(subst $(build_dir)/,,$(1))"; \
> -??????????? $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2)
> -o $(1)
> +??????????? $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c
> $(subst $(src_dir)/,,$(2)) -o $(1)
> ?compile_as_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
> ???????????? echo " AS-DEP??? $(subst $(build_dir)/,,$(1))"; \
> ???????????? printf %s `dirname $(1)`/ > $(1) && \

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Andreas Schwab]

On Nov 28 2021, Vagrant Cascadian wrote:

> diff --git a/Makefile b/Makefile
> index 8623c1c..d017534 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>  	       -MM $(2) >> $(1) || rm -f $(1)
>  compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>  	     echo " CC        $(subst $(build_dir)/,,$(1))"; \
> -	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2) -o $(1)
> +	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(subst $(src_dir)/,,$(2)) -o $(1)

How about making src_dir empty throughout?  This now depends on
$(src_dir) being identical to ".", so making that explicit would be
better.

Andreas.

-- 
Andreas Schwab, schwab at linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Vagrant Cascadian]

On 2021-11-29, Andreas Schwab wrote:
> On Nov 28 2021, Vagrant Cascadian wrote:
>
>> diff --git a/Makefile b/Makefile
>> index 8623c1c..d017534 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>>  	       -MM $(2) >> $(1) || rm -f $(1)
>>  compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>>  	     echo " CC        $(subst $(build_dir)/,,$(1))"; \
>> -	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2) -o $(1)
>> +	     $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(subst $(src_dir)/,,$(2)) -o $(1)
>
> How about making src_dir empty throughout?  This now depends on
> $(src_dir) being identical to ".", so making that explicit would be
> better.

Is this more-or-less what you were suggesting?

diff --git a/Makefile b/Makefile
index 8623c1c..cd4be3c 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@ READLINK ?= readlink
 endif
 
 # Find out source, build, and install directories
-src_dir=$(CURDIR)
+src_dir=.
 ifdef O
  build_dir=$(shell $(READLINK) -f $(O))
 else


It builds fine (haven't boot tested), and doesn't embed the build path
in the binaries.


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20211129/56b04f77/attachment-0001.sig>

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Anup Patel]

On Mon, Nov 29, 2021 at 7:25 AM Vagrant Cascadian
<vagrant@reproducible-builds.org> wrote:
>
> Upstream commit 12753d22563f7d2d01f2c6644c7b66b06eb5c90f introduced
> uses of __FILE__ which may result in the build path getting embedded
> into the resulting binary.
>
> https://reproducible-builds.org/docs/build-path/
>
> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Cc: Xiang W <wxjstz@126.com>
> Cc: Anup Patel <anup@brainfault.org>

Now that we have sbi_panic() and SBI_ASSERT() in-place (Jessica's patch), we
don't need to change the top-level Makefile because __FILE__ is not
used anywhere
in sources.

Going forward, we should avoid taking patches that use __FILE__ so that we don't
break reproducible builds.

Regards,
Anup

> ---
> Changes since v1:
>
> * Pass relative paths to the compiler instead of using
>   -ffile-prefix-map to strip out the full paths, as suggested by Xiang
>   W.
>
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 8623c1c..d017534 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -361,7 +361,7 @@ compile_cc_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>                -MM $(2) >> $(1) || rm -f $(1)
>  compile_cc = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>              echo " CC        $(subst $(build_dir)/,,$(1))"; \
> -            $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(2) -o $(1)
> +            $(CC) $(CFLAGS) $(call dynamic_flags,$(1),$(2)) -c $(subst $(src_dir)/,,$(2)) -o $(1)
>  compile_as_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>              echo " AS-DEP    $(subst $(build_dir)/,,$(1))"; \
>              printf %s `dirname $(1)`/ > $(1) && \
> --
> 2.30.2
>

[PATCH v2] Makefile: Pass relative paths to the compiler.

[Vagrant Cascadian]

On 2021-12-02, Anup Patel wrote:
> On Mon, Nov 29, 2021 at 7:25 AM Vagrant Cascadian
> <vagrant@reproducible-builds.org> wrote:
>>
>> Upstream commit 12753d22563f7d2d01f2c6644c7b66b06eb5c90f introduced
>> uses of __FILE__ which may result in the build path getting embedded
>> into the resulting binary.
>>
>> https://reproducible-builds.org/docs/build-path/
>>
>> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
>> Cc: Xiang W <wxjstz@126.com>
>> Cc: Anup Patel <anup@brainfault.org>
>
> Now that we have sbi_panic() and SBI_ASSERT() in-place (Jessica's patch), we
> don't need to change the top-level Makefile because __FILE__ is not
> used anywhere
> in sources.

I can confirm that fixes the issue of embedded build paths.


> Going forward, we should avoid taking patches that use __FILE__ so that we don't
> break reproducible builds.

Thanks for keeping reproducible builds in mind!


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20211203/d1985595/attachment.sig>