Makefile: Correctly take source date from commit date
PATCH 1/1 Makefile: Correctly take source date from commit date
For reproducible builds SOURCE_DATE_EPOCH is supposed to be set to the date of the last Buildroot commit if running from a git repository, but this doesn't actually work. This corrects this in the following way:
1) Set GIT from BR2_GIT, otherwise it may be undefined.
2) Use 'git rev-parse --is-inside-work-tree' to see if we are inside a Git repository. There is no need to use TOPDIR, because the current directory will always be TOPDIR, and in any case the Git repository may be at a higher directory level if Buildroot has been imported into a larger build system.
--- Makefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Makefile +4 -2
@@ -253,8 +253,10 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +GIT := $(call qstrip,$(BR2_GIT)) +BR2_IN_WORK_TREE = $(if $(subst false,,$(shell $(GIT) rev-parse --is-inside-work-tree 2> /dev/null || echo false)),Y,) +BR2_VERSION_GIT_EPOCH = $(shell $(GIT) log -1 --format=%at) +export SOURCE_DATE_EPOCH ?= $(if $(BR2_IN_WORK_TREE),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default
--
2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
PATCH 1/1 Makefile: Correctly take source date from commit date
Quoted 7 lines · James Byrne
For reproducible builds
SOURCE_DATE_EPOCHis supposed to be set to the date of the last Buildroot commit if running from a git repository, but this doesn't actually work. This corrects this in the following way:1) Set GIT from
BR2_GIT, otherwise it may be undefined.2) Use 'git rev-parse --is-inside-work-tree' to see if we are inside a
If you say "1 and 2" in your commit log, that's a hint it should be two
separate patches :-)
Quoted 4 lines
Git repository. There is no need to use TOPDIR, because the current directory will always be TOPDIR, and in any case the Git repository may be at a higher directory level if Buildroot has been imported into a larger build system.
Hm, but in that case, do you really want SOURCE_DATE_EPOCH to be set to the git
epoch of the larger build system? I think that depends a lot on the exact circumstances.
Anyway, a much more likely situation is that your "larger build system"
includes one or more BR2_EXTERNAL directories - and those are typically a lot more volatile than the Buildroot directory itself. So maybe the source epoch should be set to the git epoch of one of those?
I think there are just too many different situations here, and that we should
just rely on the user explicitly setting SOURCE_DATE_EPOCH to whatever is appropriate for her/him. That said, using any surrounding git repo sounds like a sane default. So in the end I'm not really opposed to your patch.
Quoted 11 lines
Signed-off-by James Byrne <[email protected]>--- Makefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)Makefile +0
-0
Makefile export TZ = UTC · line 256
Quoted 6 lines
Makefile +1
-2@@ -253,8 +253,10 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +GIT := $(call qstrip,$(BR2_GIT))
This is in fact defined in package/pkg-download.mk. I guess the problem is that
that that file doesn't get included on an unconfigured system. However, in that case, how does your BR2_REPRODUCIBLE get set? Can you specify under which circumstances this variable is not defined?
Makefile export TZ = UTC · line 257
Quoted 1 line
Makefile +1
-0+BR2_IN_WORK_TREE = $(if $(subst false,,$(shell $(GIT) rev-parse --is-inside-work-tree 2> /dev/null || echo false)),Y,)
This would be much simpler:
BR2_IN_WORK_TREE = $(shell $(GIT) rev-parse --is-inside-work-tree 2> /dev/null)
This will be empty in case it is not in a work tree, and 'true' if it is.
Makefile export TZ = UTC · line 259
Quoted 2 lines
Makefile +2
-0+BR2_VERSION_GIT_EPOCH = $(shell $(GIT) log -1 --format=%at) +export SOURCE_DATE_EPOCH ?= $(if $(BR2_IN_WORK_TREE),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH))
But actually, it could be made a whole lot simpler:
BR2_VERSION_GIT_EPOCH = $(shell $(GIT) log -1 --format=%at 2> /dev/null) export SOURCE_DATE_EPOCH ?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH))
BR2_VERSION_GIT_EPOCH will be empty if it's not in a worktree.
Regards,
Arnout
Quoted 12 lines
Makefile +0
-1endif # To put more focus on warnings, be less verbose as default --2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 Makefile: Correctly take source date from commit date
Hi Arnout,
Quoted 15 lines · Arnout Vandecappelle
Quoted 4 lines · James Byrne
Git repository. There is no need to use TOPDIR, because the current directory will always be TOPDIR, and in any case the Git repository may be at a higher directory level if Buildroot has been imported into a larger build system.
Hm, but in that case, do you really want
SOURCE_DATE_EPOCHto be set to the git
epoch of the larger build system? I think that depends a lot on the exact circumstances.Anyway, a much more likely situation is that your "larger build system"
includes one or moreBR2_EXTERNALdirectories - and those are typically a lot more volatile than the Buildroot directory itself. So maybe the source epoch should be set to the git epoch of one of those?
Quoted 4 lines
I think there are just too many different situations here, and that we should
just rely on the user explicitly settingSOURCE_DATE_EPOCHto whatever is appropriate for her/him. That said, using any surrounding git repo sounds like a sane default. So in the end I'm not really opposed to your patch.
You are right. I had not understood the significance of SOURCE_DATE_EPOCH, but do now, thanks. I still think however that taking the commit date of whatever repository you are in is a better default than only doing so if the repository is at the same level as the Buildroot tree.
Quoted 5 lines
Quoted 5 lines
Signed-off-by James Byrne <[email protected]>--- Makefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Quoted 16 lines
Makefile export TZ = UTC · line 256
Quoted 11 lines
Makefile +1
-2@@ -253,8 +253,10 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +GIT := $(call qstrip,$(BR2_GIT))This is in fact defined in
package/pkg-download.mk. I guess the problem is that
that that file doesn't get included on an unconfigured system. However, in that case, how does yourBR2_REPRODUCIBLEget set? Can you specify under which circumstances this variable is not defined?
I have to apologise here because I confused myself by adding debugging to the Makefile that caused SOURCE_DATE_EPOCH to get evaluated immediately (before Makefile.in was included), and hence I got an error instead of the correct date. In normal usage however this would never happen, so this change is unnecessary.
It did make me realise though that the 'git log' command is executed every time SOURCE_DATE_EPOCH is referenced (over 400 times in the build I tested), which is not very efficient given that the answer cannot change, so I will submit a revised patch that addresses this.
Quoted 8 lines
Makefile export TZ = UTC · line 257
Quoted 1 line
Makefile +1
-0+BR2_IN_WORK_TREE = $(if $(subst false,,$(shell $(GIT) rev-parse --is-inside-work-tree 2> /dev/null || echo false)),Y,)This would be much simpler:
BR2_IN_WORK_TREE= $(shell $(GIT) rev-parse --is-inside-work-tree 2>/dev/null)This will be empty in case it is not in a work tree, and 'true' if it is.
Quoted 8 lines
Makefile export TZ = UTC · line 259
Quoted 2 lines
Makefile +2
-0+BR2_VERSION_GIT_EPOCH = $(shell $(GIT) log -1 --format=%at) +export SOURCE_DATE_EPOCH ?= $(if $(BR2_IN_WORK_TREE),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH))But actually, it could be made a whole lot simpler:
BR2_VERSION_GIT_EPOCH= $(shell $(GIT) log -1 --format=%at 2>/dev/null) exportSOURCE_DATE_EPOCH?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH))
BR2_VERSION_GIT_EPOCHwill be empty if it's not in a worktree.
Good point, that's much neater. I will use this in my revised patch.
Regards,
James
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
PATCH 1/1 Makefile: Correctly take source date from commit date
In reply to James ByrneJames, All,
On 2018-03-28 13:26 +0100, James Byrne spake thusly:
Quoted 11 lines
For reproducible builds
SOURCE_DATE_EPOCHis supposed to be set to the date of the last Buildroot commit if running from a git repository, but this doesn't actually work. This corrects this in the following way:1) Set GIT from
BR2_GIT, otherwise it may be undefined.2) Use 'git rev-parse --is-inside-work-tree' to see if we are inside a Git repository. There is no need to use TOPDIR, because the current directory will always be TOPDIR, and in any case the Git repository may be at a higher directory level if Buildroot has been imported into a larger build system.
Besides what Arnout said, I'm not a big fan of this patch.
Buildroot does abide by the rules: if SOURCE_DATE_EPOCH is seet in the environemnt, we do not set it.
Otherwise we do set it to a date that is meaningful to Buildroot itself.
In an upper-layer buildsystem wants to use its own date, it should set it in the environment before calling us.
That's what the spec about SOURCE_DATE_EPOCH mandates:
https://reproducible-builds.org/specs/source-date-epoch/
So, I would just drop that patch.
Now, having SOURCE_DATE_EPOCH set correctly in an unconfigured tree could be fixed, in deed, but what would be the use case for that?
Regards,
Yann E. MORIN.
Quoted 30 lines
Signed-off-by James Byrne <[email protected]>--- Makefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)Makefile +4
-3@@ -253,8 +253,10 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +GIT := $(call qstrip,$(BR2_GIT)) +BR2_IN_WORK_TREE = $(if $(subst false,,$(shell $(GIT) rev-parse --is-inside-work-tree 2> /dev/null || echo false)),Y,) +BR2_VERSION_GIT_EPOCH = $(shell $(GIT) log -1 --format=%at) +export SOURCE_DATE_EPOCH ?= $(if $(BR2_IN_WORK_TREE),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default --2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
In reply to James ByrneIf SOURCE_DATE_EPOCH is not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.
This commit moves the definition of BR2_VERSION_GIT_EPOCH after the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.
In addition, the git directory is no longer explicitly set to be TOPDIR meaning that the default date will come from the commit date of whatever repository the code is commited within instead of only doing so if the repository is at the same level as the Buildroot tree.
--- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Makefile +8 -2
@@ -247,8 +247,6 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default
@@ -504,6 +502,14 @@ include support/dependencies/dependencies.mk include $(sort $(wildcard toolchain/*.mk)) include $(sort $(wildcard toolchain/*/*.mk)) +ifeq ($(BR2_REPRODUCIBLE),y) +# If SOURCE_DATE_EPOCH has not been set then use the commit date, or the last +# release date if the source tree is not within a Git repository. +# See: https://reproducible-builds.org/specs/source-date-epoch/ +BR2_VERSION_GIT_EPOCH := $(shell $(GIT) log -1 --format=%at 2> /dev/null) +export SOURCE_DATE_EPOCH ?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +endif + # Include the package override file if one has been provided in the # configuration. PACKAGE_OVERRIDE_FILE = $(call qstrip,$(BR2_PACKAGE_OVERRIDE_FILE))
--
2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
PATCH v2 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
James, All,
On 2018-04-06 11:26 +0100, James Byrne spake thusly:
Quoted 4 lines
If
SOURCE_DATE_EPOCHis not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.
Quoted 8 lines
This commit moves the definition of
BR2_VERSION_GIT_EPOCHafter the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.In addition, the git directory is no longer explicitly set to be TOPDIR meaning that the default date will come from the commit date of whatever repository the code is commited within instead of only doing so if the repository is at the same level as the Buildroot tree.
Since your are saying "in addition", it smells like this should have been two seaparate commits:
- one to change the variable to being simply expanded
- a second one to not enforce the git directory.
I am totally OK with the first change, while I am still not convinced by the second. As I already said, if Buildroot is used from a higher-level buildsystem, it is the responsibility of that higher-level buildsystem to appropriately set SOURCE_DATE_EPOCH in the environment.
Also, bear in mind that, even without a higher-level buildsystem, it is possible to call Buildroot without actualy being in Buildroot's tree, e.g.:
$ make -C /path/to/buildroot O=/path/to/build-dir
Or even:
$ cd /path/to/build-dir
$ make -C /path/to/buildroot O=$(pwd) foo_defconfig
$ makeAnd then you would lose the git info that is present in the Buildroot tree in /path/to/buildroot. I actually use either forms quite a lot, especially the second one. In fact, I even never build in the Buildroot tree, except when people report it is broken and I need to investigate.
So, I am still opposed to the second change.
Regards,
Yann E. MORIN.
Quoted 41 lines
Signed-off-by James Byrne <[email protected]>--- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)Makefile +8
-3@@ -247,8 +247,6 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default@@ -504,6 +502,14 @@ include support/dependencies/dependencies.mk include $(sort $(wildcard toolchain/*.mk)) include $(sort $(wildcard toolchain/*/*.mk)) +ifeq ($(BR2_REPRODUCIBLE),y) +# If SOURCE_DATE_EPOCH has not been set then use the commit date, or the last +# release date if the source tree is not within a Git repository. +# See: https://reproducible-builds.org/specs/source-date-epoch/ +BR2_VERSION_GIT_EPOCH := $(shell $(GIT) log -1 --format=%at 2> /dev/null) +export SOURCE_DATE_EPOCH ?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +endif + # Include the package override file if one has been provided in the # configuration. PACKAGE_OVERRIDE_FILE = $(call qstrip,$(BR2_PACKAGE_OVERRIDE_FILE)) --2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
Hi Yann,
Quoted 17 lines · Yann E. MORIN
On 2018-04-06 11:26 +0100, James Byrne spake thusly:
Quoted 4 lines
If
SOURCE_DATE_EPOCHis not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.Quoted 8 lines
This commit moves the definition of
BR2_VERSION_GIT_EPOCHafter the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.In addition, the git directory is no longer explicitly set to be TOPDIR meaning that the default date will come from the commit date of whatever repository the code is commited within instead of only doing so if the repository is at the same level as the Buildroot tree.
Since your are saying "in addition", it smells like this should have been two seaparate commits:
Quoted 8 lines
- one to change the variable to being simply expanded
- a second one to not enforce the git directory.
I am totally OK with the first change, while I am still not convinced by the second. As I already said, if Buildroot is used from a higher-level buildsystem, it is the responsibility of that higher-level buildsystem to appropriately set
SOURCE_DATE_EPOCHin the environment.
Understood, I will resubmit this patch without the second change.
Quoted 12 lines
Also, bear in mind that, even without a higher-level buildsystem, it is possible to call Buildroot without actualy being in Buildroot's tree, e.g.:
$ make -C /path/to/buildroot O=/path/to/build-dirOr even:
$ cd /path/to/build-dir $ make -C /path/to/buildroot O=$(pwd) foo_defconfig $ make
Quoted 6 lines
And then you would lose the git info that is present in the Buildroot tree in /path/to/buildroot. I actually use either forms quite a lot, especially the second one. In fact, I even never build in the Buildroot tree, except when people report it is broken and I need to investigate.
So, I am still opposed to the second change.
In both of your examples it will still work as expected since the buildroot makefile is always invoked with "-C /path/to/buildroot", which means that when the 'git log' command is executed the current directory is always '/path/to/buildroot' and so the commit date will be the date of the tree that the Buildroot makefile is in.
If you think you might change your mind I will submit that change as a new patch, as I still think it makes a better default, but if you don't agree then I won't bother as it's a very minor thing, and as you say it can easily be overridden by setting SOURCE_DATE_EPOCH in the environment.
James
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
PATCH v2 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
James, All,
On 2018-04-09 16:50 +0100, James Byrne spake thusly:
Quoted 1 line · Yann E. MORIN
[--SNIP--]
Quoted 6 lines
Quoted 1 line
So, I am still opposed to the second change.
In both of your examples it will still work as expected since the buildroot makefile is always invoked with "-C /path/to/buildroot", which means that when the 'git log' command is executed the current directory is always '/path/to/buildroot' and so the commit date will be the date of the tree that the Buildroot makefile is in.
Indeed, you are absolutely right. I was totally oblivious to that fact, even though I moderately participated in that...
Quoted 4 lines
If you think you might change your mind I will submit that change as a new patch, as I still think it makes a better default, but if you don't agree then I won't bother as it's a very minor thing, and as you say it can easily be overridden by setting
SOURCE_DATE_EPOCHin the environment.
So, with your explanations above, I am no longer opposed to that second change.
Be sure to provide adequate explanations in the commit log, though. Maybe something that list the two cases:
- if Buildroot is a git tree on its own, then the 'git log' will act
on that tree because we always cd into Buildroot's tree;
- if Buildroot is just a sub-directory in a upper-layer git tree, then the git log would act on that directory.
Thank you!
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v3 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
In reply to James ByrneIf SOURCE_DATE_EPOCH is not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.
This commit moves the definition of BR2_VERSION_GIT_EPOCH after the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.
--- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Makefile +8 -2
@@ -247,8 +247,6 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default
@@ -504,6 +502,14 @@ include support/dependencies/dependencies.mk include $(sort $(wildcard toolchain/*.mk)) include $(sort $(wildcard toolchain/*/*.mk)) +ifeq ($(BR2_REPRODUCIBLE),y) +# If SOURCE_DATE_EPOCH has not been set then use the commit date, or the last +# release date if the source tree is not within a Git repository. +# See: https://reproducible-builds.org/specs/source-date-epoch/ +BR2_VERSION_GIT_EPOCH := $(shell $(GIT) --git-dir=$(TOPDIR)/.git log -1 --format=%at 2> /dev/null) +export SOURCE_DATE_EPOCH ?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +endif + # Include the package override file if one has been provided in the # configuration. PACKAGE_OVERRIDE_FILE = $(call qstrip,$(BR2_PACKAGE_OVERRIDE_FILE))
--
2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
PATCH v3 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
James, All,
On 2018-04-09 17:08 +0100, James Byrne spake thusly:
Quoted 9 lines
If
SOURCE_DATE_EPOCHis not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.This commit moves the definition of
BR2_VERSION_GIT_EPOCHafter the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.Signed-off-by James Byrne <[email protected]>
Thanks for the respin! :-)
Regards,
Yann E. MORIN.
Quoted 36 lines
--- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)Makefile +8
-3@@ -247,8 +247,6 @@ export TZ = UTC export LANG = C export LC_ALL = C export GZIP = -n -BR2_VERSION_GIT_EPOCH = $(shell GIT_DIR=$(TOPDIR)/.git $(GIT) log -1 --format=%at) -export SOURCE_DATE_EPOCH ?= $(if $(wildcard $(TOPDIR)/.git),$(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) endif # To put more focus on warnings, be less verbose as default@@ -504,6 +502,14 @@ include support/dependencies/dependencies.mk include $(sort $(wildcard toolchain/*.mk)) include $(sort $(wildcard toolchain/*/*.mk)) +ifeq ($(BR2_REPRODUCIBLE),y) +# If SOURCE_DATE_EPOCH has not been set then use the commit date, or the last +# release date if the source tree is not within a Git repository. +# See: https://reproducible-builds.org/specs/source-date-epoch/ +BR2_VERSION_GIT_EPOCH := $(shell $(GIT) --git-dir=$(TOPDIR)/.git log -1 --format=%at 2> /dev/null) +export SOURCE_DATE_EPOCH ?= $(or $(BR2_VERSION_GIT_EPOCH),$(BR2_VERSION_EPOCH)) +endif + # Include the package override file if one has been provided in the # configuration. PACKAGE_OVERRIDE_FILE = $(call qstrip,$(BR2_PACKAGE_OVERRIDE_FILE)) --2.7.4
The contents of this email and any attachment are confidential to the intended recipient(s). If you are not an intended recipient: (i) do not use, disclose, distribute, copy or publish this email or its contents; (ii) please contact the sender immediately; and (iii) delete this email. Origami Energy Limited (company number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 Limited (company number 10933403), each registered in England and each with a registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Thomas Petazzoni
AppliedPATCH v3 1/1 Makefile: Avoid executing 'git log' each time SOURCE_DATE_EPOCH is used
In reply to James ByrneHello,
Quoted 4 lines · James Byrne
If
SOURCE_DATE_EPOCHis not defined it was given a definition that caused 'git log' to be executed each time the variable is referenced, which is not very efficient given that the answer cannot change.
Quoted 8 lines
This commit moves the definition of
BR2_VERSION_GIT_EPOCHafter the inclusion of Makefile.in (so that GIT is defined) and makes it a simply expanded variable so that it is only evaluated once.Signed-off-by James Byrne <[email protected]>--- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Applied to master, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com