Sorry, it has taken me a while to get back to this. > If you changed the patch tool to git for those recipes, doesn't it fail with this patch applied?
Even without my patch, I don't think bash works normally with PATCHTOOL = "git" since we unpack it from a tgz. I added PATCHTOOL = "git" to the bash recipe, and it seems to trash my poky repo's git history instead of the source in my bash workdir. :( There might be a separate case we should protect against for that, but I'll have to look at it separately. llvm looks like the perfect example though, and is much more interesting. Initially, it actually fails the git am attempt with "fatal: empty ident name (for <>) not allowed". I think this might be because git doesn't find a "From" line first, or something like that. (If I move Upstream-Status and Signed-off-by below "From" then git am is happy.). But by default git am fails, and the first fallback, git apply, doesn't work with my patch as I expected. git apply appears to only focus on its current directory, unlike git am which I think applies from the work-tree root. I may have only exercised the git am path initially, but I'll investigate the difference here further before providing an updated patch. > We may need to teach GitApplyTree to calculate the difference between the git repodir and patchdir and adjust the offsets accordingly? That was the approach I started with, but I couldn't come up with a scenario where we would end up with an offset other than the default (1) for commands that work from the repo root (git am). So maybe the git am case is the only one that needs to ignore the striplevel, if it truly always expects to work from the repo root. Maybe git apply is the case I unintentionally broke. When I post a revised patch, I'll also update the commit message subject as Peter pointed out. On Thu, Apr 9, 2020 at 6:07 AM Richard Purdie < [email protected]> wrote: > On Thu, 2020-04-09 at 00:41 -0400, [email protected] wrote: > > Don't pass a path removal parameter to git am/git apply since > GitApplyTree > > always applies patches from the root of the git repo, not from each > > individual patchdir (like quilt). > > > > GitApplyTree uses `git rev-parse --show-toplevel` to find the root of > > the repo no matter which patchdir is passed into the GitApplyTree > instance. > > In most cases, this works fine when applying patches since the three-way > > merge can figure out where the patch should be applied to existing files > > based on the history, even if the file paths are excessively stripped. > > > > An exception to this occurs if a patch adds a new file to the repo > > and a custom ${S} has been set to a subdirectory of the repo such > > that the patch requires a non-default striplevel parameter. > > > > When processed by GitApplyTree, the patch will have its striplevel > > parameter given to git even though it's applied at the root of the > > tree. > > > > For example, consider a patch that adds a new file src/foo to a repo. > > > > --- /dev/null > > +++ b/src/foo > > @@ -0,0 +1 @@ > > +new file test > > > > The recipe sets S = "${WORKDIR}/git/src" and the patch is added to > SRC_URI > > using "striplevel=2" since the default patch tool, quilt, applies each > > patch at its patchdir, which defaults to ${S}. > > > > When the GitApplyTree patch class is used (for example, through devtool), > > the root of the repo is passed to git as the work tree, in addition to > the > > path removal paramter. In the case above, GitApplyTree creates > > ${WORKDIR}/git/foo instead of ${WORKDIR}/git/src/foo. > > > > Interestingly enough, you can take a more typical patch example that only > > changes existing files (instead of adding a new file) and remove the -3 > > argument from the git am/git apply commands issued by GitApplyTree. In > > this scenario, the patch will also fail to apply because the path removal > > arg is still being passed, and the resulting path does not exist. > > > > Since GitApplyTree always applies from the root of the repo, it should > > ignore any striplevel arguments. > > I don't follow this argument. We have several recipes in OE-Core which > set striplevel in SRC_URI (e.g. bash, llvm and a pointless looking one > in build-compare). If you changed the patch tool to git for those > recipes, doesn't it fail with this patch applied? > > I think you're right and there is an issue here since do_patch assumes > ${S} is the root for patches whilst git assumes the root of the repo is > the patch root. If those are different, it will break. > > Put another way, I think your patch fixes one case whilst breaking > others :(. > > I'm not sure how we reconcile this. I certainly do not like hoping 'git > am -3' figures it out. We may need to teach GitApplyTree to calculate > the difference between the git repodir and patchdir and adjust the > offsets accordingly? > > Cheers, > > Richard > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137538): https://lists.openembedded.org/g/openembedded-core/message/137538 Mute This Topic: https://lists.openembedded.org/mt/72890514/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
