For git sources, instead of backporting patches and listing them in the SRC_URI, perhaps it would be neater to support specfying the commit sha ID's directly in the SRC_URI, and let bitbake/git handle cherry-picking the patches. This would only work for patches that git can cherry-pick cleanly, but still, it would mean we could avoid patch files; a backport would just be a new entry in SRC_URI.
Or maybe a git-backport.bbclass that would inspect ${GIT_BACKPORT} and append to do_patch() accordingly.
Or more general git-cherry-pick.bbclass/${GIT_CHERRY_PICK}
(In reply to comment #0) > For git sources, instead of backporting patches and listing them in the > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > patches. Not so neat would be that this would make it a lot harder to review the changes, or figuring out what patches are applied when looking at the Yocto sources. With the normal "git format-patch" generated patches I am able to see already in the SRC_URI what each patch is about. Replacing these with shas would be a clear regression for me.
(In reply to comment #3) > (In reply to comment #0) > > For git sources, instead of backporting patches and listing them in the > > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > > patches. > > Not so neat would be that this would make it a lot harder to review the > changes, or figuring out what patches are applied when looking at the Yocto > sources. > > With the normal "git format-patch" generated patches I am able to see > already in the SRC_URI what each patch is about. Replacing these with shas > would be a clear regression for me. Yeah the point would be to require less information to describe the patch, so naturally we would loose something. On the other hand, it would be guaranteed that the patch is coming from upstream, in which case it has already been reviewed by the right people one would assume. The syntax for specifying the patches could require a matching summary, like: GIT_CHERRY_PICK = "fe2f5da:squashfs-tools-fix-build-failure-against-gcc-10" which would give at least some context.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #0) > > > For git sources, instead of backporting patches and listing them in the > > > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > > > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > > > patches. > > > > Not so neat would be that this would make it a lot harder to review the > > changes, or figuring out what patches are applied when looking at the Yocto > > sources. > > > > With the normal "git format-patch" generated patches I am able to see > > already in the SRC_URI what each patch is about. Replacing these with shas > > would be a clear regression for me. > > Yeah the point would be to require less information to describe the patch, > so naturally we would loose something. I do not see what we would actually win. > On the other hand, it would be > guaranteed that the patch is coming from upstream, in which case it has > already been reviewed by the right people one would assume. It would only be guaranteed that the patch is coming from some branch in the upstream repository. What happens when this is a master-next branch that gets rebased? What happens when this is a personal development branch of one upstream developer, and the change never gets accepted upstream?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (In reply to comment #0) > > > > For git sources, instead of backporting patches and listing them in the > > > > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > > > > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > > > > patches. > > > > > > Not so neat would be that this would make it a lot harder to review the > > > changes, or figuring out what patches are applied when looking at the Yocto > > > sources. > > > > > > With the normal "git format-patch" generated patches I am able to see > > > already in the SRC_URI what each patch is about. Replacing these with shas > > > would be a clear regression for me. > > > > Yeah the point would be to require less information to describe the patch, > > so naturally we would loose something. > > I do not see what we would actually win. > Obviously less git churning. If one wants to review the patch it is available in the git repository, no need to duplicate it in yocto repos. > > On the other hand, it would be > > guaranteed that the patch is coming from upstream, in which case it has > > already been reviewed by the right people one would assume. > > It would only be guaranteed that the patch is coming from some branch in the > upstream repository. > Of course its possible to enforce it coming from a specific branch in the syntax: GIT_CHERRY_PICK = "master:fe2f5da:squashfs-tools-fix-build-failure-against-gcc-10"
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > (In reply to comment #0) > > > > > For git sources, instead of backporting patches and listing them in the > > > > > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > > > > > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > > > > > patches. > > > > > > > > Not so neat would be that this would make it a lot harder to review the > > > > changes, or figuring out what patches are applied when looking at the Yocto > > > > sources. > > > > > > > > With the normal "git format-patch" generated patches I am able to see > > > > already in the SRC_URI what each patch is about. Replacing these with shas > > > > would be a clear regression for me. > > > > > > Yeah the point would be to require less information to describe the patch, > > > so naturally we would loose something. > > > > I do not see what we would actually win. > > Obviously less git churning. If one wants to review the patch it is > available in the git repository, no need to duplicate it in yocto repos. You make patch review sound like something that would be optional, and that discouraging it by making it more burdensome would be desirable. The person adding a patch (usually Richard/Khem) is supposed to do some review, and there are several people like me (or you) who regularly skim through the submissions on the mailing lists and/or the changes in master-next.
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (In reply to comment #3) > > > > > (In reply to comment #0) > > > > > > For git sources, instead of backporting patches and listing them in the > > > > > > SRC_URI, perhaps it would be neater to support specfying the commit sha ID's > > > > > > directly in the SRC_URI, and let bitbake/git handle cherry-picking the > > > > > > patches. > > > > > > > > > > Not so neat would be that this would make it a lot harder to review the > > > > > changes, or figuring out what patches are applied when looking at the Yocto > > > > > sources. > > > > > > > > > > With the normal "git format-patch" generated patches I am able to see > > > > > already in the SRC_URI what each patch is about. Replacing these with shas > > > > > would be a clear regression for me. > > > > > > > > Yeah the point would be to require less information to describe the patch, > > > > so naturally we would loose something. > > > > > > I do not see what we would actually win. > > > > Obviously less git churning. If one wants to review the patch it is > > available in the git repository, no need to duplicate it in yocto repos. > > You make patch review sound like something that would be optional, and that > discouraging it by making it more burdensome would be desirable. > > The person adding a patch (usually Richard/Khem) is supposed to do some > review, and there are several people like me (or you) who regularly skim > through the submissions on the mailing lists and/or the changes in > master-next. I'm only considering backports here. I think if a patch can be proven to come directly from upstream, and has been merged to master, then I don't see a reason for duplicating the review work by people who possibly aren't that familiar with the project in question. Besides, reviewing patches only works for trivial fixes. As soon as it gets more complicated, you need more than 5 lines of context. But I see your stand-point. I'm not saying this should replace normal patches, just that in some situations it might be an easier and more trustworthy method of patching the SW.
We don't intend support approach because the patches would often need to be refreshed and it would be a maintenance burden.
(In reply to comment #9) > We don't intend support approach because the patches would often need to be > refreshed and it would be a maintenance burden. As I've stated, this would preferably be used for *backported* patches. They would just go away once there is a version upgrade, thus not be a maintenance burden.