Patchwork meta/recipes-core/base-passwd/base-passwd/noshadow.patch: Split it into two parts

login
register
mail settings
Submitter Laszlo Papp
Date Feb. 5, 2014, 11:13 a.m.
Message ID <1391598835-1631-1-git-send-email-lpapp@kde.org>
Download mbox | patch
Permalink /patch/66335/
State New
Headers show

Comments

Laszlo Papp - Feb. 5, 2014, 11:13 a.m.
The current change contains two different logics resulting that users like me
may need to partially override the behavior of it. It would be easier for end
users to keep one change while dropping the other if needed, like in our case.

Signed-off-by: Laszlo Papp <lpapp@kde.org>
---
 meta/recipes-core/base-passwd/base-passwd/nobash.patch | 10 +---------
 .../base-passwd/base-passwd/noshadow.patch             | 18 ++++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb    |  1 +
 3 files changed, 20 insertions(+), 9 deletions(-)
 create mode 100644 meta/recipes-core/base-passwd/base-passwd/noshadow.patch
Saul Wold - Feb. 5, 2014, 3:49 p.m.
On 02/05/2014 03:13 AM, Laszlo Papp wrote:
> The current change contains two different logics resulting that users like me
> may need to partially override the behavior of it. It would be easier for end
> users to keep one change while dropping the other if needed, like in our case.
>

I beleive that your first version of this was merged, is this different 
that the one you send on 1/30?

It seems that the line count in this patch is different than the first, 
please rebase against master and update this patch please.

Thanks

Sau!

> Signed-off-by: Laszlo Papp <lpapp@kde.org>
> ---
>   meta/recipes-core/base-passwd/base-passwd/nobash.patch | 10 +---------
>   .../base-passwd/base-passwd/noshadow.patch             | 18 ++++++++++++++++++
>   meta/recipes-core/base-passwd/base-passwd_3.5.29.bb    |  1 +
>   3 files changed, 20 insertions(+), 9 deletions(-)
>   create mode 100644 meta/recipes-core/base-passwd/base-passwd/noshadow.patch
>
> diff --git a/meta/recipes-core/base-passwd/base-passwd/nobash.patch b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
> index 6371576..eed87db 100644
> --- a/meta/recipes-core/base-passwd/base-passwd/nobash.patch
> +++ b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
> @@ -1,14 +1,6 @@
>   use /bin/sh instead of /bin/bash, since the latter may not be included in
>   some images such as minimal
>
> -comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
> -
> -remove "*" for root since we don't have a /etc/shadow so far.
> -
> -by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
> -
> -Patch managed by http://www.mn-logistik.de/unsupported/pxa250/patcher
> -
>   Upstream-Status: Invalid [configuration]
>
>   Signed-off-by: Scott Garman <scott.a.garman@intel.com>
> @@ -17,7 +9,7 @@ Signed-off-by: Scott Garman <scott.a.garman@intel.com>
>   +++ base-passwd/passwd.master
>   @@ -1,4 +1,4 @@
>   -root:*:0:0:root:/root:/bin/bash
> -+root::0:0:root:/root:/bin/sh
> ++root:*:0:0:root:/root:/bin/sh
>    daemon:*:1:1:daemon:/usr/sbin:/bin/sh
>    bin:*:2:2:bin:/bin:/bin/sh
>    sys:*:3:3:sys:/dev:/bin/sh
> diff --git a/meta/recipes-core/base-passwd/base-passwd/noshadow.patch b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
> new file mode 100644
> index 0000000..14e3959
> --- /dev/null
> +++ b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
> @@ -0,0 +1,18 @@
> +comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
> +
> +remove "*" for root since we don't have a /etc/shadow so far.
> +
> +by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
> +
> +Upstream-Status: Invalid [configuration]
> +
> +Signed-off-by: Scott Garman <scott.a.garman@intel.com>
> +
> +--- base-passwd/passwd.master~nobash
> ++++ base-passwd/passwd.master
> +@@ -1,4 +1,4 @@
> +-root:*:0:0:root:/root:/bin/sh
> ++root::0:0:root:/root:/bin/sh
> + daemon:*:1:1:daemon:/usr/sbin:/bin/sh
> + bin:*:2:2:bin:/bin:/bin/sh
> + sys:*:3:3:sys:/dev:/bin/sh
> diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> index 082037a..5b31fce 100644
> --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
> @@ -9,6 +9,7 @@ DEPENDS = "docbook-utils-native linuxdoc-tools-native"
>   SRC_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/base-passwd_${PV}.tar.gz \
>              file://add_shutdown.patch \
>              file://nobash.patch \
> +           file://noshadow.patch \
>              file://input.patch \
>              file://disable-docs.patch \
>             "
>
Laszlo Papp - Feb. 8, 2014, 11:50 a.m.
Saul, would it be possible that you leave a short message on the
mailing list in the corresponding thread if a change is merged to
avoid such things in the future?

