diff mbox series

[6/6] bitbake-setup: make sure git user is always configured

Message ID 20250901135836.2927686-6-alex.kanavin@gmail.com
State New
Headers show
Series [1/6] bitbake-setup: add the initial implementation | expand

Commit Message

Alexander Kanavin Sept. 1, 2025, 1:58 p.m. UTC
From: Gyorgy Sarvari <skandigraun@gmail.com>

In case a git username and email are not configured, then git errors
out when commiting the initial and later the changed configuration
of the setup. This can happen if bitbake-setup is running in a
container, or for example if the user just doesn't use git for
committing.

To avoid this failure, configure a dummy user to be used with
comitting the config changes.

Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
---
 bin/bitbake-setup | 3 +++
 1 file changed, 3 insertions(+)

Comments

Peter Kjellerstedt Sept. 2, 2025, 11:49 a.m. UTC | #1
> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Alexander Kanavin via lists.openembedded.org
> Sent: den 1 september 2025 15:59
> To: bitbake-devel@lists.openembedded.org
> Cc: Gyorgy Sarvari <skandigraun@gmail.com>
> Subject: [bitbake-devel] [PATCH 6/6] bitbake-setup: make sure git user is always configured
> 
> From: Gyorgy Sarvari <skandigraun@gmail.com>
> 
> In case a git username and email are not configured, then git errors
> out when commiting the initial and later the changed configuration
> of the setup. This can happen if bitbake-setup is running in a
> container, or for example if the user just doesn't use git for
> committing.
> 
> To avoid this failure, configure a dummy user to be used with
> comitting the config changes.
> 
> Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
> ---
>  bin/bitbake-setup | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/bin/bitbake-setup b/bin/bitbake-setup
> index 7382dc8d2..67188891f 100755
> --- a/bin/bitbake-setup
> +++ b/bin/bitbake-setup
> @@ -392,6 +392,9 @@ def init_config(settings, args, d):
>      os.makedirs(layerdir)
> 
>      bb.process.run("git -C {} init -b main".format(confdir))
> +    # Make sure commiting doesn't fail if no default git user is configured on the machine

commiting -> committing 

> +    bb.process.run("git -C {} config user.name bitbake-setup".format(confdir))
> +    bb.process.run("git -C {} config user.email bitbake-setup@not.set".format(confdir))
>      bb.process.run("git -C {} commit --allow-empty -m 'Initial commit'".format(confdir))

Please only set the Git name and/or mail if they are not already set 
(see, e.g., oe.patch.gitCommandUserOptions(), and check_git_config() in 
utils.bbclass). We have a global Git hook that validates the name and 
email of the committer and the above git commit command will fail in 
our environment.

> 
>      bb.event.register("bb.build.TaskProgress", handle_task_progress, data=d)
> --
> 2.39.5

//Peter
Alexander Kanavin Sept. 2, 2025, 12:33 p.m. UTC | #2
On Tue, 2 Sept 2025 at 13:49, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:

> > +    bb.process.run("git -C {} config user.name bitbake-setup".format(confdir))
> > +    bb.process.run("git -C {} config user.email bitbake-setup@not.set".format(confdir))
> >      bb.process.run("git -C {} commit --allow-empty -m 'Initial commit'".format(confdir))
>
> Please only set the Git name and/or mail if they are not already set
> (see, e.g., oe.patch.gitCommandUserOptions(), and check_git_config() in
> utils.bbclass). We have a global Git hook that validates the name and
> email of the committer and the above git commit command will fail in
> our environment.

I'm not fully comfortable with doing that. The commits in this repo
are created by a program, and so the author should be the program, not
the user running it (who did not give consent to use their name and
email, regardless of how harmless the content is).

Can you fix the hook to run on pushes, and not commits? Or add
bitbake-setup to allowlist?

Alex
Peter Kjellerstedt Sept. 2, 2025, 10:31 p.m. UTC | #3
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 2 september 2025 14:33
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: bitbake-devel@lists.openembedded.org; Gyorgy Sarvari <skandigraun@gmail.com>
> Subject: Re: [bitbake-devel] [PATCH 6/6] bitbake-setup: make sure git user is always configured
> 
> On Tue, 2 Sept 2025 at 13:49, Peter Kjellerstedt <peter.kjellerstedt@axis.com> wrote:
> 
> > > +    bb.process.run("git -C {} config user.name bitbake-setup".format(confdir))
> > > +    bb.process.run("git -C {} config user.email bitbake-setup@not.set".format(confdir))
> > >      bb.process.run("git -C {} commit --allow-empty -m 'Initial commit'".format(confdir))
> >
> > Please only set the Git name and/or mail if they are not already set
> > (see, e.g., oe.patch.gitCommandUserOptions(), and check_git_config() in
> > utils.bbclass). We have a global Git hook that validates the name and
> > email of the committer and the above git commit command will fail in
> > our environment.
> 
> I'm not fully comfortable with doing that. The commits in this repo
> are created by a program, and so the author should be the program, not
> the user running it (who did not give consent to use their name and
> email, regardless of how harmless the content is).

