* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.