Re: [OE-core] [PATCH 1/1] patch: don't strip GitApplyTree patches

["Richard Purdie" <richard.purdie@linuxfoundation.org>]

On Thu, 2020-04-09 at 00:41 -0400, bkylerussell@gmail.com 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