I must admit I have never thought that way. In my opinion, I'm 
creating the commit regardless of that it is some tool I am 
using that is creating it for me. 

> Can you fix the hook to run on pushes, and not commits? Or add
> bitbake-setup to allowlist?

The whole point of that hook is to catch misconfigurations before 
they make it into any repository, i.e., at the time a commit is 
created/updated.

There is no allowlist as the intent is that the Git configuration 
must be valid (which we define as the configured Git name and email 
must match what is in our LDAP database). Users typically use 
the --no-verify option if they have to override the hook for some 
reason. Which I guess would be an alternative here too...

(Yes, our hooks may seem draconic, but I like to believe that they 
have kept my mind at least somewhat sane given how many developers 
we have who need to use Git, and how easy it is to get it wrong 
when you are new to Git.)

> Alex

//Peter
Alexander Kanavin Sept. 3, 2025, 9:57 a.m. UTC | #4
On Wed, 3 Sept 2025 at 00:32, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> > I'm not fully comfortable with doing that. The commits in this repo
> > are created by a program, and so the author should be the program, not
> > the user running it (who did not give consent to use their name and
> > email, regardless of how harmless the content is).
>
> I must admit I have never thought that way. In my opinion, I'm
> creating the commit regardless of that it is some tool I am
> using that is creating it for me.

I would agree to have me as author of the commit only if I wrote the
tool myself (not true for users of bitbake-setup), or the tool stops
and gives me a chance to look at the changes before making them (also
not true for bitbake-setup).

It's not any different from sending automated emails, if they come
from a bot, then that should be made clear. I don't sign AUH emails
with my own name, even though I maintain the periodic autobuilder job
and the tools that produce them.

> > Can you fix the hook to run on pushes, and not commits? Or add
> > bitbake-setup to allowlist?
>
> The whole point of that hook is to catch misconfigurations before
> they make it into any repository, i.e., at the time a commit is
> created/updated.

But you can also perform that check when users run 'git push', and
reject the push. It's a far more common way of enforcing rules.
Restricting their commits is a step too far imo, and once again puts
you into the situation of having to ask the yocto project to handle
your organization's 'special needs'.

Alex
Alexander Kanavin Sept. 3, 2025, 12:14 p.m. UTC | #5
On Wed, 3 Sept 2025 at 00:32, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> There is no allowlist as the intent is that the Git configuration
> must be valid (which we define as the configured Git name and email
> must match what is in our LDAP database). Users typically use
> the --no-verify option if they have to override the hook for some
> reason. Which I guess would be an alternative here too...

This I've now added locally. I haven't pushed the changes yet, as I
also need to implement the top-dir-prefix idea.

Alex
Gyorgy Sarvari Sept. 3, 2025, 1:05 p.m. UTC | #6
On 9/1/25 15:58, Alexander Kanavin wrote:
> Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
> ---
>

As an more or less unrelated comment: you are free to squash my patches
into your ones. It will help readability and I don't insist on having my
name there (not because I disown the patches, rather they are just not
as significant to warrant my name plastered on half of the patches).
Alexander Kanavin Sept. 4, 2025, 10:52 a.m. UTC | #7
On Wed, 3 Sept 2025 at 15:05, Gyorgy Sarvari <skandigraun@gmail.com> wrote:
> As an more or less unrelated comment: you are free to squash my patches
> into your ones. It will help readability and I don't insist on having my
> name there (not because I disown the patches, rather they are just not
> as significant to warrant my name plastered on half of the patches).

No problem. I squashed them in but mentioned you in the commit message:

This commits includes fixes by Gyorgy Sarvari <skandigraun@gmail.com>
https://github.com/kanavin/bitbake/pull/2

Other reported small fixes are also added and pushed, top-dir-prefix
is still to be implemented.

Alex
diff mbox series

Patch

diff --git a/bin/bitbake-setup b/bin/bitbake-setup
index 7382dc8d2..67188891f 100755
--- a/bin/bitbake-setup
+++ b/bin/bitbake-setup
@@ -392,6 +392,9 @@  def init_config(settings, args, d):
     os.makedirs(layerdir)
 
     bb.process.run("git -C {} init -b main".format(confdir))
+    # Make sure commiting doesn't fail if no default git user is configured on the machine
+    bb.process.run("git -C {} config user.name bitbake-setup".format(confdir))
+    bb.process.run("git -C {} config user.email bitbake-setup@not.set".format(confdir))
     bb.process.run("git -C {} commit --allow-empty -m 'Initial commit'".format(confdir))
 
     bb.event.register("bb.build.TaskProgress", handle_task_progress, data=d)