diff mbox series

PR to fix WantedBy parsing of systemctl

Message ID b5c12365825c4ccf862a0d58029b0939DM5PR02MB221823F6928D9018544ECA3B834C9@DM5PR02MB2218.namprd02.prod.outlook.com
State New
Headers show
Series PR to fix WantedBy parsing of systemctl | expand

Commit Message

Robert Henz Sept. 20, 2022, 5:55 p.m. UTC 
Hi everyone,

I have a PR to fix the WantedBy parsing of the systemctl script. I tried to follow the process described in your wiki (https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded), but ran into some problems I wasn't able to figure out. I will attach the patch file for my suggested change to this email but I also went ahead and created a fork and generated a GitHub PR in case that is ok/easier?

https://github.com/openembedded/openembedded-core/pull/78

Again, I apologize for not being able to follow your defined procedure, please let me know if there is additional information you need from me.

Thanks,
Bob

Comments

Alexandre Belloni Sept. 20, 2022, 9:19 p.m. UTC | #1 
Hello,

On 20/09/2022 17:55:42+0000, Robert Henz via lists.openembedded.org wrote:
> Hi everyone,
> 
> I have a PR to fix the WantedBy parsing of the systemctl script. I
> tried to follow the process described in your wiki
> (https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded),
> but ran into some problems I wasn't able to figure out. I will attach
> the patch file for my suggested change to this email but I also went
> ahead and created a fork and generated a GitHub PR in case that is
> ok/easier?

This is not easier to me. However, I'm interested to know more about the
issues you couldn't solve as this is a recurring topic.


> From 6cdb207ed99bc82324fb7b85126425ac8d439070 Mon Sep 17 00:00:00 2001
> From: Bob Henz <robert_henz@jabil.com>
> Date: Mon, 19 Sep 2022 22:02:58 -0400
> Subject: [PATCH] Fix WantedBy processing
> 
> An empty string assignment to WantedBy should clear all prior WantedBy
> settings. This matches behavior of the current systemd implementation.

Your SoB is missing here and this is mandatory.

> ---
>  meta/recipes-core/systemd/systemd-systemctl/systemctl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl
> index 6d19666d82..cddae75a06 100755
> --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl
> +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl
> @@ -26,6 +26,9 @@ locations = list()
>  
>  class SystemdFile():
>      """Class representing a single systemd configuration file"""
> +
> +    _clearable_keys = ['WantedBy']
> +
>      def __init__(self, root, path, instance_unit_name):
>          self.sections = dict()
>          self._parse(root, path)
> @@ -80,6 +83,14 @@ class SystemdFile():
>                  v = m.group('value')
>                  if k not in section:
>                      section[k] = list()
> +
> +                # If we come across a "key=" line for a "clearable key", then
> +                # forget all preceding assignments. This works because we are
> +                # processing files in correct parse order.
> +                if k in self._clearable_keys and not v:
> +                    del section[k]
> +                    continue
> +
>                  section[k].extend(v.split())
>  
>      def get(self, section, prop):
> -- 
> 2.34.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#170905): https://lists.openembedded.org/g/openembedded-core/message/170905
> Mute This Topic: https://lists.openembedded.org/mt/93809846/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Robert Henz Sept. 26, 2022, 3:49 p.m. UTC | #2 
On Tue, Sep 20, 2022 at 05:19 PM, Alexandre Belloni wrote:

> 
> Hello,
> 
> On 20/09/2022 17:55:42+0000, Robert Henz via lists.openembedded.org wrote:
> 
> 
>> Hi everyone,
>> 
>> I have a PR to fix the WantedBy parsing of the systemctl script. I
>> tried to follow the process described in your wiki
>> ( https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded ),
>> 
>> but ran into some problems I wasn't able to figure out. I will attach
>> the patch file for my suggested change to this email but I also went
>> ahead and created a fork and generated a GitHub PR in case that is
>> ok/easier?
> 
> This is not easier to me. However, I'm interested to know more about the
> issues you couldn't solve as this is a recurring topic.

I am following the instructions (https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Sending_via_a_pull_request) under "Sending via a pull request".
I created a github fork (cloned it, created my commit and pushed it back to a branch of my forked repo).

The problem I've run into is this:

