* [PATCH] kernel: Remove unnecessary cd and rm
@ 2013-05-21 15:39 Martin Jansa
2013-05-21 21:26 ` Darren Hart
0 siblings, 1 reply; 5+ messages in thread
From: Martin Jansa @ 2013-05-21 15:39 UTC (permalink / raw)
To: openembedded-core
---
meta/classes/kernel.bbclass | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 3320747..47a96c5 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -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"
--
1.8.2.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: Remove unnecessary cd and rm
2013-05-21 15:39 [PATCH] kernel: Remove unnecessary cd and rm Martin Jansa
@ 2013-05-21 21:26 ` Darren Hart
2013-05-21 21:41 ` Darren Hart
2013-05-21 22:16 ` Martin Jansa
0 siblings, 2 replies; 5+ messages in thread
From: Darren Hart @ 2013-05-21 21:26 UTC (permalink / raw)
To: Martin Jansa; +Cc: openembedded-core
Please include a a comment. The burden is on the submitter to prove
the cd and the rm are unnecessary, not the reviewer.
On 05/21/2013 08:39 AM, Martin Jansa wrote:
> ---
> meta/classes/kernel.bbclass | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 3320747..47a96c5 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -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
>
> 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"
>
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: Remove unnecessary cd and rm
2013-05-21 21:26 ` Darren Hart
@ 2013-05-21 21:41 ` Darren Hart
2013-05-21 22:16 ` Martin Jansa
1 sibling, 0 replies; 5+ messages in thread
From: Darren Hart @ 2013-05-21 21:41 UTC (permalink / raw)
To: Martin Jansa; +Cc: openembedded-core
On 05/21/2013 02:26 PM, Darren Hart wrote:
> Please include a a comment. The burden is on the submitter to prove
> the cd and the rm are unnecessary, not the reviewer.
>
>
> On 05/21/2013 08:39 AM, Martin Jansa wrote:
>> ---
>> meta/classes/kernel.bbclass | 7 ++-----
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index 3320747..47a96c5 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -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?
I suspect the answer is absolute path, but this is a good example of where
a proper comment is useful.
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: Remove unnecessary cd and rm
2013-05-21 21:26 ` Darren Hart
2013-05-21 21:41 ` Darren Hart
@ 2013-05-21 22:16 ` Martin Jansa
2013-05-21 22:18 ` [PATCH] kernel: Remove unnecessary cd and rm, fix MODULE_TARBALL_SYMLINK_NAME creation Martin Jansa
1 sibling, 1 reply; 5+ messages in thread
From: Martin Jansa @ 2013-05-21 22:16 UTC (permalink / raw)
To: Darren Hart; +Cc: openembedded-core
[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]
On Tue, May 21, 2013 at 02:26:55PM -0700, Darren Hart wrote:
> Please include a a comment. The burden is on the submitter to prove
> the cd and the rm are unnecessary, not the reviewer.
>
>
> On 05/21/2013 08:39 AM, Martin Jansa wrote:
> > ---
> > meta/classes/kernel.bbclass | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> > index 3320747..47a96c5 100644
> > --- a/meta/classes/kernel.bbclass
> > +++ b/meta/classes/kernel.bbclass
> > @@ -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?
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.
See related
http://git.openembedded.org/openembedded-core/commit/meta/classes/kernel.bbclass?id=fd5d80ab1a405cb4ef94f2cde25f8c251da490f0
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] kernel: Remove unnecessary cd and rm, fix MODULE_TARBALL_SYMLINK_NAME creation
2013-05-21 22:16 ` Martin Jansa
@ 2013-05-21 22:18 ` Martin Jansa
0 siblings, 0 replies; 5+ messages in thread
From: Martin Jansa @ 2013-05-21 22:18 UTC (permalink / raw)
To: openembedded-core
* 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.
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
meta/classes/kernel.bbclass | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 3320747..5a4f4ea 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -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"
--
1.8.2.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-05-21 22:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-21 15:39 [PATCH] kernel: Remove unnecessary cd and rm Martin Jansa
2013-05-21 21:26 ` Darren Hart
2013-05-21 21:41 ` Darren Hart
2013-05-21 22:16 ` Martin Jansa
2013-05-21 22:18 ` [PATCH] kernel: Remove unnecessary cd and rm, fix MODULE_TARBALL_SYMLINK_NAME creation Martin Jansa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox