Comment 1 for bug 1978066

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Review for Package: jigit

[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: jigit
(jigit, libjte2, libjte-dev)
Specific binary packages built, but NOT to be promoted to main: libjte2, libjte-dev

Required TODOs:
- this seems rarely getting changes (which might be ok as it is just for
  legacy use) but as it wasn't built for 2 years could you please test and
  report back if it still builds fine today? Also the linked "recent
  build" was from 2 years ago. Everyone would hate to fix an issue to then
  be stopped by a rather hard unexpected FTBFS.
- Please add (as you suggested) the demo as self test at autopkgtest time
  and more if you find more candidates for testing it

Recommended TODOs:
- add d/watch file. I understand that the Debian maintainer doesn't need one
  as they unite Debian and Upstream maintainer. But as suggested in the bug
  plenty of tools and checks use it. At least you might ask again as there
  isn't a real burden having one right?
- Build warning like "Wunused-result", I wonder couldn't this be using
  Werror to generally catch more issues early on (and fix this simple case).
  Maybe give it a shot and propose it to Debian?

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

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  Although it was confusing first as there is a dependency to libio-compress-perl
  which is in universe
   libio-compress-perl | 2.105-1 | kinetic/universe | source, all
  But a more detailed check revealed that it is also provided by perl itself
  and therefore working fine. I hope that is working in the component mismatch
  check as well.
- no -dev/-debug/-doc packages that need exclusion
  There is one, but depedencies will not cause a mismatch
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard
- No vendoring used, all Built-Using are in main

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- 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 process arbitrary web content
- 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)

Problems:
- does not parse data formats (jigdo files which can be from external)
- can be used to create images which are then booted (max privilege level)

=> I think to stay on the save side, this should get a security revieww

[Common blockers]
OK:
- does not FTBFS currently
- no special HW required
- no new python2 dependency

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- It is not on the lto-disabled list

Problems:
- d/watch is not present - somewhat explained in Debian #697700
- Upstream update history is slow (concerning, but not a blocker as it
  is legacy code)
- Debian/Ubuntu update history is (concerning, but not a blocker as it
  is legacy code)
- d/rules is rather complex for what the PKG does, but ok still
- a few build warnings indicate we might want to use -Werror to avoid
  issues slipping in

[Upstream red flags]
OK:
- no important Errors/warnings during the build
- 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 inside
  tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu but some
  quite dated bugs
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this use case

Problems: None