On Wed, Feb 5, 2014 at 3:49 PM, Saul Wold <sgw@linux.intel.com> wrote:
> On 02/05/2014 03:13 AM, Laszlo Papp wrote:
>>
>> The current change contains two different logics resulting that users like
>> me
>> may need to partially override the behavior of it. It would be easier for
>> end
>> users to keep one change while dropping the other if needed, like in our
>> case.
>>
>
> I beleive that your first version of this was merged, is this different that
> the one you send on 1/30?
>
> It seems that the line count in this patch is different than the first,
> please rebase against master and update this patch please.
>
> Thanks
>
> Sau!
>
>
>> Signed-off-by: Laszlo Papp <lpapp@kde.org>
>> ---
>>   meta/recipes-core/base-passwd/base-passwd/nobash.patch | 10 +---------
>>   .../base-passwd/base-passwd/noshadow.patch             | 18
>> ++++++++++++++++++
>>   meta/recipes-core/base-passwd/base-passwd_3.5.29.bb    |  1 +
>>   3 files changed, 20 insertions(+), 9 deletions(-)
>>   create mode 100644
>> meta/recipes-core/base-passwd/base-passwd/noshadow.patch
>>
>> diff --git a/meta/recipes-core/base-passwd/base-passwd/nobash.patch
>> b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
>> index 6371576..eed87db 100644
>> --- a/meta/recipes-core/base-passwd/base-passwd/nobash.patch
>> +++ b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
>> @@ -1,14 +1,6 @@
>>   use /bin/sh instead of /bin/bash, since the latter may not be included
>> in
>>   some images such as minimal
>>
>> -comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
>> -
>> -remove "*" for root since we don't have a /etc/shadow so far.
>> -
>> -by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
>> -
>> -Patch managed by http://www.mn-logistik.de/unsupported/pxa250/patcher
>> -
>>   Upstream-Status: Invalid [configuration]
>>
>>   Signed-off-by: Scott Garman <scott.a.garman@intel.com>
>> @@ -17,7 +9,7 @@ Signed-off-by: Scott Garman <scott.a.garman@intel.com>
>>   +++ base-passwd/passwd.master
>>   @@ -1,4 +1,4 @@
>>   -root:*:0:0:root:/root:/bin/bash
>> -+root::0:0:root:/root:/bin/sh
>> ++root:*:0:0:root:/root:/bin/sh
>>    daemon:*:1:1:daemon:/usr/sbin:/bin/sh
>>    bin:*:2:2:bin:/bin:/bin/sh
>>    sys:*:3:3:sys:/dev:/bin/sh
>> diff --git a/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
>> b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
>> new file mode 100644
>> index 0000000..14e3959
>> --- /dev/null
>> +++ b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
>> @@ -0,0 +1,18 @@
>> +comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
>> +
>> +remove "*" for root since we don't have a /etc/shadow so far.
>> +
>> +by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
>> +
>> +Upstream-Status: Invalid [configuration]
>> +
>> +Signed-off-by: Scott Garman <scott.a.garman@intel.com>
>> +
>> +--- base-passwd/passwd.master~nobash
>> ++++ base-passwd/passwd.master
>> +@@ -1,4 +1,4 @@
>> +-root:*:0:0:root:/root:/bin/sh
>> ++root::0:0:root:/root:/bin/sh
>> + daemon:*:1:1:daemon:/usr/sbin:/bin/sh
>> + bin:*:2:2:bin:/bin:/bin/sh
>> + sys:*:3:3:sys:/dev:/bin/sh
>> diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
>> b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
>> index 082037a..5b31fce 100644
>> --- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
>> +++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
>> @@ -9,6 +9,7 @@ DEPENDS = "docbook-utils-native linuxdoc-tools-native"
>>   SRC_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/base-passwd_${PV}.tar.gz
>> \
>>              file://add_shutdown.patch \
>>              file://nobash.patch \
>> +           file://noshadow.patch \
>>              file://input.patch \
>>              file://disable-docs.patch \
>>             "
>>
>
Richard Purdie - Feb. 8, 2014, 11:59 a.m.
On Sat, 2014-02-08 at 11:50 +0000, Laszlo Papp wrote:
> Saul, would it be possible that you leave a short message on the
> mailing list in the corresponding thread if a change is merged to
> avoid such things in the future?