```

bobhenz@hound:~/src/openembedded-core$ scripts/create-pull-request -u badger
NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
NOTE: Assuming local branch HEAD, use -l to override.
fatal: unable to connect to github.com:
github.com[0: 140.82.114.4]: errno=Connection timed out

warn: No match for commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 found at git://github.com/BadgerTechnologies/openembedded-core
warn: Are you sure you pushed 'fix-wantedby-clearing' there?
ERROR: git request-pull reported an error
bobhenz@hound:~/src/openembedded-core$ git log
commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 (HEAD -> fix-wantedby-clearing, badger/fix-wantedby-clearing)
Author: Bob Henz <robert_henz@jabil.com>
Date:   Mon Sep 19 22:02:58 2022 -0400

Fix WantedBy processing

An empty string assignment to WantedBy should clear all prior WantedBy
settings. This matches behavior of the current systemd implementation.

Signed-off-by: Bob Henz <robert_henz@jabil.com>

commit 68b07bb7f0f01925f9da1cb966239ee49d5c84e3 (origin/master, origin/HEAD, badger/master, master)
Author: He Zhe <zhe.he@windriver.com>
Date:   Mon Sep 26 16:08:07 2022 +0800

lttng-tools: Disable on qemuriscv32

lttng-tools requires SYS_ppoll and SYS_pselect6 which are not supported on
riscv32. This has been confirmed by lttng-tools upstream.
https://github.com/lttng/lttng-tools/pull/162

It's also turned off for riscv32 in meta-riscv.
https://github.com/riscv/meta-riscv/blob/master/conf/layer.conf

Signed-off-by: He Zhe <zhe.he@windriver.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
```

(As you can see from the git log, that commit was pushed to the remote forked repo called "badger".)

> 
> 
>> From 6cdb207ed99bc82324fb7b85126425ac8d439070 Mon Sep 17 00:00:00 2001
>> From: Bob Henz <robert_henz@jabil.com>
>> Date: Mon, 19 Sep 2022 22:02:58 -0400
>> Subject: [PATCH] Fix WantedBy processing
>> 
>> An empty string assignment to WantedBy should clear all prior WantedBy
>> settings. This matches behavior of the current systemd implementation.
> 
> Your SoB is missing here and this is mandatory.

I will attach a new patch with a SoB. Can you explain the value of having me sign off on my own PR? I guess I thought someone with more authority would be signing off on the change...

Thanks for your guidance
Martin Jansa Sept. 26, 2022, 5:52 p.m. UTC | #3 
Looks like your badger remote is still using git:// protocol which isn't
supported by github.com anymore, you need to change it to https://.

On Mon, Sep 26, 2022 at 5:49 PM Robert Henz via lists.openembedded.org
<robert_henz=jabil.com@lists.openembedded.org> wrote:

