public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools: Retrieve git version info
@ 2020-01-05 12:41 Pankaj Bansal
  2020-01-09  6:13 ` Bob Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Bansal @ 2020-01-05 12:41 UTC (permalink / raw)
  To: devel@edk2.groups.io; +Cc: Pankaj Bansal, Bob Feng, Liming Gao

Retrieve git version info and save as environment variable
These variables can be used in modules to print the vesrion
info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see http://mywiki.wooledge.org/BashFAQ/028 for a d
 dir=$(dirname "$full_cmd")
 cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE)
+export PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-05 12:41 [PATCH] BaseTools: Retrieve git version info Pankaj Bansal
@ 2020-01-09  6:13 ` Bob Feng
  2020-01-09  7:09   ` Pankaj Bansal
  0 siblings, 1 reply; 7+ messages in thread
From: Bob Feng @ 2020-01-09  6:13 UTC (permalink / raw)
  To: Pankaj Bansal, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
Why is this function implemented in build wrapper?

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com] 
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see http://mywiki.wooledge.org/BashFAQ/028 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-09  6:13 ` Bob Feng
@ 2020-01-09  7:09   ` Pankaj Bansal
  2020-01-09 10:01     ` Bob Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Bansal @ 2020-01-09  7:09 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Bob,

Thanks for replying.
Please see inline

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Thursday, January 9, 2020 11:43 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
-> Any driver or library can use this. I had tested this patch in Qemu after adding these changes :

index 0a1469550d..8e318f776f 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -18,6 +18,9 @@
 #include <Guid/EarlyPL011BaseAddress.h>
 #include <Guid/FdtHob.h>

+#define XPRINT(x) PRINT(x)^M
+#define PRINT(x) #x^M
+^M
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -98,6 +101,8 @@ PlatformPeim (
   }

   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  DEBUG ((EFI_D_ERROR, "Edk2 version is %a\n", XPRINT(WORKSPACE_GIT_VERSION)));^M
+  DEBUG ((EFI_D_ERROR, "Edk2 platforms version is %a\n", XPRINT(PACKAGES_PATH_GIT_VERSION)));^M

   return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac2..992e89b210 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -15,6 +15,10 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib

+[BuildOptions]^M
+  GCC:*_*_*_CC_FLAGS = -DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"^M
+  GCC:*_*_*_CC_FLAGS = -DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"^M
+^M
 [Sources]
   PlatformPeiLib.c
 

PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
-> I did not know this information, maybe we can enhance this function for handling ";" in PACKAGES_PATH ?
     I would need some time to work on it. I would really appreciate if anybody can help.

Why is this function implemented in build wrapper?
-> The purpose of this patch is to provide a user method to determine which source code was booted?
     Or if a UEFI driver/application has been loaded in system from shell, which was the commit version for that?
     This helps debugging in case we are provided logs of running system or logs of driver/application. 
     Since a user can make a standalone driver/application using build command, I added this function in build wrapper.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmywiki.wooledge.org%2FBashFAQ%2F028&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Cb4903cf8660c43373b1808d794cb0ea0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637141472150600156&amp;sdata=Kdr4xWA4gM%2FgttW30aZLn0T%2FUIdCsNcH0ExB0Tx4W1k%3D&amp;reserved=0 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-09  7:09   ` Pankaj Bansal
@ 2020-01-09 10:01     ` Bob Feng
  2020-01-09 10:10       ` Pankaj Bansal
  0 siblings, 1 reply; 7+ messages in thread
From: Bob Feng @ 2020-01-09 10:01 UTC (permalink / raw)
  To: Pankaj Bansal, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Pankaj,

I think if the firmware module need to show git version information, the version information need to build into firmware binary but not set to OS environment variable.

Thanks
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com] 
Sent: Thursday, January 9, 2020 3:09 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

Thanks for replying.
Please see inline

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 11:43 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
-> Any driver or library can use this. I had tested this patch in Qemu after adding these changes :

index 0a1469550d..8e318f776f 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -18,6 +18,9 @@
 #include <Guid/EarlyPL011BaseAddress.h>  #include <Guid/FdtHob.h>

+#define XPRINT(x) PRINT(x)^M
+#define PRINT(x) #x^M
+^M
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -98,6 +101,8 @@ PlatformPeim (
   }

   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  DEBUG ((EFI_D_ERROR, "Edk2 version is %a\n", 
+ XPRINT(WORKSPACE_GIT_VERSION)));^M
+  DEBUG ((EFI_D_ERROR, "Edk2 platforms version is %a\n", 
+ XPRINT(PACKAGES_PATH_GIT_VERSION)));^M

   return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac2..992e89b210 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -15,6 +15,10 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib

+[BuildOptions]^M
+  GCC:*_*_*_CC_FLAGS = 
+-DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"^M
+  GCC:*_*_*_CC_FLAGS = 
+-DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"^M
+^M
 [Sources]
   PlatformPeiLib.c
 

PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
-> I did not know this information, maybe we can enhance this function for handling ";" in PACKAGES_PATH ?
     I would need some time to work on it. I would really appreciate if anybody can help.

Why is this function implemented in build wrapper?
-> The purpose of this patch is to provide a user method to determine which source code was booted?
     Or if a UEFI driver/application has been loaded in system from shell, which was the commit version for that?
     This helps debugging in case we are provided logs of running system or logs of driver/application. 
     Since a user can make a standalone driver/application using build command, I added this function in build wrapper.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmywiki.wooledge.org%2FBashFAQ%2F028&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Cb4903cf8660c43373b1808d794cb0ea0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637141472150600156&amp;sdata=Kdr4xWA4gM%2FgttW30aZLn0T%2FUIdCsNcH0ExB0Tx4W1k%3D&amp;reserved=0 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-09 10:01     ` Bob Feng