We did used to do this but it turns out to be a nightmare for Saul and
myself (the people who usually end up dealing with the patches) to do.
Its relatively easily to update checkouts and note if a patch was
merged.

Cheers,

Richard
Laszlo Papp - Feb. 8, 2014, 12:07 p.m.
Why is it a nightmare? For instance, maintainers in other projects
leave a "Pushed, thanks" note.

See how much discussion this generates already, instead of two words.
While you are at the change to merge it, you are already in place to
leave the comment. However, contributors need to go and find the
corresponding url, etc. Please note that contributors may want to know
whether their voluntary work got merged without even a checkout, for
instance double checking from mobile, etc.

... or is the real problem the lack of maintainer man power?

On Sat, Feb 8, 2014 at 11:59 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Sat, 2014-02-08 at 11:50 +0000, Laszlo Papp wrote:
>> Saul, would it be possible that you leave a short message on the
>> mailing list in the corresponding thread if a change is merged to
>> avoid such things in the future?
>
> We did used to do this but it turns out to be a nightmare for Saul and
> myself (the people who usually end up dealing with the patches) to do.
> Its relatively easily to update checkouts and note if a patch was
> merged.
>
> Cheers,
>
> Richard
>
>
>
Richard Purdie - Feb. 8, 2014, 12:20 p.m.
On Sat, 2014-02-08 at 12:07 +0000, Laszlo Papp wrote:
> Why is it a nightmare? For instance, maintainers in other projects
> leave a "Pushed, thanks" note.
> 
> See how much discussion this generates already, instead of two words.
> While you are at the change to merge it, you are already in place to
> leave the comment. However, contributors need to go and find the
> corresponding url, etc. Please note that contributors may want to know
> whether their voluntary work got merged without even a checkout, for
> instance double checking from mobile, etc.
> 
> ... or is the real problem the lack of maintainer man power?

Its both a man power problem and the process isn't as simple as
described. 

The changes get batched together into large units, those get tested on
the autobuilder. If they work out ok, the changes go in, if they fail,
we pull out patches until we get a successful batch, then merge.

Upon failures, we do aim to mention those on list. Having to go back
through emails to find the ones which merge and "ack" them is a pain
though since we are "not already in place" as you put it.

There are only two people in general who do this on OE-Core, myself and
Saul. We do have a ton of other pressures on our time, we do the best we
can. If someone wants to ack patches that merge I'm happy for them to do
so, it would be just as much work for them as it would for me/Saul.

I am aware there are patch management tools out there which can show
status of patches. We've looked hard at them and in general they impose
more overhead and process onto people who don't want it.

Cheers,

Richard
Martin Jansa - Feb. 8, 2014, 6:05 p.m.
On Sat, Feb 08, 2014 at 12:20:15PM +0000, Richard Purdie wrote:
> On Sat, 2014-02-08 at 12:07 +0000, Laszlo Papp wrote:
> > Why is it a nightmare? For instance, maintainers in other projects
> > leave a "Pushed, thanks" note.
> > 
> > See how much discussion this generates already, instead of two words.
> > While you are at the change to merge it, you are already in place to
> > leave the comment. However, contributors need to go and find the
> > corresponding url, etc. Please note that contributors may want to know
> > whether their voluntary work got merged without even a checkout, for
> > instance double checking from mobile, etc.
> > 
> > ... or is the real problem the lack of maintainer man power?
> 
> Its both a man power problem and the process isn't as simple as
> described. 
> 
> The changes get batched together into large units, those get tested on
> the autobuilder. If they work out ok, the changes go in, if they fail,
> we pull out patches until we get a successful batch, then merge.
> 
> Upon failures, we do aim to mention those on list. Having to go back
> through emails to find the ones which merge and "ack" them is a pain
> though since we are "not already in place" as you put it.
> 
> There are only two people in general who do this on OE-Core, myself and
> Saul. We do have a ton of other pressures on our time, we do the best we
> can. If someone wants to ack patches that merge I'm happy for them to do
> so, it would be just as much work for them as it would for me/Saul.
> 
> I am aware there are patch management tools out there which can show
> status of patches. We've looked hard at them and in general they impose
> more overhead and process onto people who don't want it.

