kernel: Remove unnecessary cd and rm
PATCH 1/1 kernel: Remove unnecessary cd and rm
--- meta/classes/kernel.bbclass | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
meta/classes/kernel.bbclass +2 -5
@@ -378,13 +378,10 @@ kernel_do_deploy() { ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} fi - cd ${DEPLOYDIR} - rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGETYPE} + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGE_SYMLINK_NAME}.bin + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGETYPE} cp ${COREBASE}/meta/files/deploydir_readme.txt ${DEPLOYDIR}/README_-_DO_NOT_DELETE_FILES_IN_THIS_DIRECTORY.txt - cd - } do_deploy[dirs] = "${DEPLOYDIR} ${B}" do_deploy[prefuncs] += "package_get_auto_pr"
Signature · 1 line
1.8.2.1
Please include a a comment. The burden is on the submitter to prove the cd and the rm are unnecessary, not the reviewer.
Quoted 9 lines · Martin Jansa
--- meta/classes/kernel.bbclass | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)meta/classes/kernel.bbclass +0
-0
meta/classes/kernel.bbclass kernel_do_deploy() { · line 382
Quoted 9 lines
meta/classes/kernel.bbclass +2
-4@@ -378,13 +378,10 @@ kernel_do_deploy() { ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} fi - cd ${DEPLOYDIR} - rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGETYPE} + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGE_SYMLINK_NAME}.bin + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGETYPE}
So we are now in a different directory (do we know which one?) but the source of the ln commands doesn't change. If we are in the same directory, then why add DEPLOYDIR to the destination?
--
Darren
Quoted 7 lines
meta/classes/kernel.bbclass +0
-1cp ${COREBASE}/meta/files/deploydir_readme.txt ${DEPLOYDIR}/README_-_DO_NOT_DELETE_FILES_IN_THIS_DIRECTORY.txt - cd - } do_deploy[dirs] = "${DEPLOYDIR} ${B}" do_deploy[prefuncs] += "package_get_auto_pr"
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Technical Lead - Linux Kernel
Quoted 17 lines · Darren Hart
Please include a a comment. The burden is on the submitter to prove the cd and the rm are unnecessary, not the reviewer.
Quoted 12 lines · Martin Jansa
--- meta/classes/kernel.bbclass | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)meta/classes/kernel.bbclass +0
-0@@ -378,13 +378,10 @@ kernel_do_deploy() { ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} fi
Quoted 10 lines
meta/classes/kernel.bbclass kernel_do_deploy() { · line 382
Quoted 6 lines
meta/classes/kernel.bbclass +2
-4- cd ${DEPLOYDIR} - rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGETYPE} + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGE_SYMLINK_NAME}.bin + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGETYPE}So we are now in a different directory (do we know which one?) but the source of the ln commands doesn't change. If we are in the same directory, then why add DEPLOYDIR to the destination?
I suspect the answer is absolute path, but this is a good example of where a proper comment is useful.
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Technical Lead - Linux Kernel
Quoted 17 lines · Darren Hart
Please include a a comment. The burden is on the submitter to prove the cd and the rm are unnecessary, not the reviewer.
Quoted 12 lines · Martin Jansa
--- meta/classes/kernel.bbclass | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)meta/classes/kernel.bbclass +0
-0@@ -378,13 +378,10 @@ kernel_do_deploy() { ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} fi
Quoted 10 lines
meta/classes/kernel.bbclass kernel_do_deploy() { · line 382
Quoted 6 lines
meta/classes/kernel.bbclass +2
-4- cd ${DEPLOYDIR} - rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGETYPE} + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGE_SYMLINK_NAME}.bin + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGETYPE}So we are now in a different directory (do we know which one?) but the source of the ln commands doesn't change. If we are in the same directory, then why add DEPLOYDIR to the destination?
yes we know which one - ${B} as defined in deploy.bbclass:
do_deploy[dirs] = "${DEPLOYDIR} ${B}", and the same is defined again (maybe unnecessary) in kernel.bbclass
DEPLOYDIR is defined in deploy.bbclass as well
DEPLOYDIR = "${WORKDIR}/deploy-${PN}"
DEPLOYDIR is used as target for every other file deployed by do_deploy, this was just unnecessary switching CWD to create relative symlink and I have no idea why rm -f was used together with ln -sf.
I'll send v2, because
ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} is wrong.
Signature · 1 line
Martin 'JaMa' Jansa jabber: [email protected]
PATCH 1/1 kernel: Remove unnecessary cd and rm, fix MODULE_TARBALL_SYMLINK_NAME creation
* DEPLOYDIR is used as target for every other file deployed by do_deploy, this was just unnecessary switching CWD to create relative symlink and I have no idea why rm -f was used together with ln -sf.
--- meta/classes/kernel.bbclass | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
meta/classes/kernel.bbclass +3 -6
@@ -375,16 +375,13 @@ kernel_do_deploy() { if [ ${MODULE_TARBALL_DEPLOY} = "1" ] && (grep -q -i -e '^CONFIG_MODULES=y$' .config); then mkdir -p ${D}/lib tar -cvzf ${DEPLOYDIR}/${MODULE_TARBALL_BASE_NAME} -C ${D} lib - ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${MODULE_TARBALL_SYMLINK_NAME} + ln -sf ${MODULE_TARBALL_BASE_NAME}.bin ${DEPLOYDIR}/${MODULE_TARBALL_SYMLINK_NAME} fi - cd ${DEPLOYDIR} - rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin - ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGETYPE} + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGE_SYMLINK_NAME}.bin + ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${DEPLOYDIR}/${KERNEL_IMAGETYPE} cp ${COREBASE}/meta/files/deploydir_readme.txt ${DEPLOYDIR}/README_-_DO_NOT_DELETE_FILES_IN_THIS_DIRECTORY.txt - cd - } do_deploy[dirs] = "${DEPLOYDIR} ${B}" do_deploy[prefuncs] += "package_get_auto_pr"
Signature · 1 line
1.8.2.1