THRIFT-5794 netstd fix syntax error in deepcopy() method for v0.20.0

[SvenRoederer]

The ending ";" was missing when creating instance of class which caused a syntax error.

This was introduced when adding .net8 support (4115e952b5bed2887113af053b63acd3a03c6e19)

example of generated broken code

    public CIELab DeepCopy()
    {
      var tmp5 = new CIELab()    // <----- here the closing ";" is missing
      if(__isset.L)
      {
        tmp5.L = this.L;
      }

This issue was fixed in master inside 4f18395. This PR only picks the relevant fix into v0.20.0 without adding the feature of this commit.

[Jens-G]

Testcase?

[Jens-G]

Not so sure if that really works. It does not compile and even if it would, it would still be wrong now - because it now writes to the wrong target variable: instead of initializing tmp0.Wtf it copies this.Wtf over with its own value.

Plus I still cannot repro the original issue after spending 30 minutes on it.

struct Foo {
	1: required i32 wtf = 42
}

struct Bar {
	1: required Foo Foo = { "wtf": 132 }
}

[SvenRoederer]

@Jens-G you looking at the wrong place. I updated the PR text,to point to the error-location. It's the 1st instruction after the method-header.

regarding testcase: I need to check what test are running for the project / how tests are organized. But I wonder why this syntax-error was not seen by tests already. As it's syntax error it pops up directly, as long ad deepcopy() is not disabled.

[Jens-G]

Well, it is hard not to look at the right place. I applied your patch and got the errors shown above. The compiler stops there. So what other place would that be? that I should tell the C# compiler to look instead?

It's the 1st instruction after the method-header.

Correct. As in my picture above. If you look carefully, you will find the newly added semicolon, and it is the exact reason that breaks the code after applying your patch.

But I wonder why this syntax-error was not seen by tests already.

Possibly because they had the same reproducible test case as I do - still none. Please provide one or I close the PR.

[SvenRoederer]

Sorry for the confusion, but I sorted it out now.
The issue only affects v0.20.0, where I found it. I fixed the code on my system for v0.20.0 and tested the generator. When adapting my fix to current master, I broke the code as pointed out by @Jens-G.
Looking in depth I realized, that current master has already a fix for the issue. The trick is, that this fix was included to 4f1839575f3af168f960110414114255bd344203 ("implement full deprecation support"). I did not expect this change to fix the syntax-error, while just following the commit messages.

Finally I just updated the PR to mark the change only relevant for v0.20.0.

[Jens-G]

And yet we still don't have a test case ... ok, thanks anyway for your efforts

EDIT: Ah restcase its in the patch now ...

[SvenRoederer]

Yes, I put some hours in adding GitHubAction for netstd (#2995) and added a testcase with separate commit.
I made it also on top of v0.20.0 to see that it actually detects the problem seen.

Feel free to cherry-pick these commits into master.

Is it planned to make a v0.20.1 for this fix or will this version stay not-usable for us and we need to wait for 0.21.0?

[Jens-G]

Is it planned to make a v0.20.1 for this fix or will this version stay not-usable for us and we need to wait for 0.21.0?

I integrated the changes for convenience into the 0.20.0 branch, however there will be most likely no 0.20.1 release just because of this. There is a workaround, you have a patch and it is all open source. The Windows compiler EXE is a convenience build anyway.

PS: Unusable to me is when an update of some closed-source OS leads you into a state where the entire docker engine effectively and reproducibly stalls when you try to run two docker compose sets on it, and you can't get out of that lockup state until you reboot the entire machine and waste four hours on trying to spot the issue in your own code until it dawns on you that it might not be your fault at all - oh wait, exactly that just happened (still unsolved BTW).

[SvenRoederer]

Just seen that @Jens-G created a Jira issue for this.
I squashed all commits into a sinlge one and added updated references to THRIFT-5794. Feel free to cherry-pick the result to master (SvenRoederer@06d37f2)

EDIT: all commits of the test