FWIW: for meta-oe I still use patchwork
(http://patchwork.openembedded.org/) the main advantage for me as
maintainer is that I can split incoming patches into different bundles
(categories) and leave some bundles for sublayers with own dedicated
maintainer for him to handle them.

The hook for automatic "Accepted" flag quite often doesn't close right
patch or doesn't close anything at all and contributers very rarely
update their own patches as well (e.g. when they send v2 they almost
never mark v1 as superseded) so the overall benefit from patchwork is
quite low, but still better than nothing for me.

From contributor POV I don't see need for "ack/Pushed" e-mail, I rebase all
my local changes quite often, so when some are merged I'll notice that
they are gone from my local branch. When some patch is stuck there for
very long time, it's again easy to spot this and send reminder on that
patch email. Another advantage of rebasing my submitted patches is that
I'll see if my patch was modified prior to merge or when older revision
of the patch was merged and rebase leaves me with some significant delta
between v1 and v2 which I can easily submit as incremental patch.
Ross Burton - Feb. 10, 2014, 11:08 a.m.
On 8 February 2014 18:05, Martin Jansa <martin.jansa@gmail.com> wrote:
> From contributor POV I don't see need for "ack/Pushed" e-mail, I rebase all
> my local changes quite often, so when some are merged I'll notice that
> they are gone from my local branch. When some patch is stuck there for
> very long time, it's again easy to spot this and send reminder on that
> patch email. Another advantage of rebasing my submitted patches is that
> I'll see if my patch was modified prior to merge or when older revision
> of the patch was merged and rebase leaves me with some significant delta
> between v1 and v2 which I can easily submit as incremental patch.

FWIW, this is my workflow too.  Lots of feature branches for each
logical unit that I'm working, that get rebased every day (a basic
git-rebase-all is less than ten lines of shell).  I get to notice
patches that no longer apply and branches that are entirely merged
this way.

Ross

Patch

diff --git a/meta/recipes-core/base-passwd/base-passwd/nobash.patch b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
index 6371576..eed87db 100644
--- a/meta/recipes-core/base-passwd/base-passwd/nobash.patch
+++ b/meta/recipes-core/base-passwd/base-passwd/nobash.patch
@@ -1,14 +1,6 @@ 
 use /bin/sh instead of /bin/bash, since the latter may not be included in
 some images such as minimal
 
-comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
-
-remove "*" for root since we don't have a /etc/shadow so far.
-
-by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
-
-Patch managed by http://www.mn-logistik.de/unsupported/pxa250/patcher
-
 Upstream-Status: Invalid [configuration]
 
 Signed-off-by: Scott Garman <scott.a.garman@intel.com>
@@ -17,7 +9,7 @@  Signed-off-by: Scott Garman <scott.a.garman@intel.com>
 +++ base-passwd/passwd.master
 @@ -1,4 +1,4 @@
 -root:*:0:0:root:/root:/bin/bash
-+root::0:0:root:/root:/bin/sh
++root:*:0:0:root:/root:/bin/sh
  daemon:*:1:1:daemon:/usr/sbin:/bin/sh
  bin:*:2:2:bin:/bin:/bin/sh
  sys:*:3:3:sys:/dev:/bin/sh
diff --git a/meta/recipes-core/base-passwd/base-passwd/noshadow.patch b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
new file mode 100644
index 0000000..14e3959
--- /dev/null
+++ b/meta/recipes-core/base-passwd/base-passwd/noshadow.patch
@@ -0,0 +1,18 @@ 
+comment added by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
+
+remove "*" for root since we don't have a /etc/shadow so far.
+
+by Kevin Tian <kevin.tian@intel.com>, 2010-07-06
+
+Upstream-Status: Invalid [configuration]
+
+Signed-off-by: Scott Garman <scott.a.garman@intel.com>
+
+--- base-passwd/passwd.master~nobash
++++ base-passwd/passwd.master
+@@ -1,4 +1,4 @@
+-root:*:0:0:root:/root:/bin/sh
++root::0:0:root:/root:/bin/sh
+ daemon:*:1:1:daemon:/usr/sbin:/bin/sh
+ bin:*:2:2:bin:/bin:/bin/sh
+ sys:*:3:3:sys:/dev:/bin/sh
diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
index 082037a..5b31fce 100644
--- a/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
+++ b/meta/recipes-core/base-passwd/base-passwd_3.5.29.bb
@@ -9,6 +9,7 @@  DEPENDS = "docbook-utils-native linuxdoc-tools-native"
 SRC_URI = "${DEBIAN_MIRROR}/main/b/base-passwd/base-passwd_${PV}.tar.gz \
            file://add_shutdown.patch \
            file://nobash.patch \
+           file://noshadow.patch \
            file://input.patch \
            file://disable-docs.patch \
           "