@ 2020-01-09 10:10       ` Pankaj Bansal
  2020-01-10  6:27         ` Bob Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Bansal @ 2020-01-09 10:10 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Bob,

This OS environment variable is only defined up until the point build command is running.
As soon as build command finishes, these OS environment variables also vanish.

Regards,
Pankaj Bansal

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Thursday, January 9, 2020 3:31 PM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I think if the firmware module need to show git version information, the version information need to build into firmware binary but not set to OS environment variable.

Thanks
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Thursday, January 9, 2020 3:09 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

Thanks for replying.
Please see inline

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 11:43 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
-> Any driver or library can use this. I had tested this patch in Qemu after adding these changes :

index 0a1469550d..8e318f776f 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -18,6 +18,9 @@
 #include <Guid/EarlyPL011BaseAddress.h>  #include <Guid/FdtHob.h>

+#define XPRINT(x) PRINT(x)^M
+#define PRINT(x) #x^M
+^M
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -98,6 +101,8 @@ PlatformPeim (
   }

   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  DEBUG ((EFI_D_ERROR, "Edk2 version is %a\n", 
+ XPRINT(WORKSPACE_GIT_VERSION)));^M
+  DEBUG ((EFI_D_ERROR, "Edk2 platforms version is %a\n", 
+ XPRINT(PACKAGES_PATH_GIT_VERSION)));^M

   return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac2..992e89b210 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -15,6 +15,10 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib

+[BuildOptions]^M
+  GCC:*_*_*_CC_FLAGS =
+-DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"^M
+  GCC:*_*_*_CC_FLAGS =
+-DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"^M
+^M
 [Sources]
   PlatformPeiLib.c
 

PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
-> I did not know this information, maybe we can enhance this function for handling ";" in PACKAGES_PATH ?
     I would need some time to work on it. I would really appreciate if anybody can help.

