#165 Fedora 31: ctypes: Disable checks for union types being passed by value
Merged 4 years ago by zuul. Opened 4 years ago by churchyard.
rpms/ churchyard/python3 f31-00339  into  f31

@@ -0,0 +1,75 @@ 

+ From 9dbf5d3bc2033940cdca35440cf08814544f81e4 Mon Sep 17 00:00:00 2001

+ From: Vinay Sajip <vinay_sajip@yahoo.co.uk>

+ Date: Sun, 12 Jan 2020 20:55:54 +0000

+ Subject: [PATCH] [3.7] bpo-16575: Disabled checks for union types being passed

+  by value. (GH-17960) (GH-17970)

+ 

+ Although the underlying libffi issue remains open, adding these

+ checks have caused problems in third-party projects which are in

+ widespread use. See the issue for examples.

+ 

+ The corresponding tests have also been skipped.

+ (cherry picked from commit c12440c371025bea9c3bfb94945f006c486c2c01)

+ ---

+  Lib/ctypes/test/test_structures.py |  3 ++-

+  Modules/_ctypes/_ctypes.c          | 19 +++++++++++++++++++

+  2 files changed, 21 insertions(+), 1 deletion(-)

+ 

+ diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py

+ index c129377041f03..2eb057a9f3009 100644

+ --- a/Lib/ctypes/test/test_structures.py

+ +++ b/Lib/ctypes/test/test_structures.py

+ @@ -532,6 +532,7 @@ class U(Union):

+              self.assertEqual(f2, [0x4567, 0x0123, 0xcdef, 0x89ab,

+                                    0x3210, 0x7654, 0xba98, 0xfedc])

+  

+ +    @unittest.skipIf(True, 'Test disabled for now - see bpo-16575/bpo-16576')

+      def test_union_by_value(self):

+          # See bpo-16575

+  

+ @@ -612,7 +613,7 @@ class Test5(Structure):

+          self.assertEqual(test5.nested.an_int, 0)

+          self.assertEqual(test5.another_int, 0)

+  

+ -    #@unittest.skipIf('s390' in MACHINE, 'Test causes segfault on S390')

+ +    @unittest.skipIf(True, 'Test disabled for now - see bpo-16575/bpo-16576')

+      def test_bitfield_by_value(self):

+          # See bpo-16576

+  

+ diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c

+ index c8fed44599543..dd0c61fd8a9a9 100644

+ --- a/Modules/_ctypes/_ctypes.c

+ +++ b/Modules/_ctypes/_ctypes.c

+ @@ -2277,6 +2277,23 @@ converters_from_argtypes(PyObject *ob)