> On Tue, Sep 20, 2022 at 05:19 PM, Alexandre Belloni wrote:
>
> Hello,
>
> On 20/09/2022 17:55:42+0000, Robert Henz via lists.openembedded.org wrote:
>
> Hi everyone,
>
> I have a PR to fix the WantedBy parsing of the systemctl script. I
> tried to follow the process described in your wiki
> (https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded),
> but ran into some problems I wasn't able to figure out. I will attach
> the patch file for my suggested change to this email but I also went
> ahead and created a fork and generated a GitHub PR in case that is
> ok/easier?
>
> This is not easier to me. However, I'm interested to know more about the
> issues you couldn't solve as this is a recurring topic.
>
> I am following the instructions (
> https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Sending_via_a_pull_request)
> under "Sending via a pull request".
> I created a github fork (cloned it, created my commit and pushed it back
> to a branch of my forked repo).
>
> The problem I've run into is this:
>
> ```
>
> bobhenz@hound:~/src/openembedded-core$ scripts/create-pull-request -u
> badger
> NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
> NOTE: Assuming local branch HEAD, use -l to override.
> fatal: unable to connect to github.com:
> github.com[0: 140.82.114.4]: errno=Connection timed out
>
> warn: No match for commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 found
> at git://github.com/BadgerTechnologies/openembedded-core
> warn: Are you sure you pushed 'fix-wantedby-clearing' there?
> ERROR: git request-pull reported an error
> bobhenz@hound:~/src/openembedded-core$ git log
> commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 (HEAD ->
> fix-wantedby-clearing, badger/fix-wantedby-clearing)
> Author: Bob Henz <robert_henz@jabil.com>
> Date:   Mon Sep 19 22:02:58 2022 -0400
>
>     Fix WantedBy processing
>
>     An empty string assignment to WantedBy should clear all prior WantedBy
>     settings. This matches behavior of the current systemd implementation.
>
>     Signed-off-by: Bob Henz <robert_henz@jabil.com>
>
> commit 68b07bb7f0f01925f9da1cb966239ee49d5c84e3 (origin/master,
> origin/HEAD, badger/master, master)
> Author: He Zhe <zhe.he@windriver.com>
> Date:   Mon Sep 26 16:08:07 2022 +0800
>
>     lttng-tools: Disable on qemuriscv32
>
>     lttng-tools requires SYS_ppoll and SYS_pselect6 which are not
> supported on
>     riscv32. This has been confirmed by lttng-tools upstream.
>     https://github.com/lttng/lttng-tools/pull/162
>
>     It's also turned off for riscv32 in meta-riscv.
>     https://github.com/riscv/meta-riscv/blob/master/conf/layer.conf
>
>     Signed-off-by: He Zhe <zhe.he@windriver.com>
>     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ```
>
> (As you can see from the git log, that commit was pushed to the remote
> forked repo called "badger".)
>
> From 6cdb207ed99bc82324fb7b85126425ac8d439070 Mon Sep 17 00:00:00 2001
> From: Bob Henz <robert_henz@jabil.com>
> Date: Mon, 19 Sep 2022 22:02:58 -0400
> Subject: [PATCH] Fix WantedBy processing
>
> An empty string assignment to WantedBy should clear all prior WantedBy
> settings. This matches behavior of the current systemd implementation.
>
> Your SoB is missing here and this is mandatory.
>
> I will attach a new patch with a SoB. Can you explain the value of having
> me sign off on my own PR? I guess I thought someone with more authority
> would be signing off on the change...
>
> Thanks for your guidance
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#171068):
> https://lists.openembedded.org/g/openembedded-core/message/171068
> Mute This Topic: https://lists.openembedded.org/mt/93809846/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Robert Henz Sept. 26, 2022, 7:50 p.m. UTC | #4 
I think I'm missing some context. Not really understanding what this script is trying to accomplish I don't know what "remote branch" is vs "local branch". Should "remote branch" be "master" and the local branch be the branch with the change I want to make? Here's that script running with "set -x" turned on 1/2 way through...

bobhenz@hound:~/src/openembedded-core$ scripts/create-pull-request -u badger
+ USER_RE=[A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?
+ PROTO_RE=[a-z][a-z+]*://
+ GIT_RE=\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)
+ REMOTE_URL=https://github.com/BadgerTechnologies/openembedded-core
+ echo https://github.com/BadgerTechnologies/openembedded-core
+ sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#\5#
+ REMOTE_REPO=BadgerTechnologies/openembedded-core
+ echo https://github.com/BadgerTechnologies/openembedded-core
+ sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#git://\4/\5#
+ REMOTE_URL=git://github.com/BadgerTechnologies/openembedded-core
+ [ -z  ]
+ git branch
+ grep -e ^\*
+ cut -d  -f2
+ BRANCH=fix-wantedby-clearing
+ echo NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
+ [ -z  ]
+ L_BRANCH=HEAD
+ echo NOTE: Assuming local branch HEAD, use -l to override.
NOTE: Assuming local branch HEAD, use -l to override.
+ [ 0 -eq 1 ]
+ WEB_URL=
+ WEB_URL=https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing
+ [ -n https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing ]
+ [  = 1 ]
+ wget --no-check-certificate -q https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing -O /dev/null
+ [ 0 -ne 0 ]
+ [ -e pull-186156 ]
+ mkdir pull-186156
+ [ -n  ]
+ git format-patch -M40 --subject-prefix=PATCH -n -o pull-186156 --thread=shallow --cover-letter master..HEAD
+ ls -A pull-186156
+ [ -z 0000-cover-letter.patch
0001-Fix-WantedBy-processing.patch ]
+ [ -n  ]
+ echo pull-186156/0000-cover-letter.patch
+ CL=pull-186156/0000-cover-letter.patch
+ PM=pull-186156/pull-msg
+ + trgit -d --version
[:alpha:][:space:].
+ sed s/\(...\).*/\1/
+ git version 2.25.1
+ GIT_VERSION=225
+ NEWER_GIT_VERSION=210
+ [ 225 -lt 210 ]
+ git request-pull master git://github.com/BadgerTechnologies/openembedded-core HEAD:fix-wantedby-clearing
fatal: unable to connect to github.com:
github.com[0: 140.82.113.3]: errno=Connection timed out

