[PATCH 5/6] winegstreamer: Rename gstdemux.c to quartz_parser.c.

[Zebediah Figura (she/her)]

On 2/23/21 8:12 AM, Francois Gouget wrote:
> On Tue, 23 Feb 2021, Francois Gouget wrote:
> 
>> On Mon, 22 Feb 2021, Zebediah Figura (she/her) wrote:
>> [...]
>>> https://testbot.winehq.org/JobDetails.pl?Key=85998
>> [...]
>>>> === debiant2 (build log) ===
>>>>
>>>> /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/winegstreamer/quartz_parser.c:1509: undefined reference to `heap_realloc'
>>>
>>> I have no idea what happened here. The patch on the jobs page looks
>>> correct, and changes all of the heap_* calls mentioned here. At any rate
>>> it compiles fine for me, of course...
>>
>> I can reproduce the build error if I apply the TestBot patch on top of 
>> 4981785f0fbc. The issue is that quartz_parser.c does not #include 
>> wine/heap.h. Maybe it assumes some other header is going to do it but 
>> that's not happening.
> 
> So the thing is that 'git apply' does not give the same result as 
> 'patch -p1'. I think it's a bug in the way 'git apply' handles renamed 
> files, or maybe more specifically in the way it handles patches that 
> first modify a file and then rename it.
> 
> The latter may be outside the scope of what git apply is meant to 
> support if one supposes that the patches it is meant to work on come 
> from a single Git commit.
> 
> When a commit both renames and modifies a file, the resulting Git diff 
> does both in one operation:
> 
> diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/quartz_parser.c
> similarity index 97%
> rename from dlls/winegstreamer/gstdemux.c
> rename to dlls/winegstreamer/quartz_parser.c
> index 84e6bcd61e4..e9e7c3a0f9f 100644
> --- a/dlls/winegstreamer/gstdemux.c
> +++ b/dlls/winegstreamer/quartz_parser.c
> 
> 
> Here the job's patch is the result of concatenating the patch series 
> parts. So it first modifies gstdemux.c three times and only then renames 
> it to quartz_parser.c.
> 
> But it seems that instead of doing that, git apply first makes a copy of 
> gstdemux.c as quartz_parser.c, and only then patches gstdemux.c (and 
> never deletes gstdemux.c). [1]
> 
> So the result is that with git apply one gets the unmodified gstdemux.c 
> which still references heap_*().
> 
> Options:
> 1. Use patch -p1. But I believe Alexandre uses git apply which 
>    sometimes rejects standalone patches that patch -p1 accepts.
>    patch -p1 may also have other issues of its own (e.g. [1]).
> 
> 2. Use git apply for standalone patches and patch -p1 for patch series?
>    (Or just patches where there's a rename). Yuck.
> 
> 3. Apply each part one at a time. That would require putting some 
>    delimiter in the concatenated patch so it can be split and have each 
>    part committed one at a time on the build machine.

My guess is that (3) is the best option, or at least, it's the most
reliable. [That or we never concat the patches in the first place...?]

> 
> 
> 
> [1] I suspect this is so git apply can handle Git-generated patches that 
>     rename A to B and B to A at the same time. I wonder if patch -p1 can 
>     handle that.
> 

Yeah, that's a fair point, so it's probably not a bug after all...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20210223/79503443/attachment.sig>