+      for (i = 0; i < nArgs; ++i) {

+          PyObject *tp = PyTuple_GET_ITEM(ob, i);

+          PyObject *cnv;

+ +/*

+ + *      The following checks, relating to bpo-16575 and bpo-16576, have been

+ + *      disabled. The reason is that, although there is a definite problem with

+ + *      how libffi handles unions (https://github.com/libffi/libffi/issues/33),

+ + *      there are numerous libraries which pass structures containing unions

+ + *      by values - especially on Windows but examples also exist on Linux

+ + *      (https://bugs.python.org/msg359834).

+ + *

+ + *      It may not be possible to get proper support for unions and bitfields

+ + *      until support is forthcoming in libffi, but for now, adding the checks

+ + *      has caused problems in otherwise-working software, which suggests it

+ + *      is better to disable the checks.

+ + *

+ + *      Although specific examples reported relate specifically to unions and

+ + *      not bitfields, the bitfields check is also being disabled as a

+ + *      precaution.

+ +

+          StgDictObject *stgdict = PyType_stgdict(tp);

+  

+          if (stgdict != NULL) {

+ @@ -2304,6 +2321,8 @@ converters_from_argtypes(PyObject *ob)

+                  return NULL;

+              }

+          }

+ + */

+ +

+          cnv = PyObject_GetAttrString(tp, "from_param");

+          if (!cnv)

+              goto argtypes_error_1;

file modified
+14 -1
@@ -17,7 +17,7 @@ 

  #global prerel ...

  %global upstream_version %{general_version}%{?prerel}

  Version: %{general_version}%{?prerel:~%{prerel}}

- Release: 1%{?dist}

+ Release: 2%{?dist}

  License: Python

  

  
@@ -307,6 +307,15 @@ 

  # In upstream since 3.8: https://bugs.python.org/issue38347

  Patch335: 00335-backport-pathfix-change.patch

  

+ # 00339 #

+ # Disabled checks for union types being passed by value

+ # Although the underlying libffi issue remains open, adding these

+ # checks have caused problems in third-party projects which are in

+ # widespread use. See the issue for examples.

+ # Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1794572

+ # Fixed upstream: https://bugs.python.org/issue16575

+ Patch339: 00339-bpo-16575.patch

+ 

  # (New patches go here ^^^)

  #

  # When adding new patches to "python" and "python3" in Fedora, EL, etc.,
@@ -646,6 +655,7 @@ 

  %patch316 -p1

  %patch328 -p1

  %patch335 -p1

+ %patch339 -p1

  

  

  # Remove files that should be generated by the build
@@ -1554,6 +1564,9 @@ 

  # ======================================================

  

  %changelog

+ * Tue Jan 28 2020 Miro Hrončok <mhroncok@redhat.com> - 3.7.6-2

+ - ctypes: Disable checks for union types being passed by value (#1794572)

+ 

  * Thu Dec 19 2019 Miro Hrončok <mhroncok@redhat.com> - 3.7.6-1

  - Update to 3.7.6

  

Build failed.

@fbo @tdecacqu We will need longer timeout even when running on single architecture. Could you please help me with that?

@churchyard we can set the timeout job attribute. To find where a job is defined, you first need to visit the job page, e.g. https://fedora.softwarefactory-project.io/zuul/job/rpm-scratch-build . The context field indicate the repository, file and branch, e.g. fedora-zuul-jobs-config.

Then we can open a PR like so: https://pagure.io/fedora-zuul-jobs-config/pull-request/21

Thanks. Can I make this PR Depends-On that one by editing the PR description and then type "recheck" to make it build with the new timeout?

@churchyard, not in that case, ... let me try to explain: when Zuul create a PR job context, it does not load CI configurations changes from PRs opened against a trusted repository. In Zuul, a repository can be attached as a trusted or untrusted repository. Repositories that contain critical CI configuration must be attached as trusted, all others repositories can be attached as untrusted.

For instance fedora-zuul-jobs-config is a trusted repository (also called config repository) so it is not possible to use depends-on against it. A secret is contained in fedora-zuul-jobs-config, that's one of the reason to defined it as trusted. It means, we need to carefully review changes on fedora-zuul-job-config before merging.

The usage of depends-on here https://src.fedoraproject.org/rpms/python-gear/pull-request/17 was possible because zuul-distro-jobs is an untrusted repository.

Here https://fedora.softwarefactory-project.io/zuul/projects can been the type for all repositories attached to Zuul.

Build failed.

Let's bump the timeout for rpm-test as well. I have a change in progress that we can use as a depends on https://pagure.io/fedora-zuul-jobs/pull-request/28.

Could you change the initial message of the pull request by adding:
Depends-on: https://pagure.io/fedora-zuul-jobs/pull-request/28

Build succeeded.

Metadata Update from @churchyard:
- Pull-request tagged with: gateit

4 years ago

@churchyard yes no gateit configured by default. The default for rpms/.* is build, lint, test. See: https://pagure.io/fedora-zuul-jobs-config/blob/master/f/zuul.d/projects.yaml#_5

So this change https://pagure.io/fedora-zuul-jobs-config/pull-request/23 just enabled the gateit for python3.

If all the recommended settings from here https://fedoraproject.org/wiki/Zuul-based-ci#Configure_the_repository_for_Zuul have been set then: just remove "gateit" then add again "gateit".

Metadata Update from @churchyard:
- Pull-request untagged with: gateit

4 years ago

Metadata Update from @churchyard:
- Pull-request tagged with: gateit

4 years ago

Build succeeded (gate pipeline).

Pull-Request has been merged by zuul

4 years ago

Merged \o/

If the real Koji build starts, does @zuul post a comment here?

It should, so the configuration needs to be improved. I'll see how to do that.

And actually here a feedback would have help really helpful ... because the rpm-build job in the promote pipeline failed https://fedora.softwarefactory-project.io/zuul/builds?project=rpms%2Fpython3

I'll have a deeper look to the cause of the failure.

So if you need that commit to be build asap then please run the koji build manually.

I've merged a fix for the promote pipeline job. https://pagure.io/fedora-zuul-jobs-config/pull-request/26. So then, I can trigger the build job (promote pipeline) manually. Let me know if your are fine with that.

Ok so after another fix to the job definition (https://pagure.io/fedora-zuul-jobs-config/pull-request/27) I've triggered the rpm-build job in the promote pipeline for that PR. Then now python3 on f31 is building there: https://koji.fedoraproject.org/koji/taskinfo?taskID=41241485

Let me know if that's look ok.

Also should I trigger the job manually for the f30 branch as well ?

Next gateit the process should be automatic this the issue seems fixed.

Thanks. I've triggered the f30 build.

One question: When @zuul triggers a regular build, does it then send a result here, if everything works? Because usually that means I need to prep an update and I have mechanics in place that notify me when my builds are finished, but not somebody elses, like here.

Build succeeded.

Yes it does. Actually I've just found and fixed the issue in the promote pipeline configuration. Here: https://pagure.io/fedora-project-config/pull-request/50

Now, promote pipeline's jobs builds results will be sent here. So you'll know if rpm-build job succeeded or not.

Ah ! \o/ so, just before the rpm-build job, that I've trigger manually, finished Zuul reconfigured itself with the new promote pipeline definition from PR50 then we got the rpm-build result here.