Why is this function implemented in build wrapper?
-> The purpose of this patch is to provide a user method to determine which source code was booted?
     Or if a UEFI driver/application has been loaded in system from shell, which was the commit version for that?
     This helps debugging in case we are provided logs of running system or logs of driver/application. 
     Since a user can make a standalone driver/application using build command, I added this function in build wrapper.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmywiki.wooledge.org%2FBashFAQ%2F028&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7C19b30bd4b7a842b5bcab08d794eae947%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637141608975487253&amp;sdata=qq%2FYPydngaQUjvinGG5I46VRT4EMXH3j4tcIntBCBGI%3D&amp;reserved=0 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-09 10:10       ` Pankaj Bansal
@ 2020-01-10  6:27         ` Bob Feng
  2020-01-10  8:40           ` Pankaj Bansal
  0 siblings, 1 reply; 7+ messages in thread
From: Bob Feng @ 2020-01-10  6:27 UTC (permalink / raw)
  To: Pankaj Bansal, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Pankaj,

With your example in previous mail, I understand the usage model. Thanks.
What about adding a scripts in BaseTools/Scripts to implementing this function? I'd expect the build wrapper only have the simple logic of calling build.py.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com] 
Sent: Thursday, January 9, 2020 6:10 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

This OS environment variable is only defined up until the point build command is running.
As soon as build command finishes, these OS environment variables also vanish.

Regards,
Pankaj Bansal

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 3:31 PM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I think if the firmware module need to show git version information, the version information need to build into firmware binary but not set to OS environment variable.

Thanks
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Thursday, January 9, 2020 3:09 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

Thanks for replying.
Please see inline

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 11:43 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
-> Any driver or library can use this. I had tested this patch in Qemu after adding these changes :

index 0a1469550d..8e318f776f 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -18,6 +18,9 @@
 #include <Guid/EarlyPL011BaseAddress.h>  #include <Guid/FdtHob.h>

+#define XPRINT(x) PRINT(x)^M
+#define PRINT(x) #x^M
+^M
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -98,6 +101,8 @@ PlatformPeim (
   }

   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  DEBUG ((EFI_D_ERROR, "Edk2 version is %a\n", 
+ XPRINT(WORKSPACE_GIT_VERSION)));^M
+  DEBUG ((EFI_D_ERROR, "Edk2 platforms version is %a\n", 
+ XPRINT(PACKAGES_PATH_GIT_VERSION)));^M

   return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac2..992e89b210 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -15,6 +15,10 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib

+[BuildOptions]^M
+  GCC:*_*_*_CC_FLAGS =
+-DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"^M
+  GCC:*_*_*_CC_FLAGS =
+-DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"^M
+^M
 [Sources]
   PlatformPeiLib.c
 

PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
-> I did not know this information, maybe we can enhance this function for handling ";" in PACKAGES_PATH ?
     I would need some time to work on it. I would really appreciate if anybody can help.

Why is this function implemented in build wrapper?
-> The purpose of this patch is to provide a user method to determine which source code was booted?
     Or if a UEFI driver/application has been loaded in system from shell, which was the commit version for that?
     This helps debugging in case we are provided logs of running system or logs of driver/application. 
     Since a user can make a standalone driver/application using build command, I added this function in build wrapper.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmywiki.wooledge.org%2FBashFAQ%2F028&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7C19b30bd4b7a842b5bcab08d794eae947%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637141608975487253&amp;sdata=qq%2FYPydngaQUjvinGG5I46VRT4EMXH3j4tcIntBCBGI%3D&amp;reserved=0 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools: Retrieve git version info
  2020-01-10  6:27         ` Bob Feng
