pytypes: Resurrect via leak instead of un-finalize in Python 3.8

[EricCousineau-TRI]

See: RobotLocomotion/drake#13026

Relates to the Drake 3.7 -> 3.8 transition from RobotLocomotion/drake#13097.

Requires:

• ci: Fix Travis to actually run Python 3.8 #40 CI updates to actually run Python 3.8 (folded in here to leverage 3.8 actually working)
• Drake testing (local for 3.8, and basic CI) to ensure this doesn't get too funky

---

This change is 

[EricCousineau-TRI]

+@jwnimmer-tri for review, please.

\cc @jamiesnape

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @jwnimmer-tri)

[jwnimmer-tri]

Hmm. Who was the original reviewer for the unique_ptr adaptations? They might better understand the core of the fix. I'm happy to review the remaining shrapnel, though.

[EricCousineau-TRI]

Hm... lemme check.

[EricCousineau-TRI]

@m-chaturvedi was on PR #2.

Jeremy, are you up for reviewing these changes? (they're relatively limited in scope, and is at least a fix for now.)

[jwnimmer-tri]

I have absolutely now idea how unique_ptr & shared_ptr & GC works in pybind11. At best you'll get a rubber stamp from me, but at this point with macOS on fire perhaps that's the best path forward anyway. It would help if someone who knew how GC worked reviewed the pytypes.h fix post-merge.

sign my life away, as our friend says.

Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @EricCousineau-TRI)

a discussion (no related file):
nit It seems bad that we don't have bionic CI, since we only support bionic in Drake? Open a tracking issue?

---

---

include/pybind11/pytypes.h, line 1485 at r1 (raw file):

        _PyGC_SET_FINALIZED(patient_.ptr(), 0);
    }
#endif  // PY_VERSION_HEX >= 0x03080000

nit Technically, this endif is still closing the 3.0.0 check, not the 3.8.0 one, I think?

[jamiesnape]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @EricCousineau-TRI and @jwnimmer-tri)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit It seems bad that we don't have bionic CI, since we only support bionic in Drake? Open a tracking issue?

You can assign it to me. At this stage of the fork, we can probably nuke a lot of the testing that we do not care about anyway.

---

[EricCousineau-TRI]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jamiesnape and @jwnimmer-tri)

a discussion (no related file):

Previously, jamiesnape (Jamie Snape) wrote…

You can assign it to me. At this stage of the fork, we can probably nuke a lot of the testing that we do not care about anyway.

I don't really want to nuke any of the testing from upstream that doesn't prove to be an issue. It's nice to have a (low cost) closer connection to upstream so that merges can focus on the main meat of the updates, and just take whatever CI changes.

I also don't see a huge value gain in testing Bionic in just this fork without submitting an upstream patch.
If we do an upstream patch, then I'm all for it. Also, I have not yet seen any drawbacks from not testing Bionic locally, as the Drake testing itself solves that issue.
(CI patches tend to have a slightly higher response rate.)

That being said, once there's a non-trivial cost (e.g. Py2.7 testing breaks, we see a concrete issue in not having Bionic), then sure, I'm all for ditching.

---

---

include/pybind11/pytypes.h, line 1485 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit Technically, this endif is still closing the 3.0.0 check, not the 3.8.0 one, I think?

Er, I dunno how we decorate #if #elif #endif... Will check GSG to see if there is explicit guidance, or see what other examples there are.

[EricCousineau-TRI]

Woot, thanks!

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jamiesnape and @jwnimmer-tri)

[EricCousineau-TRI]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jamiesnape and @jwnimmer-tri)

a discussion (no related file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

I don't really want to nuke any of the testing from upstream that doesn't prove to be an issue. It's nice to have a (low cost) closer connection to upstream so that merges can focus on the main meat of the updates, and just take whatever CI changes.

I also don't see a huge value gain in testing Bionic in just this fork without submitting an upstream patch.
If we do an upstream patch, then I'm all for it. Also, I have not yet seen any drawbacks from not testing Bionic locally, as the Drake testing itself solves that issue.
(CI patches tend to have a slightly higher response rate.)

That being said, once there's a non-trivial cost (e.g. Py2.7 testing breaks, we see a concrete issue in not having Bionic), then sure, I'm all for ditching.

(Will still make Drake issue, though!)

---

---

include/pybind11/pytypes.h, line 1485 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Er, I dunno how we decorate #if #elif #endif... Will check GSG to see if there is explicit guidance, or see what other examples there are.

OK There is no explicit guidance in our fork of GSG, upstream GSG, nor implicit guidance in the pybind11 upstream code or Drake source code for how to decorate #if #elif #endif. For now, will leave it as is.

[EricCousineau-TRI]

Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

(Will still make Drake issue, though!)

OK Filed issue: RobotLocomotion/drake#13034

---

[EricCousineau-TRI]

CI check is not indicated as complete due to duplicate Travis CI trigger:

However, both of those links are to the same build, and it passed:
https://travis-ci.com/github/RobotLocomotion/pybind11/builds/159715844

Merging.