warn: No match for commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 found at git://github.com/BadgerTechnologies/openembedded-core
warn: Are you sure you pushed 'fix-wantedby-clearing' there?
+ [ 1 -ne 0 ]
+ echo ERROR: git request-pull reported an error
ERROR: git request-pull reported an error
+ rm -rf pull-186156
+ exit 1
Martin Jansa Sept. 26, 2022, 8:31 p.m. UTC | #5 
see the REMOTE_URL changes shown in set -x, relevant context is in:
https://git.openembedded.org/openembedded-core/commit/?id=675e88e6e0bbd5ab2dcd4bdf97b0de59925a1be6

We forgot to update it after:
https://github.blog/2021-09-01-improving-git-protocol-security-github/#no-more-unauthenticated-git
and unfortunately it shows how rarely this script is being used.

I've sent a fix for this, please test it.

On Mon, Sep 26, 2022 at 9:50 PM Robert Henz via lists.openembedded.org
<robert_henz=jabil.com@lists.openembedded.org> wrote:

> I think I'm missing some context. Not really understanding what this
> script is trying to accomplish I don't know what "remote branch" is vs
> "local branch". Should "remote branch" be "master" and the local branch be
> the branch with the change I want to make? Here's that script running with
> "set -x" turned on 1/2 way through...
>
> bobhenz@hound:~/src/openembedded-core$ scripts/create-pull-request -u
> badger
> + USER_RE=[A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?
> + PROTO_RE=[a-z][a-z+]*://
> + GIT_RE=\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@
> -]*$\?@\)\?\([^:/]*\)[:/]\(.*\)
> + REMOTE_URL=https://github.com/BadgerTechnologies/openembedded-core
> + echo https://github.com/BadgerTechnologies/openembedded-core
> + sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@
> -]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#\5#
> + REMOTE_REPO=BadgerTechnologies/openembedded-core
> + echo https://github.com/BadgerTechnologies/openembedded-core
> + sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@
> -]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#git://\4/\5#
> + REMOTE_URL=git://github.com/BadgerTechnologies/openembedded-core
> + [ -z  ]
> + git branch
> + grep -e ^\*
> + cut -d  -f2
> + BRANCH=fix-wantedby-clearing
> + echo NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to
> override.
> NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
> + [ -z  ]
> + L_BRANCH=HEAD
> + echo NOTE: Assuming local branch HEAD, use -l to override.
> NOTE: Assuming local branch HEAD, use -l to override.
> + [ 0 -eq 1 ]
> + WEB_URL=
> + WEB_URL=
> https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing
> + [ -n
> https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing
> ]
> + [  = 1 ]
> + wget --no-check-certificate -q
> https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing
> -O /dev/null
> + [ 0 -ne 0 ]
> + [ -e pull-186156 ]
> + mkdir pull-186156
> + [ -n  ]
> + git format-patch -M40 --subject-prefix=PATCH -n -o pull-186156
> --thread=shallow --cover-letter master..HEAD
> + ls -A pull-186156
> + [ -z 0000-cover-letter.patch
> 0001-Fix-WantedBy-processing.patch ]
> + [ -n  ]
> + echo pull-186156/0000-cover-letter.patch
> + CL=pull-186156/0000-cover-letter.patch
> + PM=pull-186156/pull-msg
> + + trgit -d --version
>  [:alpha:][:space:].
> + sed s/\(...\).*/\1/
> + git version 2.25.1
> + GIT_VERSION=225
> + NEWER_GIT_VERSION=210
> + [ 225 -lt 210 ]
> + git request-pull master git://
> github.com/BadgerTechnologies/openembedded-core HEAD:fix-wantedby-clearing
> fatal: unable to connect to github.com:
> github.com[0: 140.82.113.3]: errno=Connection timed out
>
> warn: No match for commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 found
> at git://github.com/BadgerTechnologies/openembedded-core
> warn: Are you sure you pushed 'fix-wantedby-clearing' there?
> + [ 1 -ne 0 ]
> + echo ERROR: git request-pull reported an error
> ERROR: git request-pull reported an error
> + rm -rf pull-186156
> + exit 1
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#171073):
> https://lists.openembedded.org/g/openembedded-core/message/171073
> Mute This Topic: https://lists.openembedded.org/mt/93809846/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Robert Henz Sept. 27, 2022, 1:30 a.m. UTC | #6 
I have confirmed your patch worked for me. Perhaps the documentation should also be updated to indicate that remotes need to be cloned/added using the https protocol?

I still can't send the resulting two files via git send-email (the final step), as I would have to do a significant amount of configuration in my _personal_ gmail account in order to use git send-email to send the final patch. I will attach the resulting two files here and perhaps that will be sufficient for someone to begin reviewing my actual PR.

