Hello Daichi, To make the review easier and avoiding possible conflicts, I suggest that you rebase your changes on the main repo, I have done this on my side for now but it's a good practice that will save you from a headache: git checkout debian/master git remote add team g...@salsa.debian.org:pkg-security-team/unhide.git git fetch team git rebase team
> * Fix path for python3 interpreter which unhideGui.py invokes https://salsa.debian.org/dfukui/unhide/-/commit/2543fb0034d238df0ad1e94856522faba7bdbba4 I can see that without this patch, the following lintian gets thrown: E: unhide: wrong-path-for-interpreter /bin/python3 != /usr/bin/python3 [usr/lib/unhide/unhideGui.py] That seems like an outdated lintian finding, since starting on bookworm we can assume that all systems have the merged-usr root filesystem layout, so this shouldn't matter. But in any case, a patch is fine. I recommend changing the "Copyright" part of the patch in favor of a DEP3 header, you can use "Author": https://dep-team.pages.debian.net/deps/dep3/ > The most important part to be discussed would be how to incorporate a private > python module ToolTip.py [2] into the unhide deb package. > Note that only unhideGui.py is supposed to use the module. > Referring to a similar discussion [3], I am trying to install ToolTip.py and > unhideGui.py to /usr/lib/unhide/ and then symlink the latter one to > /usr/sbin/unhideGui. > What do you think about this change? Do you think the change above makes > sense? I think that's fine, I'd be interested to hear better alternatives if anybody knows it, but at least we know this will work with no issues. > By the way, the new source package was tested using salsa CI and all jobs > successfully passed [4]. Great. I got the time to checkout unhideGui in more detail now: It looks like parts of it are in French, with no support for translations: https://github.com/YJesus/Unhide/blob/master/unhideGui.py#L291 https://github.com/YJesus/Unhide/blob/master/unhideGui.py#L294 https://github.com/YJesus/Unhide/blob/master/unhideGui.py#L303 This worries me a bit since it might mean that the GUI is not up to standards for packaging it on Debian. I could be wrong, so let me know if you feel otherwise. I'm now thinking we should not ship it with the package but I wouldn't oppose it if anybody acks these issues and says it would still benefit our users. I'm gonna still review the inclusion of the GUI nonetheless, as it might be useful: 1) desktop file: For GUI applications it's always a good idea to provide a .desktop file, so users can start it from their DEs. But in the case of unhide, the GUI would need to change to identify when it's not running as root and call polkit (or something similar) to get the privileges. You don't need to do this, but it's a nice addition for the future (and if we end up shipping the GUI). 2) unhideGui: I think the name of the binary could be improved to try to follow the pattern we usually see on unix tools (and in the other binaries of unhide), maybe unhide-gui (all lowercase)? 3) commit: There's just a single commit which is adding the GUI, the ToolTip and a few other things: https://salsa.debian.org/dfukui/unhide/-/commit/b095ddb22e6c1ad3d403f7ae0c244b3a1a76cc36 I suggest either trying to break down changes like these into multiple commits or writting a bullet-point summary of all the changes of that commit in its description. There are a couple of changes which might have been done by mistake or are unneeded: 3.1) add-new-filename-ToolTip.py.patch: https://salsa.debian.org/dfukui/unhide/-/commit/b095ddb22e6c1ad3d403f7ae0c244b3a1a76cc36#9c7d4d4cec790969b52d1b790c004d070274f8d6_0_1 I don't think this patch is needed, as "tar_list" is only used by upstream to generate the tarball. 3.2) allocate-pid-arrays-from-heap.patch: https://salsa.debian.org/dfukui/unhide/-/commit/b095ddb22e6c1ad3d403f7ae0c244b3a1a76cc36#1f4bc5bbd803ea3626eeb2d591d190112c858dad_1_1 The changes to these patch were probably done automatically by some tool, they're not needed as the patch already has the DEP-3 headers. Thank you for the patience waiting for the review, I try to always reply within 7 days but I didn't have a lot of free time lately. As a rule of thumb, feel free to always ping me after 7 days if you get no reply. Cheers, -- Samuel Henrique <samueloph>