cmd: Adjust CMD_LIST to a tree-like structure and implement command chains

Flávio J. Saraiva flaviojs2005 at gmail.com
Thu Mar 23 17:10:14 CDT 2017


I wish it were that simple...

Here's the current situation:
1) some months ago I already added tests that demonstrate the command
chain problems (which are most of the @todo_wine@ that i'm removing in
the commit because they were fixed)
2) IF and FOR actually contain command chains:
   "IF [«options»] «test» «chain» [ELSE «chain»]"
   "FOR [«options»] «var» IN («stuff») DO «chain»"
3) the current tests are heavily dependent on IF, FOR and CALL
4) currently IF, ELSE and FOR are parsed in a hackish way (causing the
first command to be part of the "if ", "else " or "do " command unless
it starts with parens), and because of that particular parsing problem
the IF and FOR commands don't follow the normal execution path... for
some reason parsing was never fixed and instead we get
WCMD_part_execute, which is only needed because of the hackish parsing
5) CALL is also handled in a hackish way that just screams "spaghetti
code" (in particular the called/retryCall parameter and all it's hard
to grasp side effects)
6) pipes and redirects are also handled in a hackish way that only
works for simple cases
7) labels are pretty simple, I just have to make sure they aren't executed

In short:
- if I try to fix IF or FOR first then i'm forced to add chains to
avoid getting lots of errors
- if I try to add chains first then i'm forced to fix IF and FOR to
avoid getting lots of errors
- if I change FOR then I have to handle the difficult 'CALL FOR' test,
which relies on a spagetti of hackish code

So I ask again, considering that I don't see how it can be split while
keeping the tests working, how should I proceed? Is it ok if I split
it into logical bits and only have the tests working at the end? Maybe
splitting the final test file modifications is enough?

Flávio J. Saraiva


2017-03-23 17:14 GMT+00:00 Frédéric Delanoy <frederic.delanoy at gmail.com>:
> On Wed, Mar 22, 2017 at 11:34 AM, Flávio J. Saraiva
> <flaviojs2005 at gmail.com> wrote:
>> Hi,
>>
>> Some time ago I was told that every patch in a patch set all patches
>> should produce a state that passes all the tests.
>> Given how much the current code is interconnected, i don't think i can
>> split this while keeping the tests functional.
>>
>> How should I proceed in this case, can I split the code into logical
>> bits knowing that the intermediate states will not pass the tests?
>>
>> Flávio J. Saraiva
>
> You could start by submitting test-only patches first, demonstrating the issues.
> Then, split the code. For instance, your patch says "Adjust CMD_LIST
> to a tree-like structure and implement command chainsAdjust CMD_LIST
> to a tree-like structure and implement command chains", maybe the
> structure change could be a single patch, ensuring tests still pass?
> Then add one or more patches fixing the tests.
>
> Just my 2 ¢
>
> --
> Frédéric Delanoy
>
>
>> 2017-03-21 21:06 GMT+00:00 Marvin <testbot at winehq.org>:
>>> Thank you for your contribution to Wine!
>>>
>>> This is an automated notification to let you know that your patch has
>>> been reviewed and its status set to "Needs splitting".
>>>
>>> This means that the patch needs to be split, either because it
>>> contains unrelated changes that should be their own separate patch, or
>>> because it's too large for review. You need to find a way to resend
>>> the changes in smaller chunks.
>>>
>>> If you do not understand the reason for this status, disagree with our
>>> assessment, or are simply not sure how to proceed next, please ask for
>>> clarification by replying to this email.



More information about the wine-devel mailing list