| Message ID | 20220419154734.6346-1-devarsht@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | [meta-ti,dunfell] u-boot-ti: Use SRCREV to get short commit ID | expand |
On 21:17-20220419, Devarsh Thakkar wrote: > Due to recent security update in git, we are > not able to fetch revision currently using existing method: > https://github.blog/2022-04-12-git-security-vulnerability-announced/ > > So instead, use the SRCREV to parse the short commit ID > and set the UBOOT_LOCALVERSION variable. > > Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > --- > recipes-bsp/u-boot/u-boot-ti.inc | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/recipes-bsp/u-boot/u-boot-ti.inc b/recipes-bsp/u-boot/u-boot-ti.inc > index 231b7647..cc775e2e 100644 > --- a/recipes-bsp/u-boot/u-boot-ti.inc > +++ b/recipes-bsp/u-boot/u-boot-ti.inc > @@ -1,14 +1,6 @@ > # UBOOT_LOCALVERSION can be set to add a tag to the end of the > # U-boot version string. such as the commit id > -def get_git_revision(p): > - import subprocess > - > - try: > - return subprocess.Popen("git rev-parse HEAD 2>/dev/null ", cwd=p, shell=True, stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].rstrip() I see a similar logic in recipes-kernel/linux/setup-defconfig.inc as well. Considering similar problem https://lore.kernel.org/all/20220413155249.3458236-2-raj.khem@gmail.com/ was wondering as to what might be a better way to solve this? There is also git rev-parse HEAD instances in oe-core as well and bitbake(lib/layerindexlib/cooker.py) as well. I wonder since we know cwd=p, could we use that to set https://git-scm.com/docs/git/2.35.2#Documentation/git.txt-codeGITCEILINGDIRECTORIEScode (which if my understanding is right, came in around v1.5.5.1-319-g0454dd93bfb2) OR maybe just set it to the base conf similar to what was done on master oe-core/meta/conf/bitbake.conf (commit 02ecf3e2a98a614805f6f2574c2bf14162192d01 "bitbake.conf: Prevent git from detecting parent repo in recipe")? I am not sure if we should considering just side stepping this issue via just not using the git to get the version string.. just my 2 cents. > - except OSError: > - return None > - > -UBOOT_LOCALVERSION = "-g${@get_git_revision('${S}').__str__()[:10]}" > +UBOOT_LOCALVERSION = "-g${@d.getVar("SRCREV", False).__str__()[:10]}" > > UBOOT_SUFFIX ?= "img" > SPL_BINARY ?= "MLO" > -- > 2.17.1 >
On 20/04/22 05:04, Nishanth Menon wrote: > On 21:17-20220419, Devarsh Thakkar wrote: >> Due to recent security update in git, we are >> not able to fetch revision currently using existing method: >> https://github.blog/2022-04-12-git-security-vulnerability-announced/ >> >> So instead, use the SRCREV to parse the short commit ID >> and set the UBOOT_LOCALVERSION variable. >> >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> >> --- >> recipes-bsp/u-boot/u-boot-ti.inc | 10 +--------- >> 1 file changed, 1 insertion(+), 9 deletions(-) >> >> diff --git a/recipes-bsp/u-boot/u-boot-ti.inc b/recipes-bsp/u-boot/u-boot-ti.inc >> index 231b7647..cc775e2e 100644 >> --- a/recipes-bsp/u-boot/u-boot-ti.inc >> +++ b/recipes-bsp/u-boot/u-boot-ti.inc >> @@ -1,14 +1,6 @@ >> # UBOOT_LOCALVERSION can be set to add a tag to the end of the >> # U-boot version string. such as the commit id >> -def get_git_revision(p): >> - import subprocess >> - >> - try: >> - return subprocess.Popen("git rev-parse HEAD 2>/dev/null ", cwd=p, shell=True, stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].rstrip() > > I see a similar logic in > recipes-kernel/linux/setup-defconfig.inc as well. > > Considering similar problem > > https://lore.kernel.org/all/20220413155249.3458236-2-raj.khem@gmail.com/ > > was wondering as to what might be a better way to solve this? > > There is also git rev-parse HEAD instances in oe-core as well and > bitbake(lib/layerindexlib/cooker.py) as well. > > I wonder since we know cwd=p, could we use that to set > https://git-scm.com/docs/git/2.35.2#Documentation/git.txt-codeGITCEILINGDIRECTORIEScode > (which if my understanding is right, came in around v1.5.5.1-319-g0454dd93bfb2) > > OR maybe just set it to the base conf similar to what was done on > master oe-core/meta/conf/bitbake.conf (commit > 02ecf3e2a98a614805f6f2574c2bf14162192d01 "bitbake.conf: Prevent git from > detecting parent repo in recipe")? > > I am not sure if we should considering just side stepping this issue via > just not using the git to get the version string.. just my 2 cents. My top level understanding was the security update was suggesting to avoid doing what we were doing already i.e. calling git from a sub-process through a recipe due to security concerns and so avoided using git and also I think below change also achieves same what was achieved before with SRCREV, I have similar fix on the kernel bb too which was failing with same error. >> - except OSError: >> - return None >> - >> -UBOOT_LOCALVERSION = "-g${@get_git_revision('${S}').__str__()[:10]}" >> +UBOOT_LOCALVERSION = "-g${@d.getVar("SRCREV", False).__str__()[:10]}" >> >> UBOOT_SUFFIX ?= "img" >> SPL_BINARY ?= "MLO" >> -- >> 2.17.1 >>
On Wed, Apr 27, 2022 at 07:50:13PM +0530, Devarsh Thakkar via lists.yoctoproject.org wrote: > > On 20/04/22 05:04, Nishanth Menon wrote: > > On 21:17-20220419, Devarsh Thakkar wrote: > >> Due to recent security update in git, we are > >> not able to fetch revision currently using existing method: > >> https://github.blog/2022-04-12-git-security-vulnerability-announced/ > >> > >> So instead, use the SRCREV to parse the short commit ID > >> and set the UBOOT_LOCALVERSION variable. > >> > >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > >> --- > >> recipes-bsp/u-boot/u-boot-ti.inc | 10 +--------- > >> 1 file changed, 1 insertion(+), 9 deletions(-) > >> > >> diff --git a/recipes-bsp/u-boot/u-boot-ti.inc b/recipes-bsp/u-boot/u-boot-ti.inc > >> index 231b7647..cc775e2e 100644 > >> --- a/recipes-bsp/u-boot/u-boot-ti.inc > >> +++ b/recipes-bsp/u-boot/u-boot-ti.inc > >> @@ -1,14 +1,6 @@ > >> # UBOOT_LOCALVERSION can be set to add a tag to the end of the > >> # U-boot version string. such as the commit id > >> -def get_git_revision(p): > >> - import subprocess > >> - > >> - try: > >> - return subprocess.Popen("git rev-parse HEAD 2>/dev/null ", cwd=p, shell=True, stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].rstrip() > > > > I see a similar logic in > > recipes-kernel/linux/setup-defconfig.inc as well. > > > > Considering similar problem > > > > https://lore.kernel.org/all/20220413155249.3458236-2-raj.khem@gmail.com/ > > > > was wondering as to what might be a better way to solve this? > > > > There is also git rev-parse HEAD instances in oe-core as well and > > bitbake(lib/layerindexlib/cooker.py) as well. > > > > I wonder since we know cwd=p, could we use that to set > > https://git-scm.com/docs/git/2.35.2#Documentation/git.txt-codeGITCEILINGDIRECTORIEScode > > (which if my understanding is right, came in around v1.5.5.1-319-g0454dd93bfb2) > > > > OR maybe just set it to the base conf similar to what was done on > > master oe-core/meta/conf/bitbake.conf (commit > > 02ecf3e2a98a614805f6f2574c2bf14162192d01 "bitbake.conf: Prevent git from > > detecting parent repo in recipe")? > > > > I am not sure if we should considering just side stepping this issue via > > just not using the git to get the version string.. just my 2 cents. > > My top level understanding was the security update was suggesting to avoid > > doing what we were doing already i.e. calling git from > > a sub-process through a recipe due to security concerns and so avoided > using git > > and also I think below change also achieves same what was achieved > before with SRCREV, I have > > similar fix on the kernel bb too which was failing with same error. > > >> - except OSError: > >> - return None > >> - > >> -UBOOT_LOCALVERSION = "-g${@get_git_revision('${S}').__str__()[:10]}" > >> +UBOOT_LOCALVERSION = "-g${@d.getVar("SRCREV", False).__str__()[:10]}" You should probably use SRCPV here to also work with AUTOREV. > >> UBOOT_SUFFIX ?= "img" > >> SPL_BINARY ?= "MLO" > >> -- > >> 2.17.1 > >>
diff --git a/recipes-bsp/u-boot/u-boot-ti.inc b/recipes-bsp/u-boot/u-boot-ti.inc index 231b7647..cc775e2e 100644 --- a/recipes-bsp/u-boot/u-boot-ti.inc +++ b/recipes-bsp/u-boot/u-boot-ti.inc @@ -1,14 +1,6 @@ # UBOOT_LOCALVERSION can be set to add a tag to the end of the # U-boot version string. such as the commit id -def get_git_revision(p): - import subprocess - - try: - return subprocess.Popen("git rev-parse HEAD 2>/dev/null ", cwd=p, shell=True, stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].rstrip() - except OSError: - return None - -UBOOT_LOCALVERSION = "-g${@get_git_revision('${S}').__str__()[:10]}" +UBOOT_LOCALVERSION = "-g${@d.getVar("SRCREV", False).__str__()[:10]}" UBOOT_SUFFIX ?= "img" SPL_BINARY ?= "MLO"
Due to recent security update in git, we are not able to fetch revision currently using existing method: https://github.blog/2022-04-12-git-security-vulnerability-announced/ So instead, use the SRCREV to parse the short commit ID and set the UBOOT_LOCALVERSION variable. Signed-off-by: Devarsh Thakkar <devarsht@ti.com> --- recipes-bsp/u-boot/u-boot-ti.inc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)