Thanks,
Bob
Martin Jansa Sept. 27, 2022, 2:50 a.m. UTC | #7 
>  I still can't send the resulting two files via git send-email (the final
step), as I would have to do a significant amount of configuration in my
_personal_ gmail account in order to use git send-email to send the final
patch.

FWIW: I'm also using my gmail account and the configuration was quite
simple, install msmtp, create app password in google security settings,
write it in ~/.msmtprc and git send-email works fine (the only issue is
that I'm seeing each sent e-mail twice, once as sent by me and once the
same e-mail received through ML).

Regards,

On Tue, Sep 27, 2022 at 3:30 AM Robert Henz via lists.openembedded.org
<robert_henz=jabil.com@lists.openembedded.org> wrote:

> I have confirmed your patch worked for me. Perhaps the documentation
> should also be updated to indicate that remotes need to be cloned/added
> using the https protocol?
>
> I still can't send the resulting two files via git send-email (the final
> step), as I would have to do a significant amount of configuration in my
> _personal_ gmail account in order to use git send-email to send the final
> patch. I will attach the resulting two files here and perhaps that will be
> sufficient for someone to begin reviewing my actual PR.
>
> Thanks,
> Bob
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#171081):
> https://lists.openembedded.org/g/openembedded-core/message/171081
> Mute This Topic: https://lists.openembedded.org/mt/93809846/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Alexandre Belloni Sept. 27, 2022, 9:50 a.m. UTC | #8 
On 26/09/2022 08:49:36-0700, Robert Henz via lists.openembedded.org wrote:
> >> An empty string assignment to WantedBy should clear all prior WantedBy
> >> settings. This matches behavior of the current systemd implementation.
> > 
> > Your SoB is missing here and this is mandatory.
> 
> I will attach a new patch with a SoB. Can you explain the value of having me sign off on my own PR? I guess I thought someone with more authority would be signing off on the change...
> 

You are singing of on the DCO:
https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin
Ross Burton Sept. 27, 2022, 10:13 a.m. UTC | #9 
Just use git-send-email directly, for a simple patch you don’t need the overhead of create-pull-request.

Ross

From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> on behalf of Robert Henz via lists.openembedded.org <robert_henz=jabil.com@lists.openembedded.org>
Date: Tuesday, 27 September 2022 at 10:53
To: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org>
Subject: Re: [OE-core] PR to fix WantedBy parsing of systemctl
I think I'm missing some context. Not really understanding what this script is trying to accomplish I don't know what "remote branch" is vs "local branch". Should "remote branch" be "master" and the local branch be the branch with the change I want to make? Here's that script running with "set -x" turned on 1/2 way through...

bobhenz@hound:~/src/openembedded-core$ scripts/create-pull-request -u badger
+ USER_RE=[A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?
+ PROTO_RE=[a-z][a-z+]*://
+ GIT_RE=\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)
+ REMOTE_URL=https://github.com/BadgerTechnologies/openembedded-core
+ echo https://github.com/BadgerTechnologies/openembedded-core
+ sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#\5#
+ REMOTE_REPO=BadgerTechnologies/openembedded-core
+ echo https://github.com/BadgerTechnologies/openembedded-core
+ sed s#\(^\([a-z][a-z+]*://\)\?\)\([A-Za-z0-9_.@][A-Za-z0-9_.@-]*$\?@\)\?\([^:/]*\)[:/]\(.*\)#git://\4/\5#
+ REMOTE_URL=git://github.com/BadgerTechnologies/openembedded-core
+ [ -z  ]
+ git branch
+ grep -e ^\*
+ cut -d  -f2
+ BRANCH=fix-wantedby-clearing
+ echo NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
NOTE: Assuming remote branch 'fix-wantedby-clearing', use -b to override.
+ [ -z  ]
+ L_BRANCH=HEAD
+ echo NOTE: Assuming local branch HEAD, use -l to override.
NOTE: Assuming local branch HEAD, use -l to override.
+ [ 0 -eq 1 ]
+ WEB_URL=
+ WEB_URL=https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing
+ [ -n https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing ]
+ [  = 1 ]
+ wget --no-check-certificate -q https://github.com/BadgerTechnologies/openembedded-core/tree/fix-wantedby-clearing -O /dev/null
+ [ 0 -ne 0 ]
+ [ -e pull-186156 ]
+ mkdir pull-186156
+ [ -n  ]
+ git format-patch -M40 --subject-prefix=PATCH -n -o pull-186156 --thread=shallow --cover-letter master..HEAD
+ ls -A pull-186156
+ [ -z 0000-cover-letter.patch
0001-Fix-WantedBy-processing.patch ]
+ [ -n  ]
+ echo pull-186156/0000-cover-letter.patch
+ CL=pull-186156/0000-cover-letter.patch
+ PM=pull-186156/pull-msg
+ + trgit -d --version
 [:alpha:][:space:].
