- Wine-devel - list.winehq.org

Gitlab experiment wrapup
by Alexandre Julliard 12 Jul '22

12 Jul '22

Folks, Here's a summary of the Gitlab experiment discussion, with some additional comments. Thanks to everybody who sent their feedback! * Things that people like: - Fetching commits directly with git instead of applying patches from emails (Huw, Jacek, Paul, Zeb). Indeed that's a major help for me as well. - Better tracker, easy to see the list of pending reviews (Huw). - Possibility to host more Wine projects, as well as private Wine trees to share WIP patches (Jacek). Indeed it would be nice to have all Wine projects in one place instead of the current mix of github/sourceforge/etc. - Potential for automation (Jacek). Gitlab offers many services that we will be able to take advantage of, the most obvious being CI for the testbot. Having all the data in a proper database instead of free-form emails should make it possible to do other interesting things as well. * Some other things I like: - Updating status doesn't need to go through me, people can assign reviewers, supersede patches, etc. directly. That reduces my workload and improves the bus factor. Once we have figured out how to make testbot results reliable, we could also have maintainers merge commits directly. - The full discussion thread for a given MR is readily accessible, it doesn't require hunting down the multiple revisions of a patch and associated threads in the mailman archive. * Things that could be improved: - Signoffs are a bit cumbersome (Rémi). We should change the requirements to something better adapted to Gitlab. - It's only possible to approve the whole MR, not individual commits (Huw, Zeb). I think that's an acceptable trade-off, but we could imagine other approaches. - The mailing list gateway creates too much noise; mixing comments from Gitlab and mailing list isn't very clean (Jacek, Rémi, Alex). We can make some tweaks, or use a separate list, or even rethink the approach of the mail gateway. - Gitlab threading support is limited, nested comment threads are not supported (Zeb). That's true, but looking through the past few months of patch reviews, it seems that we almost never use nested threads, so I think we can live with that limitation. - Reviewers can push fixups to commits, but that requires the author to grant explicit permission (Jacek). Hopefully we can tweak access rights to allow this by default. * Conclusion I think Gitlab is working well for us, and most people seem generally happy with it. So my plan is to go forward and make Gitlab the main development platform for Wine. I'll start working on the transition, and on the improvements mentioned above. Any help will be welcome! I'll be posting a roadmap shortly. -- Alexandre Julliard julliard(a)winehq.org

8 25

Re: [PATCH 1/1] mshtml: Wine Gecko 2.47.3 release.
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118828 Your paranoid android. === debian11 (32 bit report) === mshtml: activex: Timeout dom: Timeout events: Timeout htmllocation: Timeout Report validation errors: misc: Timeout === debian11 (build log) === WineRunWineTest.pl:error: The task timed out === debian11 (32 bit WoW report) === mshtml: activex: Timeout dom: Timeout events: Timeout htmllocation: Timeout Report validation errors: misc: Timeout === debian11 (build log) === WineRunWineTest.pl:error: The task timed out

1 0

Re: [PATCH v3 8/8] ntoskrnl: Set Device ContainerID from driver when registering an interface
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118812 Your paranoid android. === debian11 (32 bit report) === ntoskrnl.exe: driver_pnp.c:703: Test failed: Got 1 remove events.

1 0

Re: [PATCH v3 7/8] ntoskrnl: Set device ContainerID from driver
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118811 Your paranoid android. === debian11 (64 bit WoW report) === ntoskrnl.exe: driver_pnp.c:703: Test failed: Got 1 remove events.

1 0

Re: [PATCH v3 6/8] ntoskrnl: Improve error handling in get_device_id
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118810 Your paranoid android. === debian11 (64 bit WoW report) === ntoskrnl.exe: driver_pnp.c:703: Test failed: Got 1 remove events.

1 0

Re: [PATCH v2 7/8] ntoskrnl: Set device ContainerID from driver
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118793 Your paranoid android. === debian11 (32 bit report) === ntoskrnl.exe: driver_pnp.c:703: Test failed: Got 1 remove events.

1 0

Re: [PATCH 1/1] po: Update Dutch translation
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118786 Your paranoid android. === debian11 (build log) === error: patch failed: po/nl.po:16050 Task: Patch failed to apply === debian11 (build log) === error: patch failed: po/nl.po:16050 Task: Patch failed to apply

1 0

Re: [PATCH v2 5/8] imm32: Don't allow disabling other thread's IME by thread ID.
by Marvin 12 Jul '22

12 Jul '22

Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118782 Your paranoid android. === w1064_2qxl (64 bit report) === imm32: imm32.c:1351: Test failed: Expected no IME windows

1 0

[tools] testbot/web: Directly report multiple special job errors using AddError().
by Francois Gouget 12 Jul '22

