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