Comment 6 for bug 1983100

Thanks for the revised debdiff, Tom! I appreciate that you added a short description of each patch to the bug, it's really helpful!

We'll need to adjust a few things before proceeding, a few comments below.

On the debian changelog:
- Since we have many patch files, it would be good to pull the descriptive entry to the top, and list the files underneath
- We should include the LP bug number in the new changelog entry. This should be in the format "(LP: #1983100)", preferably at the end of your topmost entry

On the DEP-3 headers:
- let's point the Origin tags to the commit id of each change, so that a reviewer will be able to find the original commits in a local repo (without going through a github pull request), e.g.:
Origin: upstream, https://github.com/openssl/openssl/commit/56806f432b6c
- for the Bug-Ubuntu tag, we should use the short-link version:
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1983100

On the patches themselves:

I've noticed that patch 4/7 isn't from the same PR series as the rest (it's from PR 7626 [0]), so we should correct the Origin header in that patch file. PR 7626 also has another commit that doesn't seem to be included in the debdiff, did you run tests to ensure it isn't needed for them to work properly?
[0] https://github.com/openssl/openssl/pull/7626

I'm also not fully convinced patch 7/7 is required for this SRU, as it's specifically targeted to Windows systems. I don't think our builds define _WIN32, so I'd imagine the patch would only introduce dead code. Could we drop it from the next version?

Finally, I'd like to double check the patches that are focused on shlibloadtest (patches 1, 3-7). I understand some of them aren't related to the new NO_ATEXIT change, but are fixing the underlying test that's used by that specific patch. Does this mean that shlibloadtest is currently broken for bionic? Or are these patches needed for the additional tests introduced by patch 5?

Ultimately, I wonder if it would be appropriate to e.g. spin the shlibloadtest fixes into a separate LP bug, or if the NO_ATEXIT tests will just outright break without them. Could you please confirm whether this is the case?

Thanks again for all the work on this, Tom! Given we're touching a critical component (openssl) of an LTS release that's soon to move out of standard support, we'll have to be a bit more strict than usual and this SRU is likely to receive extra attention, so thanks for being understanding about it.

Cheers,
Heitor