12 Jul '22

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/admin/SpecialJobs.pl | 36 +++++++++++--------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/testbot/web/admin/SpecialJobs.pl b/testbot/web/admin/SpecialJobs.pl index d8294c1aaf..066c41be33 100644 --- a/testbot/web/admin/SpecialJobs.pl +++ b/testbot/web/admin/SpecialJobs.pl @@ -158,7 +158,6 @@ sub OnSubmit($) { my ($self) = @_; my $VMKey; - my @Errors = (); # Update Build VMs $VMKey = $self->{JobTemplates}->{Build}->{VMKey}; @@ -166,9 +165,8 @@ sub OnSubmit($) { $VMKey = undef if ($VMKey eq "*All Build VMs"); my $VMs = GetReconfigVMs($VMKey, "build"); - push @Errors, "Found no build VM to update" if (!@$VMs); - my $ErrMessage = AddReconfigJob($VMs, $VMKey, "build"); - push @Errors, $ErrMessage if (defined $ErrMessage); + $self->AddError("Found no build VM to update") if (!@$VMs); + $self->AddError(AddReconfigJob($VMs, $VMKey, "build")); } # Run 32 bit WineTest @@ -183,9 +181,8 @@ sub OnSubmit($) foreach my $BaseJob (@BaseJobs) { my $VMs = GetWindowsTestVMs($VMKey, "exe32", $BaseJob); - push @Errors, "Found no $BaseJob VM to run WineTest on" if (!@$VMs); - my $ErrMessage = AddWindowsTestJob($VMs, $VMKey, "exe32", $BaseJob, "winetest-latest.exe"); - push @Errors, $ErrMessage if (defined $ErrMessage); + $self->AddError("Found no $BaseJob VM to run WineTest on") if (!@$VMs); + $self->AddError(AddWindowsTestJob($VMs, $VMKey, "exe32", $BaseJob, "winetest-latest.exe")); } } @@ -201,9 +198,8 @@ sub OnSubmit($) undef; $VMKey = undef if ($VMKey eq "*All Base and WineTest VMs"); my $VMs = GetWindowsTestVMs($VMKey, "exe64", $BaseJob); - push @Errors, "Found no 64 bit VM to run WineTest on" if (!@$VMs); - my $ErrMessage = AddWindowsTestJob($VMs, $VMKey, "exe64", $BaseJob, "winetest64-latest.exe"); - push @Errors, $ErrMessage if (defined $ErrMessage); + $self->AddError("Found no 64 bit VM to run WineTest on") if (!@$VMs); + $self->AddError(AddWindowsTestJob($VMs, $VMKey, "exe64", $BaseJob, "winetest64-latest.exe")); } # Update Wine VMs @@ -212,9 +208,8 @@ sub OnSubmit($) { $VMKey = undef if ($VMKey eq "*All Wine VMs"); my $VMs = GetReconfigVMs($VMKey, "wine"); - push @Errors, "Found no Wine VM to update" if (!@$VMs); - my $ErrMessage = AddReconfigJob($VMs, $VMKey, "wine"); - push @Errors, $ErrMessage if (defined $ErrMessage); + $self->AddError("Found no Wine VM to update") if (!@$VMs); + $self->AddError(AddReconfigJob($VMs, $VMKey, "wine")); } # Run WineTest on Wine @@ -223,21 +218,14 @@ sub OnSubmit($) { $VMKey = undef if ($VMKey eq "*All Wine VMs"); my $VMs = GetWineTestVMs($VMKey); - push @Errors, "Found no Wine VM to run WineTest on" if (!@$VMs); - my $ErrMessage = AddWineTestJob($VMs, $VMKey); - push @Errors, $ErrMessage if (defined $ErrMessage); + $self->AddError("Found no Wine VM to run WineTest on") if (!@$VMs); + $self->AddError(AddWineTestJob($VMs, $VMKey)); } # Notify engine - my $ErrMessage = RescheduleJobs(); - push @Errors, $ErrMessage if (defined $ErrMessage); - - if (@Errors) - { - $self->AddError(join("\n", @Errors)); - return undef; - } + $self->AddError(RescheduleJobs()); + return undef if ($self->GetErrMessage()); exit($self->Redirect("/")); } -- 2.30.2

1 0

[tools] testbot/cgi: Report all of a form's invalid fields at once.
by Francois Gouget 12 Jul '22

12 Jul '22

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/ObjectModel/CGI/FormPage.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm index c299d54b4a..d6ded1b07b 100644 --- a/testbot/lib/ObjectModel/CGI/FormPage.pm +++ b/testbot/lib/ObjectModel/CGI/FormPage.pm @@ -599,6 +599,7 @@ sub Validate($) { my ($self) = @_; + my $Ret = 1; foreach my $PropertyDescriptor (@{$self->GetPropertyDescriptors()}) { my $Value = $self->GetParam($PropertyDescriptor->GetName()); @@ -606,11 +607,11 @@ sub Validate($) if ($ErrMessage) { $self->AddError($ErrMessage, $PropertyDescriptor->GetName()); - return !1; + $Ret = !1; } } - return 1; + return $Ret; } sub SaveProperty($$$) -- 2.30.2

1 0