Hello UEFI builders!
I'd like your feedback on this patch.
I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can often recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code.
As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with "dirty" appended if the user is building with uncommited mods.
I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill.
Comments?
Cheers, Ryan.
--
From af952a40005536215cedbbeea9102915e461486f Mon Sep 17 00:00:00 2001
From: Ryan Harkin ryan.harkin@linaro.org Date: Tue, 30 Jun 2015 15:10:56 +0100 Subject: [PATCH] Use commit id as default FIRMWARE_VER
If a FIRMWARE_VER setting is not provided and we are building from a GIT repo, provide a default value based on the SHA ID of the commit where we built from.
If the repo has local uncommited changes, then we mark the version string with "-dirty".
This is based loosely on the same idea from the linux kernel in: scripts/setlocalversion
Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- uefi-build.sh | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/uefi-build.sh b/uefi-build.sh index 486cf86..60ea872 100755 --- a/uefi-build.sh +++ b/uefi-build.sh @@ -27,6 +27,16 @@ function build_platform PLATFORM_DSC="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o dsc`" PLATFORM_ARCH="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o arch`"
+ if [[ $PLATFORM_BUILDFLAGS != *"FIRMWARE_VER"* ]]; then + if test -d .git && head=`git rev-parse --verify --short HEAD 2>/dev/null`; then + FIRMWARE_VER=`git rev-parse --short HEAD` + if ! git diff-index --quiet HEAD --; then + FIRMWARE_VER="${FIRMWARE_VER}-dirty" + fi + PLATFORM_BUILDFLAGS="$PLATFORM_BUILDFLAGS -D FIRMWARE_VER=$FIRMWARE_VER" + fi + fi + set_cross_compile CROSS_COMPILE="$TEMP_CROSS_COMPILE"
On 30 June 2015 at 18:22, Ryan Harkin ryan.harkin@linaro.org wrote:
Hello UEFI builders!
I'd like your feedback on this patch.
I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can often recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code.
As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with "dirty" appended if the user is building with uncommited mods.
I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill.
Comments?
Looks good to me. I haven't been in this position myself, but I see how it would be very useful to have this information available in a binary build.
I have got the similar lines in my CI system. It is quite useful. But I only enable the FIRMWARE_VER when the binary is produced by the CI. When the engineer build the firmware on his/her machine I leave the version empty.
But your ‘-dirty’ addition is also useful.
From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Ryan Harkin Sent: 30 June 2015 17:22 To: Linaro UEFI Mailman List; Leif Lindholm Subject: [Linaro-uefi] [RFC] Use commit id as default FIRMWARE_VER
Hello UEFI builders! I'd like your feedback on this patch. I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can often recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code. As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with "dirty" appended if the user is building with uncommited mods. I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill. Comments? Cheers, Ryan.
-- From af952a40005536215cedbbeea9102915e461486f Mon Sep 17 00:00:00 2001 From: Ryan Harkin <ryan.harkin@linaro.orgmailto:ryan.harkin@linaro.org> Date: Tue, 30 Jun 2015 15:10:56 +0100 Subject: [PATCH] Use commit id as default FIRMWARE_VER
If a FIRMWARE_VER setting is not provided and we are building from a GIT repo, provide a default value based on the SHA ID of the commit where we built from.
If the repo has local uncommited changes, then we mark the version string with "-dirty".
This is based loosely on the same idea from the linux kernel in: scripts/setlocalversion
Signed-off-by: Ryan Harkin <ryan.harkin@linaro.orgmailto:ryan.harkin@linaro.org> --- uefi-build.sh | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/uefi-build.sh b/uefi-build.sh index 486cf86..60ea872 100755 --- a/uefi-build.sh +++ b/uefi-build.sh @@ -27,6 +27,16 @@ function build_platform PLATFORM_DSC="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o dsc`" PLATFORM_ARCH="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o arch`"
+ if [[ $PLATFORM_BUILDFLAGS != *"FIRMWARE_VER"* ]]; then + if test -d .git && head=`git rev-parse --verify --short HEAD 2>/dev/null`; then + FIRMWARE_VER=`git rev-parse --short HEAD` + if ! git diff-index --quiet HEAD --; then + FIRMWARE_VER="${FIRMWARE_VER}-dirty" + fi + PLATFORM_BUILDFLAGS="$PLATFORM_BUILDFLAGS -D FIRMWARE_VER=$FIRMWARE_VER" + fi + fi + set_cross_compile CROSS_COMPILE="$TEMP_CROSS_COMPILE"
-- 2.1.0
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
On 1 July 2015 at 15:24, Olivier Martin Olivier.Martin@arm.com wrote:
I have got the similar lines in my CI system. It is quite useful.
But I only enable the FIRMWARE_VER when the binary is produced by the CI.
I like the feature from the Linux script where it will use an (annotated) tag if one exists. I thought that would be useful for releases. But that's relies on the tags being annotated, which we rarely do.
When the engineer build the firmware on his/her machine I leave the version empty.
With our new Platforms releases, the users are all building from source now, so that's my main use case - to make sure they're building from the correct source ;-)
I'm also thinking about a similar patch for ARM-TF.
But your ‘-dirty’ addition is also useful.
Yes, it should help me identify sneaky users!!
*From:* Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] *On Behalf Of *Ryan Harkin *Sent:* 30 June 2015 17:22 *To:* Linaro UEFI Mailman List; Leif Lindholm *Subject:* [Linaro-uefi] [RFC] Use commit id as default FIRMWARE_VER
Hello UEFI builders!
I'd like your feedback on this patch.
I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can often recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code.
As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with "dirty" appended if the user is building with uncommited mods.
I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill.
Comments?
Cheers,
Ryan.
--
From af952a40005536215cedbbeea9102915e461486f Mon Sep 17 00:00:00 2001 From: Ryan Harkin ryan.harkin@linaro.org Date: Tue, 30 Jun 2015 15:10:56 +0100 Subject: [PATCH] Use commit id as default FIRMWARE_VER
If a FIRMWARE_VER setting is not provided and we are building from a GIT repo, provide a default value based on the SHA ID of the commit where we built from.
If the repo has local uncommited changes, then we mark the version string with "-dirty".
This is based loosely on the same idea from the linux kernel in: scripts/setlocalversion
Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
uefi-build.sh | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/uefi-build.sh b/uefi-build.sh index 486cf86..60ea872 100755 --- a/uefi-build.sh +++ b/uefi-build.sh @@ -27,6 +27,16 @@ function build_platform PLATFORM_DSC="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o dsc`" PLATFORM_ARCH="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $board get -o arch`"
- if [[ $PLATFORM_BUILDFLAGS != *"FIRMWARE_VER"* ]]; then
if test -d .git && head=`git rev-parse --verify --short HEAD2>/dev/null`; then
FIRMWARE_VER=`git rev-parse --short HEAD`if ! git diff-index --quiet HEAD --; thenFIRMWARE_VER="${FIRMWARE_VER}-dirty"fiPLATFORM_BUILDFLAGS="$PLATFORM_BUILDFLAGS -DFIRMWARE_VER=$FIRMWARE_VER"
fi- fi
- set_cross_compile CROSS_COMPILE="$TEMP_CROSS_COMPILE"
-- 2.1.0
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
On Wed, Jul 01, 2015 at 03:42:11PM +0100, Ryan Harkin wrote:
On 1 July 2015 at 15:24, Olivier Martin Olivier.Martin@arm.com wrote:
When the engineer build the firmware on his/her machine I leave the version empty.
With our new Platforms releases, the users are all building from source now, so that's my main use case - to make sure they're building from the correct source ;-)
Indeed - a common use case for this is user reports that "I built $THING" when they did nothing of the sort.
On Tue, Jun 30, 2015 at 05:22:18PM +0100, Ryan Harkin wrote:
Hello UEFI builders!
Hello UEFI builder!
I'd like your feedback on this patch.
I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can often recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code.
As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with "dirty" appended if the user is building with uncommited mods.
I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill.
Comments?
I really like the idea, but we should be able to break it out to the top level, and only perform the logic once per run of the script - like so:
diff --git a/uefi-build.sh b/uefi-build.sh index 486cf86..228b885 100755 --- a/uefi-build.sh +++ b/uefi-build.sh @@ -219,6 +219,16 @@ fi
EDK2_DIR="$PWD"
+if [[ "${EXTRA_OPTIONS[@]}" != *"FIRMWARE_VER"* ]]; then + if test -d .git && head=`git rev-parse --verify --short HEAD 2>/dev/null`; then + FIRMWARE_VER=`git rev-parse --short HEAD` + if ! git diff-index --quiet HEAD --; then + FIRMWARE_VER="${FIRMWARE_VER}-dirty" + fi + EXTRA_OPTIONS=( $EXTRA_OPTIONS "-D" FIRMWARE_VER=$FIRMWARE_VER ) + fi +fi + uefishell
for board in "${builds[@]}" ; do
Would you have any objections to that modification?
/ Leif
On 8 July 2015 at 14:18, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Jun 30, 2015 at 05:22:18PM +0100, Ryan Harkin wrote:
Hello UEFI builders!
Hello UEFI builder!
:-)
I'd like your feedback on this patch.
I often look at boot logs from LAVA and other people and don't quite know where the UEFI binary came from. With ones that I've provided, I can
often
recognise the build date stamp. But at other times, I'm simply trying to work out if someone is building the code they say they are, or some other code.
As the commit message says, rather than leaving the version string blank, my patch will append the git commit id of the current commit, with
"dirty"
appended if the user is building with uncommited mods.
I considered including the code from linux scripts/setlocalversion, but apart from giving me a "-" at the front, I thought it was overkill.
Comments?
I really like the idea, but we should be able to break it out to the top level, and only perform the logic once per run of the script - like so:
diff --git a/uefi-build.sh b/uefi-build.sh index 486cf86..228b885 100755 --- a/uefi-build.sh +++ b/uefi-build.sh @@ -219,6 +219,16 @@ fi
EDK2_DIR="$PWD"
+if [[ "${EXTRA_OPTIONS[@]}" != *"FIRMWARE_VER"* ]]; then
if test -d .git && head=`git rev-parse --verify --short HEAD2>/dev/null`; then
FIRMWARE_VER=`git rev-parse --short HEAD`if ! git diff-index --quiet HEAD --; thenFIRMWARE_VER="${FIRMWARE_VER}-dirty"fiEXTRA_OPTIONS=( $EXTRA_OPTIONS "-D"FIRMWARE_VER=$FIRMWARE_VER )
fi+fi
uefishell
for board in "${builds[@]}" ; do
Would you have any objections to that modification?
Good point, well spotted. I'm happy for you to submit it like that, but let me know if you prefer me to post an updated patch.
On 8 July 2015 at 15:25, Leif Lindholm leif.lindholm@linaro.org wrote:
On Wed, Jul 08, 2015 at 02:44:16PM +0100, Ryan Harkin wrote:
Would you have any objections to that modification?
Good point, well spotted. I'm happy for you to submit it like that, but let me know if you prefer me to post an updated patch.
Pushed, thanks.
Marvellous, thank you sir.
While I have your attention.... testing the change, I noticed something funny I hadn't paid attention to before. This patch:
commit 6bc0d0a33fa3a327f9baa85d3443fcae323f1618 Author: Leif Lindholm leif.lindholm@linaro.org Date: Sun Nov 9 13:05:54 2014 +0000
Add APM Mustang platform to config.
... adds "-D FIRMWARE_VENDOR=Linaro -D FIRMWARE_VERSION=2014.10-a2" to BUILDFLAGS. I take it that was an accident or something?
On Wed, Jul 08, 2015 at 04:31:59PM +0100, Ryan Harkin wrote:
Would you have any objections to that modification?
Good point, well spotted. I'm happy for you to submit it like that, but let me know if you prefer me to post an updated patch.
Pushed, thanks.
Marvellous, thank you sir.
While I have your attention.... testing the change, I noticed something funny I hadn't paid attention to before. This patch:
commit 6bc0d0a33fa3a327f9baa85d3443fcae323f1618 Author: Leif Lindholm <leif.lindholm@linaro.org> Date: Sun Nov 9 13:05:54 2014 +0000 Add APM Mustang platform to config.... adds "-D FIRMWARE_VENDOR=Linaro -D FIRMWARE_VERSION=2014.10-a2" to BUILDFLAGS. I take it that was an accident or something?
Blimey - what a mistake-a to make-a. Now gone.
/ Leif
On 9 July 2015 at 10:21, Leif Lindholm leif.lindholm@linaro.org wrote:
On Wed, Jul 08, 2015 at 04:31:59PM +0100, Ryan Harkin wrote:
Would you have any objections to that modification?
Good point, well spotted. I'm happy for you to submit it like that,
but
let me know if you prefer me to post an updated patch.
Pushed, thanks.
Marvellous, thank you sir.
While I have your attention.... testing the change, I noticed something funny I hadn't paid attention to before. This patch:
commit 6bc0d0a33fa3a327f9baa85d3443fcae323f1618 Author: Leif Lindholm <leif.lindholm@linaro.org> Date: Sun Nov 9 13:05:54 2014 +0000 Add APM Mustang platform to config.... adds "-D FIRMWARE_VENDOR=Linaro -D FIRMWARE_VERSION=2014.10-a2" to BUILDFLAGS. I take it that was an accident or something?
Blimey - what a mistake-a to make-a. Now gone.
Nice commit message :-)
-
Ard Biesheuvel -
Leif Lindholm -
Mark Brown -
Olivier Martin -
Ryan Harkin