+ sed s/\(...\).*/\1/
+ git version 2.25.1
+ GIT_VERSION=225
+ NEWER_GIT_VERSION=210
+ [ 225 -lt 210 ]
+ git request-pull master git://github.com/BadgerTechnologies/openembedded-core HEAD:fix-wantedby-clearing
fatal: unable to connect to github.com:
github.com[0: 140.82.113.3]: errno=Connection timed out

warn: No match for commit b637a46f58b5adc7dcc76bdef90a7be8c8462df8 found at git://github.com/BadgerTechnologies/openembedded-core
warn: Are you sure you pushed 'fix-wantedby-clearing' there?
+ [ 1 -ne 0 ]
+ echo ERROR: git request-pull reported an error
ERROR: git request-pull reported an error
+ rm -rf pull-186156
+ exit 1
Robert Henz Sept. 27, 2022, 1:33 p.m. UTC | #10 
Ross,

I was trying to use the Pull Request method because originally I thought it would mean I didn't have to use git send-email. Now, of course, I realize that it's just some helpful wrapper scripts around using git send-email. :-(
Robert Henz Sept. 27, 2022, 1:46 p.m. UTC | #11 
I'll make you all a deal. When you all update your documentation ( https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded ) to include the procedure for setting up my personal gmail account to allow git send-email to work from my work laptop, I'll do it and submit my patch that way.

The accepted answer here seems overly complex but probably (mostly?) correct and a little scathing of this entire process so might not be where you want people trying to help with your project to have to read through: https://stackoverflow.com/questions/68238912/how-to-configure-and-use-git-send-email-to-work-with-gmail-to-email-patches-to
Ross Burton Oct. 28, 2022, 11:46 a.m. UTC | #12 
On 27 Sep 2022, at 14:46, Robert Henz via lists.openembedded.org <robert_henz=jabil.com@lists.openembedded.org> wrote:
> 
> I'll make you all a deal. When you all update your documentation (https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded) to include the procedure for setting up my personal gmail account to allow git send-email to work from my work laptop, I'll do it and submit my patch that way.

We’re not going to expand the wiki to cover how to setup git-send-email for every email server.

If your MTA (like GMail) needs authenticated SMTP, then you need the password. If your MTA uses 2FA (like GMail) then you typically need an application password.  Generating this is step 2 of that SO page you linked to, although the use of git-credentials is overkill as you can just tell git the password the same way you tell it what sever to use.

[sendemail]
from = Ross Burton <ross@burtonini.com>
smtpserver = smtp.gmail.com
smtpuser = ...
smtppass = …

Ross
diff mbox series

Patch

From 6cdb207ed99bc82324fb7b85126425ac8d439070 Mon Sep 17 00:00:00 2001
From: Bob Henz <robert_henz@jabil.com>
Date: Mon, 19 Sep 2022 22:02:58 -0400
Subject: [PATCH] Fix WantedBy processing

An empty string assignment to WantedBy should clear all prior WantedBy
settings. This matches behavior of the current systemd implementation.
---
 meta/recipes-core/systemd/systemd-systemctl/systemctl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl
index 6d19666d82..cddae75a06 100755
--- a/meta/recipes-core/systemd/systemd-systemctl/systemctl
+++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl
@@ -26,6 +26,9 @@  locations = list()
 
 class SystemdFile():
     """Class representing a single systemd configuration file"""
+
+    _clearable_keys = ['WantedBy']
+
     def __init__(self, root, path, instance_unit_name):
         self.sections = dict()
         self._parse(root, path)
@@ -80,6 +83,14 @@  class SystemdFile():
                 v = m.group('value')
                 if k not in section:
                     section[k] = list()
+
+                # If we come across a "key=" line for a "clearable key", then
+                # forget all preceding assignments. This works because we are
+                # processing files in correct parse order.
+                if k in self._clearable_keys and not v:
+                    del section[k]
+                    continue
+
                 section[k].extend(v.split())
 
     def get(self, section, prop):
-- 
2.34.1