On Fri, Jan 30, 2026 at 2:54 PM Josh Poimboeuf <[email protected]> wrote:
>
> On Fri, Jan 30, 2026 at 12:46:19PM -0800, Song Liu wrote:
> > On Fri, Jan 30, 2026 at 12:14 PM Joe Lawrence <[email protected]> 
> > wrote:
> > >
> > > On Fri, Jan 30, 2026 at 11:58:06AM -0800, Song Liu wrote:
> > > > On Fri, Jan 30, 2026 at 10:00 AM Joe Lawrence <[email protected]> 
> > > > wrote:
> > > > [...]
> > > > > @@ -807,6 +906,8 @@ build_patch_module() {
> > > > >  process_args "$@"
> > > > >  do_init
> > > > >
> > > > > +maybe_rebase_patches
> > > > > +
> > > > >  if (( SHORT_CIRCUIT <= 1 )); then
> > > >
> > > > I think we should call maybe_rebase_patches within this
> > > > if condition.
> > > >
> > >
> > > Hi Song,
> > >
> > > Ah yeah I stumbled on this, probably overthinking it:
> > >
> > >   - we want to validate rebased patches (when requested)
> > >   - validate_patches() isn't really required for step 1 (building the
> > >     original kernel) but ...
> > >   - it's nice to check the patches before going off and building a full
> > >     kernel
> > >   - the patches are needed in step 2 (building the patched kernel) but ...
> > >   - patch validation occurs in step 1
> >
> > Hmm.. I see your point now.
> >
> > > so given the way the short circuiting works, I didn't see a good way to
> > > fold it in there.  The user might want to jump right to building the
> > > patched kernel with patch rebasing.  Maybe that's not valid thinking if
> > > the rebase occurs in step 1 and they are left behind in klp-tmp/ (so
> > > jumping to step 2 will just use the patches in the scratch dir and not
> > > command line?).  It's Friday, maybe I'm missing something obvious? :)
> >
> > Maybe we should add another SHORT_CIRCUIT level for the validate
> > and rebase step? It could be step 0, or we can shift all existing steps.
>
> I don't see how that solves the problem?  For --short-circuit=1 and
> --short-circuit=2 we still want to validate and rebase the patches
> because they are used in step 2.  But as Joe mentioned, that can be done
> before step 1 to catch any patch errors quickly.
>
> Something like this?
>
> if (( SHORT_CIRCUIT <= 2 )); then
>         status "Validating patch(es)"
>         validate_patches
>         fix_patches     # including fixing fuzz???
> fi

I was thinking to change the above as
   if (( SHORT_CIRCUIT <= 0 ))

Then we can save the fixed version of all the patches.

But I think "SHORT_CIRCUIT <= 2" is cleaner, so this version is better.

Thanks,
Song

Reply via email to