Bug 1912120 - Review Request: imhex - Hex editor for reverse engineering
Summary: Review Request: imhex - Hex editor for reverse engineering
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2021-01-03 16:41 UTC by Artur Frenszek-Iwicki
Modified: 2021-06-28 18:43 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-06-28 11:45:21 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Artur Frenszek-Iwicki 2021-01-03 16:41:09 UTC
spec: https://svgames.pl/fedora/imhex-1.5.0-1/imhex.spec
srpm: https://svgames.pl/fedora/imhex-1.5.0-1/imhex-1.5.0-1.fc33.src.rpm
koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=58846929

Description: ImHex is a hex editor for reverse engineers, programmers, and people who value their eyesight while working late at night.

Fedora Account System Username: suve

Comment 1 Ben Beasley 2021-02-20 16:58:13 UTC
I’m glad to see you have correctly handled the bundled dependencies in the License field. You still need to follow https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling: if upstream does not support building against an external/system version of the bundled dependencies, you must contact them publicly about a path to doing so, and you must add the appropriate virtual Provides to indicate the bundling.

The latest version is 1.7.0, and it has additional bundled dependencies which you must also handle.

Since this is a GUI application, you need a desktop file (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_files). If upstream does not provide one, you must write one. You should (not must, but should) also write an AppData XML file (https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/). It makes sense to offer both of these upstream.

When ignoring format-security warnings, you should comment on whether you think they are spurious, and if not, why not. Or, report them upstream and let upstream decide, and link the bug report.

Comment 2 Otto Liljalaakso 2021-04-21 09:39:12 UTC
Nothing seems to be happening with this review. Do you still intend to complete the package?

Comment 3 Artur Frenszek-Iwicki 2021-04-21 11:09:10 UTC
>Since this is a GUI application, you need a desktop file [...] also an AppData XML file [...]
Sure.

>When ignoring format-security warnings, you should comment on whether you think they are spurious, and if not, why not. Or, report them upstream and let upstream decide, and link the bug report.
This has been fixed since, 1.7.0 does not require the patch.

>Do you still intend to complete the package?
I tried updating to 1.7.0 and building that fails because of RPATH issues. I have exactly zero knowledge of how RPATH works, so unless upstream fixes this on their side, I don't really see myself pushing this forward.

ERROR   0002: file '/usr/bin/imhex' contains an invalid rpath '/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex' in [/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui/external/glad:]
ERROR   0002: file '/usr/bin/imhex' contains an invalid rpath '/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui' in [/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui/external/glad:]
ERROR   0002: file '/usr/bin/imhex' contains an invalid rpath '/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui/external/glad' in [/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui/external/glad:]
ERROR   0010: file '/usr/bin/imhex' contains an empty rpath in [/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui:/home/rpmbuilder/rpmbuild/BUILD/ImHex-1.7.0/x86_64-redhat-linux-gnu/plugins/libimhex/external/ImGui/external/glad:]

Comment 4 Ben Beasley 2021-04-21 12:47:06 UTC
RPATH is a path for finding shared libraries, hard-coded into the executable instead of picked up from the environment. We usually have to remove these or prevent their creation in Fedora, but they can be acceptable for internal libraries like plugins (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_rpath_for_internal_libraries), which at a glance is likely the case here. However, the build is setting RPATHs that are totally wrong, since they refer to the build environment. They will probably need to be adjusted with chrpath.

I can probably help you figure out how to deal with the RPATH issues, if you are willing to work through them along with correctly dealing with the bundled dependencies, writing the desktop/AppData files, and any other issues. For example, based on the README at https://github.com/WerWolv/ImHex#linux, there are likely a lot more files that need to be manually installed.

On the other hand, if you are having second thoughts and don’t want to finish the work needed for this package, you can say so and close this bug as WONTFIX, and that’s just fine too.

Comment 5 Artur Frenszek-Iwicki 2021-04-23 07:32:40 UTC
> I can probably help you figure out how to deal with the RPATH issues
Much appreciated.

> correctly dealing with the bundled dependencies
Some CMake help may also be needed.

> writing the desktop/AppData files
No biggie.

> if you are having second thoughts and don’t want to finish the work needed for this package
Not guaranteeing that I'm gonna see this through, though the program seems interesting enough to me that I'd like to give it one more go.
Worst case scenario we'll end up with a bunch of patches that, hopefully, will be of interest to upstream.

I'll start with un-bundling new libraries introduced since 1.5.0 and let you know about progress.

Comment 6 Ben Beasley 2021-06-24 19:23:55 UTC
Just checking if you are still working on this or not…

Comment 7 Artur Frenszek-Iwicki 2021-06-28 11:45:21 UTC
Unfortunately I haven't really touched on this since our last exchange.

I'll close this in case someone else wants to have a go, so they won't be blocked by the "no duplicate requests" policy.


Note You need to log in before you can comment on or make changes to this bug.