Comment 3 for bug 1990655

Review for Package: libgit2

[Summary]
I can’t yet ACK the MIR based on the amount of work needed before getting it approved. Check for the Required and Recommended TODOs.
This will need a security review too, but I’m not assigning ubuntu-security right away as the version is not up to date, and some unpackaged releases contains multiple security fixes.

Also there is one question: do we want libgit2-fixtures to be promoted too?

Notes:
Required TODOs:
- as mentioned on the MIR, we need first http-parser to be approved in main.
- however, both -dev and binary package depends on libssh2-1 which is in universe. This one needs to be MIRed too or checked if we can swap with libssh-4.
- as mention on the initial report, the test suite for autopkgtests is kind of trivial. When we are going to introduce cargo, can we have some tests leveraging the library so that this is better covered by its reverse dependency?
- The 4 last upstream releases (some are many months old > 6 months) are not packaged. In particular, v 1.4.4 includes security fixes. This should be updated before taking any consideration on the security team side so that they check a recent code.
Recommended TODOs:
- There are a lot of warnings, making the logs very hard to read. The good news is that it seems that almost all of them are due to __atomic_exchange call. Can this be investigated?

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems:
- as mentioned on the MIR, we need first http-parser to be approved in main.
- however, both -dev and binary package depends on libssh2-1 which is in universe. This one needs to be MIRed too or checked if we can swap with libssh-4.

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

[Security]
OK:
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems:
- long list, but mostly well handled history of CVEs. Due to that, I think a security review will always be beneficial.
- parse some data format (git history) from potentially untrusted sources. This is another advocate to have a security check.

[Common blockers]

OK:
- does not FTBFS currently
- does have a test suite that runs at build time
- test suite fails will fail the build upon error.
- no new python2 dependency

Problems:
- as mention on the initial report, the test suite for autopkgtests is kind of trivial. When we are going to introduce cargo, can we have some tests leveraging the library so that this is better covered by its reverse dependency?

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under control
- symbols tracking is in place
- d/watch is present and looks ok
- Upstream update history is good
- Debian/Ubuntu update history is generally good, but not up to date (see below)
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list

Problems:
- The 4 last upstream releases (some are many months old > 6 months) are not packaged. In particular, v 1.4.4 includes security fixes. This should be updated before taking any consideration on the security team side so that they check a recent code.

[Upstream red flags]
OK:
- no incautious use of malloc/sprintf (as far as we can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK insidev tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu (apart from one in Debian, but it doesn’t seem popular enough to justify blocking the MIR)
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this cas

Problems:
- There are a lot of warnings, making the logs very hard to read. The good news is that it seems that almost all of them are due to __atomic_exchange call. Can this be investigated?