@ 2020-01-10  8:40           ` Pankaj Bansal
  0 siblings, 0 replies; 7+ messages in thread
From: Pankaj Bansal @ 2020-01-10  8:40 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Bob,

You are right. Ideal place for retrieving the git tag/commit info should be a python script.
That way it can be used in all build environments (linux/windows).
Unfortunately I don't have much experience of python programming, which is why I made changes in shell script.
I have made these changes in such a way so as to not impact any exiting functionality and to not cause any build issues.
I thought that applying these changes right now doesn't have any harm.
When someday such changes are put in python script, we can remove these changes from shell script.

Regards,
Pankaj Bansal

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Friday, January 10, 2020 11:57 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

With your example in previous mail, I understand the usage model. Thanks.
What about adding a scripts in BaseTools/Scripts to implementing this function? I'd expect the build wrapper only have the simple logic of calling build.py.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Thursday, January 9, 2020 6:10 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

This OS environment variable is only defined up until the point build command is running.
As soon as build command finishes, these OS environment variables also vanish.

Regards,
Pankaj Bansal

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 3:31 PM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I think if the firmware module need to show git version information, the version information need to build into firmware binary but not set to OS environment variable.

Thanks
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Thursday, January 9, 2020 3:09 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Bob,

Thanks for replying.
Please see inline

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com>
Sent: Thursday, January 9, 2020 11:43 AM
To: Pankaj Bansal <pankaj.bansal@nxp.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools: Retrieve git version info

Hi Pankaj,

I would have some questions.  Which module or tool will read WORKSPACE_GIT_VERSION and PACKAGES_PATH_GIT_VERSION?
-> Any driver or library can use this. I had tested this patch in Qemu after adding these changes :

index 0a1469550d..8e318f776f 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -18,6 +18,9 @@
 #include <Guid/EarlyPL011BaseAddress.h>  #include <Guid/FdtHob.h>

+#define XPRINT(x) PRINT(x)^M
+#define PRINT(x) #x^M
+^M
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -98,6 +101,8 @@ PlatformPeim (
   }

   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  DEBUG ((EFI_D_ERROR, "Edk2 version is %a\n", 
+ XPRINT(WORKSPACE_GIT_VERSION)));^M
+  DEBUG ((EFI_D_ERROR, "Edk2 platforms version is %a\n", 
+ XPRINT(PACKAGES_PATH_GIT_VERSION)));^M

   return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac2..992e89b210 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -15,6 +15,10 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib

+[BuildOptions]^M
+  GCC:*_*_*_CC_FLAGS =
+-DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"^M
+  GCC:*_*_*_CC_FLAGS =
+-DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"^M
+^M
 [Sources]
   PlatformPeiLib.c
 

PACKAGES_PATH can include multiple path separated by ";",  looks this patch can't handle such case.
-> I did not know this information, maybe we can enhance this function for handling ";" in PACKAGES_PATH ?
     I would need some time to work on it. I would really appreciate if anybody can help.

Why is this function implemented in build wrapper?
-> The purpose of this patch is to provide a user method to determine which source code was booted?
     Or if a UEFI driver/application has been loaded in system from shell, which was the commit version for that?
     This helps debugging in case we are provided logs of running system or logs of driver/application. 
     Since a user can make a standalone driver/application using build command, I added this function in build wrapper.

Thanks,
Bob

-----Original Message-----
From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
Sent: Sunday, January 5, 2020 8:41 PM
To: devel@edk2.groups.io
Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] BaseTools: Retrieve git version info

Retrieve git version info and save as environment variable These variables can be used in modules to print the vesrion info when uefi boots.
This helps in identifying the codebase from logs.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    When i ran PatchCheck.py script on this patch i received two errors:
    1. Line ending ('\n') is not CRLF
    2. The commit message format is not valid:
     * Contributed-under! (Note: this must be removed by the code contributor!)
    
    I have fixed the [2] but i have not fixed [1], as this file's line endings
    are already unix like. Please suggest if i need to change these to windows
    like?

 BaseTools/BinWrappers/PosixLike/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/build b/BaseTools/BinWrappers/PosixLike/build
index f3770eed42..f32796db5d 100755
--- a/BaseTools/BinWrappers/PosixLike/build
+++ b/BaseTools/BinWrappers/PosixLike/build
@@ -10,5 +10,23 @@ full_cmd=${BASH_SOURCE:-$0} # see https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmywiki.wooledge.org%2FBashFAQ%2F028&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7C182153bf4eee4c093bb608d795962069%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637142344334904194&amp;sdata=llmV5loUBDj%2Fzj71lYTUcGfITbGWe9oKh3Ks%2FXaebcA%3D&amp;reserved=0 for a d  dir=$(dirname "$full_cmd")  cmd=${full_cmd##*/}
 
+git_version()
+{
+  command -v git>/dev/null 2>&1
+  if [ $? -eq 0 ] && [ -n "$1" ]
+  then
+    head_or_tag=`git -C $1 describe --always 2>/dev/null`
+    printf $head_or_tag
+    git -C $1 diff-index --ignore-submodules --exit-code HEAD>/dev/null
+    if [ $? -eq 1 ]; then
+      printf '%s' -dirty
+    fi
+  else
+    printf "unknown"
+  fi
+}
+
+export WORKSPACE_GIT_VERSION=$(git_version $WORKSPACE) export 
+PACKAGES_PATH_GIT_VERSION=$(git_version $PACKAGES_PATH)
 export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
 exec "${python_exe:-python}" "$dir/../../Source/Python/$cmd/$cmd.py" "$@"
--
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-10  8:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-05 12:41 [PATCH] BaseTools: Retrieve git version info Pankaj Bansal
2020-01-09  6:13 ` Bob Feng
2020-01-09  7:09   ` Pankaj Bansal
2020-01-09 10:01     ` Bob Feng
2020-01-09 10:10       ` Pankaj Bansal
2020-01-10  6:27         ` Bob Feng
2020-01-10  8:40           ` Pankaj Bansal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox