* [PATCH V2 1/2] base-files: move shells generating process to pkg_postinst
@ 2013-10-17 10:03 Ming Liu
2013-10-17 10:03 ` [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells Ming Liu
0 siblings, 1 reply; 8+ messages in thread
From: Ming Liu @ 2013-10-17 10:03 UTC (permalink / raw)
To: openembedded-core
The "shells" file lists several valid login shells, of which some might be
not present in a particular rootfs, this is unreasonable.
Fix it by moving "shells" file generating process to pkg_postinst, for other
shells must do the same thing to register themselves to this file.
Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
meta/recipes-core/base-files/base-files/shells | 8 --------
meta/recipes-core/base-files/base-files_3.0.14.bb | 8 ++++++--
2 files changed, 6 insertions(+), 10 deletions(-)
delete mode 100644 meta/recipes-core/base-files/base-files/shells
diff --git a/meta/recipes-core/base-files/base-files/shells b/meta/recipes-core/base-files/base-files/shells
deleted file mode 100644
index ce39b3d..0000000
--- a/meta/recipes-core/base-files/base-files/shells
+++ /dev/null
@@ -1,8 +0,0 @@
-# /etc/shells: valid login shells
-/bin/sh
-/bin/ash
-/bin/bash
-/bin/dash
-/bin/ksh
-/usr/bin/ksh
-/usr/bin/screen
diff --git a/meta/recipes-core/base-files/base-files_3.0.14.bb b/meta/recipes-core/base-files/base-files_3.0.14.bb
index 054fefa..a169117 100644
--- a/meta/recipes-core/base-files/base-files_3.0.14.bb
+++ b/meta/recipes-core/base-files/base-files_3.0.14.bb
@@ -16,7 +16,6 @@ SRC_URI = "file://rotation \
file://inputrc \
file://host.conf \
file://profile \
- file://shells \
file://fstab \
file://filesystems \
file://issue.net \
@@ -92,7 +91,6 @@ do_install () {
install -m 0644 ${WORKDIR}/usbd ${D}${sysconfdir}/default/usbd
sed -i "s#ROOTHOME#${ROOT_HOME}#" ${WORKDIR}/profile
install -m 0644 ${WORKDIR}/profile ${D}${sysconfdir}/profile
- install -m 0644 ${WORKDIR}/shells ${D}${sysconfdir}/shells
install -m 0755 ${WORKDIR}/share/dot.profile ${D}${sysconfdir}/skel/.profile
install -m 0755 ${WORKDIR}/share/dot.bashrc ${D}${sysconfdir}/skel/.bashrc
install -m 0644 ${WORKDIR}/inputrc ${D}${sysconfdir}/inputrc
@@ -137,6 +135,12 @@ do_install_append_linuxstdbase() {
done
}
+pkg_postinst_${PN} () {
+ touch $D${sysconfdir}/shells
+ grep -q "^/bin/sh$" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
+}
+
+
PACKAGES = "${PN}-doc ${PN} ${PN}-dev ${PN}-dbg"
FILES_${PN} = "/"
FILES_${PN}-doc = "${docdir} ${datadir}/common-licenses"
--
1.8.3.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:03 [PATCH V2 1/2] base-files: move shells generating process to pkg_postinst Ming Liu
@ 2013-10-17 10:03 ` Ming Liu
2013-10-17 10:11 ` Phil Blundell
2013-10-17 10:33 ` Martin Jansa
0 siblings, 2 replies; 8+ messages in thread
From: Ming Liu @ 2013-10-17 10:03 UTC (permalink / raw)
To: openembedded-core
Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
meta/recipes-extended/screen/screen_4.0.3.bb | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/meta/recipes-extended/screen/screen_4.0.3.bb b/meta/recipes-extended/screen/screen_4.0.3.bb
index 1a7eb20..81e418b 100644
--- a/meta/recipes-extended/screen/screen_4.0.3.bb
+++ b/meta/recipes-extended/screen/screen_4.0.3.bb
@@ -41,3 +41,8 @@ do_install_append () {
fi
done
}
+
+pkg_postinst_${PN} () {
+ touch $D${sysconfdir}/shells
+ grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
+}
--
1.8.3.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:03 ` [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells Ming Liu
@ 2013-10-17 10:11 ` Phil Blundell
2013-10-17 10:28 ` Ming Liu
2013-10-17 10:33 ` Martin Jansa
1 sibling, 1 reply; 8+ messages in thread
From: Phil Blundell @ 2013-10-17 10:11 UTC (permalink / raw)
To: Ming Liu; +Cc: openembedded-core
On Thu, 2013-10-17 at 18:03 +0800, Ming Liu wrote:
> +pkg_postinst_${PN} () {
> + touch $D${sysconfdir}/shells
> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
> +}
Is that "touch" really desirable? It seems as though screen ought not
really to be creating /etc/shells if it isn't there already, and
probably ought not to be updating the timestamp if it isn't actually
changing the file either.
p.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:11 ` Phil Blundell
@ 2013-10-17 10:28 ` Ming Liu
2013-10-17 10:53 ` Phil Blundell
0 siblings, 1 reply; 8+ messages in thread
From: Ming Liu @ 2013-10-17 10:28 UTC (permalink / raw)
To: Phil Blundell; +Cc: openembedded-core
On 10/17/2013 06:11 PM, Phil Blundell wrote:
> On Thu, 2013-10-17 at 18:03 +0800, Ming Liu wrote:
>> +pkg_postinst_${PN} () {
>> + touch $D${sysconfdir}/shells
>> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
>> +}
> Is that "touch" really desirable? It seems as though screen ought not
> really to be creating /etc/shells if it isn't there already,
No, it's supposed to be created if /etc/shells doesn't exist already,
otherwise, how could we echo text to it?
> and
> probably ought not to be updating the timestamp if it isn't actually
> changing the file either.
Yes, it may happen that the timestamp be updated even the it isn't
actually changing the file, but only for upgrade case, and I think it's
not a big deal. Actually I copied this piece of script from bash recipe,
it's doing the same thing there.
//Ming Liu
>
> p.
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:28 ` Ming Liu
@ 2013-10-17 10:53 ` Phil Blundell
2013-10-17 11:06 ` Ming Liu
0 siblings, 1 reply; 8+ messages in thread
From: Phil Blundell @ 2013-10-17 10:53 UTC (permalink / raw)
To: Ming Liu; +Cc: openembedded-core
On Thu, 2013-10-17 at 18:28 +0800, Ming Liu wrote:
> On 10/17/2013 06:11 PM, Phil Blundell wrote:
> > On Thu, 2013-10-17 at 18:03 +0800, Ming Liu wrote:
> >> +pkg_postinst_${PN} () {
> >> + touch $D${sysconfdir}/shells
> >> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
> >> +}
> > Is that "touch" really desirable? It seems as though screen ought not
> > really to be creating /etc/shells if it isn't there already,
> No, it's supposed to be created if /etc/shells doesn't exist already,
> otherwise, how could we echo text to it?
I guess there are two things to say about this:
1) not everybody wants /etc/shells, and having screen (or any other
recipe) create that file unilaterally seems like a bad thing.
Especially since there doesn't seem to be any corresponding postrm to
clear it up if screen is then removed again, nor is /etc/shells
mentioned in CONFFILES.
2) even if you did want to create /etc/shells, the touch is unnecessary
because "echo foo >> FILE" is perfectly capable of creating the FILE if
it didn't already exist.
p.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:53 ` Phil Blundell
@ 2013-10-17 11:06 ` Ming Liu
0 siblings, 0 replies; 8+ messages in thread
From: Ming Liu @ 2013-10-17 11:06 UTC (permalink / raw)
To: Phil Blundell, Martin Jansa; +Cc: openembedded-core
On 10/17/2013 06:53 PM, Phil Blundell wrote:
> On Thu, 2013-10-17 at 18:28 +0800, Ming Liu wrote:
>> On 10/17/2013 06:11 PM, Phil Blundell wrote:
>>> On Thu, 2013-10-17 at 18:03 +0800, Ming Liu wrote:
>>>> +pkg_postinst_${PN} () {
>>>> + touch $D${sysconfdir}/shells
>>>> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
>>>> +}
>>> Is that "touch" really desirable? It seems as though screen ought not
>>> really to be creating /etc/shells if it isn't there already,
>> No, it's supposed to be created if /etc/shells doesn't exist already,
>> otherwise, how could we echo text to it?
> I guess there are two things to say about this:
>
> 1) not everybody wants /etc/shells, and having screen (or any other
> recipe) create that file unilaterally seems like a bad thing.
> Especially since there doesn't seem to be any corresponding postrm to
> clear it up if screen is then removed again, nor is /etc/shells
> mentioned in CONFFILES.
>
> 2) even if you did want to create /etc/shells, the touch is unnecessary
> because "echo foo >> FILE" is perfectly capable of creating the FILE if
> it didn't already exist.
Yes, you are correct for both of the two points, I will send the V3,
includes the changes:
1 add postrm script, removing the entry in /etc/shells.
2 rm the unnecessary 'touch'.
3 change the regex for screen and bash recipes.(As Martin pointed out)
Anything else?
//Ming Liu
>
> p.
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:03 ` [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells Ming Liu
2013-10-17 10:11 ` Phil Blundell
@ 2013-10-17 10:33 ` Martin Jansa
2013-10-17 10:44 ` Ming Liu
1 sibling, 1 reply; 8+ messages in thread
From: Martin Jansa @ 2013-10-17 10:33 UTC (permalink / raw)
To: Ming Liu; +Cc: openembedded-core
[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]
On Thu, Oct 17, 2013 at 06:03:12PM +0800, Ming Liu wrote:
> Signed-off-by: Ming Liu <ming.liu@windriver.com>
> ---
> meta/recipes-extended/screen/screen_4.0.3.bb | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/meta/recipes-extended/screen/screen_4.0.3.bb b/meta/recipes-extended/screen/screen_4.0.3.bb
> index 1a7eb20..81e418b 100644
> --- a/meta/recipes-extended/screen/screen_4.0.3.bb
> +++ b/meta/recipes-extended/screen/screen_4.0.3.bb
> @@ -41,3 +41,8 @@ do_install_append () {
> fi
> done
> }
> +
> +pkg_postinst_${PN} () {
> + touch $D${sysconfdir}/shells
> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
^ the same, we don't want to match /usr/bin/screenie
if it's there and "screen" isn't
Is there patch to add this postinst to bash and busybox?
> +}
> --
> 1.8.3.3
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells
2013-10-17 10:33 ` Martin Jansa
@ 2013-10-17 10:44 ` Ming Liu
0 siblings, 0 replies; 8+ messages in thread
From: Ming Liu @ 2013-10-17 10:44 UTC (permalink / raw)
To: Martin Jansa; +Cc: openembedded-core
[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]
On 10/17/2013 06:33 PM, Martin Jansa wrote:
> On Thu, Oct 17, 2013 at 06:03:12PM +0800, Ming Liu wrote:
>> Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> ---
>> meta/recipes-extended/screen/screen_4.0.3.bb | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/meta/recipes-extended/screen/screen_4.0.3.bb b/meta/recipes-extended/screen/screen_4.0.3.bb
>> index 1a7eb20..81e418b 100644
>> --- a/meta/recipes-extended/screen/screen_4.0.3.bb
>> +++ b/meta/recipes-extended/screen/screen_4.0.3.bb
>> @@ -41,3 +41,8 @@ do_install_append () {
>> fi
>> done
>> }
>> +
>> +pkg_postinst_${PN} () {
>> + touch $D${sysconfdir}/shells
>> + grep -q "${bindir}/screen" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
> ^ the same, we don't want to match /usr/bin/screenie
> if it's there and "screen" isn't
Yes, you are correct, I am too less comprehensive.
>
> Is there patch to add this postinst to bash and busybox?
For bash, it already has it, as following:
------------------
pkg_postinst_${PN} () {
touch $D${sysconfdir}/shells
grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >>
$D${sysconfdir}/shells
grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >>
$D${sysconfdir}/shells
}
-----------------
But as you pointed out, it needs to be more stringent too.
For busybox, I don't think it need this script for it provides the
symbol link: /bin/sh.
//Ming Liu
>
>> +}
>> --
>> 1.8.3.3
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
[-- Attachment #2: Type: text/html, Size: 3108 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-17 11:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17 10:03 [PATCH V2 1/2] base-files: move shells generating process to pkg_postinst Ming Liu
2013-10-17 10:03 ` [PATCH V2 2/2] screen: add pkg_postinst to register entry to /etc/shells Ming Liu
2013-10-17 10:11 ` Phil Blundell
2013-10-17 10:28 ` Ming Liu
2013-10-17 10:53 ` Phil Blundell
2013-10-17 11:06 ` Ming Liu
2013-10-17 10:33 ` Martin Jansa
2013-10-17 10:44 ` Ming Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox