On Sat, Oct 28, 2023 at 8:33 AM Bruce Ashfield via
lists.openembedded.org
<bruce.ashfield=gmail....@lists.openembedded.org> wrote:
>
> On Fri, Oct 27, 2023 at 7:27 PM Peter Kjellerstedt
> <peter.kjellerst...@axis.com> wrote:
> >
> > > -----Original Message-----
> > > From: openembedded-core@lists.openembedded.org <openembedded-
> > > c...@lists.openembedded.org> On Behalf Of Bruce Ashfield
> > > Sent: den 26 oktober 2023 16:26
> > > To: William Kennington <w...@google.com>; Richard Purdie
> > > <richard.pur...@linuxfoundation.org>
> > > Cc: openembedded-core@lists.openembedded.org
> > > Subject: Re: [OE-core] [PATCH] kernel: Commit without running hooks
> > >
> > > On Thu, Oct 26, 2023 at 6:38 AM William Kennington <w...@google.com>
> > > wrote:
> > > >
> > > > On Wed, Oct 25, 2023 at 7:08 PM Bruce Ashfield
> > > <bruce.ashfi...@gmail.com> wrote:
> > > > >
> > > > > On Wed, Oct 25, 2023 at 6:37 PM William A. Kennington III via
> > > > > lists.openembedded.org <wak=google....@lists.openembedded.org> wrote:
> > > > > >
> > > > > > The hooks are pulled from the impure environment and are often
> > > broken in
> > > > > > our environments. There is no reason to add extra metadata or verify
> > > the
> > > > > > commit message as its arbitrary to turn the tarball into a git repo.
> > > > >
> > > > > But what about the other uses of git ? If the hooks are broken during
> > > > > the creation step, other uses of git should also be broken.
> > > >
> > > > The only time hooks are run is for `git commit`. AFAIK our build
> > > > process has no other invocations of `git commit`. All other git
> > > > operations work fine.
> > >
> > > Right. There are a few others that you just aren't hitting. There's
> > > the git option for patching (for any recipe), and also some use in the
> > > kern-tools that you wouldn't see unless you are also patching or
> > > doing merges as part of the kernel patch phase.
> > >
> > > It would be nice to fix them all at the same time.
> > >
> > > >
> > > > >
> > > > > We've had the ability to check the git config for quite some time, we
> > > > > could take it further an inhibit the use of the host config if we are
> > > > > worried about breakage like this.
> > > >
> > > > Maybe this would be the best idea overall for our git support inside
> > > > the build system, avoiding the build user global git config. That
> > > > probably solves the issue we generally have. I'm okay either way.
> > > >
> > >
> > > Agreed.
> > >
> > > There's some complexity with git versions and the variables that
> > > are available to inhibit local/host/system configuration, but really,
> > > if we want to have isolated and reproducible builds, we should
> > > provide our own configuration.
> > >
> > > Richard: Does that sound reasonable to you ? If you want to fix
> > > this immediate problem, i don't see any harm in the patch, but it
> > > may just mask things and ensure we don't ever fix it fully.
> > >
> > > William: could you raise a bugzilla (and assign it to me) about
> > > git configuration from the user/system influencing our build ? That
> > > would ensure we don't forget it completely :)
> > >
> > > Bruce
> >
> > I would argue that it can be a bad idea to ignore the user's Git
> > configuration. It may, e.g., contain insteadOf settings, which if
> > ignored, prevents the build from fetching from the correct sources.
> > There may be proxy settings that are needed. There may be hooks
> > that are needed. And the list goes on. Trying to avoid problems
> > by completely ignoring the user's configuration is likely to open
> > up another can of problems.
> >
>
> I'd argue that anything critical to the build should be explicitly
> configured and part of our environment. We don't really want our
> fetchers to pull sources using locally configured values, why would
> we let git ?
>
> Relying on a builder or an organization putting these settings
> in a git configuration that is part of the host is error prone as
> well.
>
> I'd much rather flush out dependencies like this and have
> error reports, than to keep them hidden. I'll definitely be
> continuing to work on inhibiting the host environment and see
> what pops up.
I should also add (for the archive's benefit), that I'm only talking
about a limited set of places where we'd opt into this isolation
(for now). In particular for the kernel-yocto bits, and possibly for
the patch.py portions. So just patching / applying changes, I'm
not going to muck with devtool or workflows like that.
What they are presumably applying has already had a basic
level of sanity applied.
Bruce
>
> > That said, specifically ignoring the pre-commit and commit-msg
> > hooks by specifying -n (or --no-verify as I prefer) to git commit
> > is probably a good idea. At least if others are like us and have
> > global Git hooks that verify that the commit message follows our
> > rules...
> >
>
> I only see this as a fallback solution.
>
> Bruce
>
> > //Peter
> >
> > > > >
> > > > > Bruce
> > > > >
> > > > > >
> > > > > > Signed-off-by: William A. Kennington III <w...@google.com>
> > > > > > ---
> > > > > > meta/classes-recipe/kernel-yocto.bbclass | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/meta/classes-recipe/kernel-yocto.bbclass
> > > > > > b/meta/classes-recipe/kernel-yocto.bbclass
> > > > > > index 4ac977b122..cb9cd26b09 100644
> > > > > > --- a/meta/classes-recipe/kernel-yocto.bbclass
> > > > > > +++ b/meta/classes-recipe/kernel-yocto.bbclass
> > > > > > @@ -408,7 +408,7 @@ do_kernel_checkout() {
> > > > > > git init
> > > > > > check_git_config
> > > > > > git add .
> > > > > > - git commit -q -m "baseline commit: creating repo
> > > > > > for ${PN}-${PV}"
> > > > > > + git commit -q -n -m "baseline commit: creating repo
> > > > > > for ${PN}-${PV}"
> > > > > > git clean -d -f
> > > > > > fi
> > > > > >
> > > > > > --
> > > > > > 2.42.0.820.g83a721a137-goog
> > > > >
> > > > > --
> > > > > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > > > > thee at its end
> > > > > - "Use the force Harry" - Gandalf, Star Trek II
> > >
> > > --
> > > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > > thee at its end
> > > - "Use the force Harry" - Gandalf, Star Trek II
> >
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189763):
https://lists.openembedded.org/g/openembedded-core/message/189763
Mute This Topic: https://lists.openembedded.org/mt/102189231/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
