Bug 2346284 - Review Request: hexcurse - Ncurses-based console hexeditor
Summary: Review Request: hexcurse - Ncurses-based console hexeditor
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Jakub Kadlčík
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/prso/hexcurse-ng
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-02-18 16:51 UTC by Davide Bosio
Modified: 2025-05-31 11:39 UTC (History)
1 user (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
jkadlcik: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 9072358 to 9104307 (434 bytes, patch)
2025-05-30 11:16 UTC, Fedora Review Service
no flags Details | Diff

Description Davide Bosio 2025-02-18 16:51:38 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08668934-hexcurse/hexcurse.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08668934-hexcurse/hexcurse-1.60-1.fc43.src.rpm
Description: Console-based hex editor allowing you to edit files in hexadecimal and ASCII representation.
Fedora Account System Username: brosion

Comment 1 Jakub Kadlčík 2025-02-21 19:21:56 UTC
Hello Davide,
thank you for the package.

> Issues:
> =======
> - The License field must be a valid SPDX expression.
>  Note: Not a valid SPDX expression 'GPL-2.0'.
>  See: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1

This was reported by the `fedora-review` tool


Small issue, but I don't like the inconsistent whitespace between
sections. Please pick whether one or two blank lines and stick with it.


> rm -rf %{buildroot}

We used to do this in the past, but not anymore. As per:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections

> The contents of the buildroot SHOULD NOT be removed in the first line of
> %install. 

You can remove the line


> Source0:        https://github.com/Bosiux/hexcurse/releases/download/v1.60/hexcurse-1.60.tar.gz

Multiple things:

- We recently started referencing a single source as `Source:` instead of
  `Source0:`
- It's better to not hardcode the version and package name here. You can use
  %{name}, %{version}, and optionally even %{URL} variables. This way, once a
  new version is released, you'll only update the `Version:` tag.
- The URL seems to be wrong, it returns 404. This may be helpful
  https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags


> %description    

The description should be wrapped to 80 characters per line
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description


> URL:            https://github.com/Bosiux/hexcurse

This is the biggest one, but I am not sure what the rules are. 

This is obviously a two weeks old fork of a decade old 
https://github.com/LonnyGomes/hexcurse . I checked all the major distributions 
and everybody packages hexcurse from that upstream. It doesn't sound reasonable
to me to package the fork. But as I said, I am not sure what the rules are. 
I will have to check.

If your fork contains changes that are required for the software to run on
Fedora, I'd rather package the original upstream and added the changes as
downstream patches
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines

Or maybe we should package this one instead?
https://github.com/prso/hexcurse-ng/

Comment 2 Davide Bosio 2025-02-22 07:43:07 UTC
Hi Jakub, thank you for the time you spent helping me.
As I told you, this is a repack i made for my own, it wasn't originally 
designed for distribution, I made the git repository once you told me to bring 
it up a couple of days ago.

Now i deleted my fork, I acknowledge that its not the upstream project but
the actual one doesn't even work on RHEL systems due to a
bad designed code and style used.

I even tryed to contact the original developer to create an upstream branch
in the original repository but I'm still waiting for a response.

Since packing a fork seems also bad from me, is there any way to pack it 
from the original repository with all changes needed to work? 
For example adding the Stack error handling, implementing a better 'Find' 
solution from the rusty original one, adding the mouse clicking events 
that has never been enabled and so on.

Please tell me how to move, you know I'm new.

Have a good day,
Davide.

Comment 3 Jakub Kadlčík 2025-02-22 10:11:18 UTC
> Now i deleted my fork, I acknowledge that its not the upstream project 

I am sorry Davide, I didn't mean you to delete your fork. Forking an upstream
project is perfectly fine, and sharing it with people is too. Don't worry, you
did a good thing there.

I am only concerned whether we can package it for Fedora, saying that the fork
is the upstream. 


> the actual one doesn't even work on RHEL systems due to a bad designed code
> and style used. 

This being the case, I think the correct solution would be packaging the
original project and applying your changes as patches. More about them here
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines

The upstream is looking inactive/dead, but you should propose the patches via
pull requests anyway. Then we can confidently say we did everything we could.


But I have an alternative proposal. The choice is entirely up to you, but I think
this could be less work.

There is a hexcurse-ng project - https://github.com/prso/hexcurse-ng/ which
looks alive. The maintainer proposed several changes to the original project and
didn't get them merged, so they are probably in the same situation as you
are. Do you want to try if the hexcurse-ng works for you, and potentially
package that project instead of the original hexcurse?

Comment 4 Davide Bosio 2025-03-09 07:46:56 UTC
# Update
Spec URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08743523-hexcurse/hexcurse.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08743523-hexcurse/hexcurse-1.60-1.fc43.src.rpm
A fast, lightweight console hex editor for viewing and editing files in hex.
Fedora Account System Username: brosion

Comment 5 Jakub Kadlčík 2025-03-31 06:45:06 UTC
Hello Davide,
my apologies for the delay, I had a lot of other things going on.

I really like the changes you did.


> Name:           hexcurse

Just for the record, I am saying out loud that I am not sure if we should name the package hexcurse or hexcurse-ng but since it provides /usr/bin/hexcurse I think we can keep the name hexcurse.


> Version:        1.60

Is there a reason why you packaged 1.60 instead of the latest 1.70.0? If possible, we always try to package the latest version. Also, looking at the tag history
https://github.com/prso/hexcurse-ng/tags
the version would be 1.60.0, not 1.60


> BuildRequires:  autoconf

The autoconf dependency must be really important because you listed it twice ;-)


> Issues:
> =======
> - The License field must be a valid SPDX expression.
>   Note: Not a valid SPDX expression 'GPLv2+'. It seems that you are using
>   the old Fedora license abbreviations. Try `license-fedora2spdx' for
>   converting it to SPDX.
>   See: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1

The fedora-review tool complains about this. Try using the license-fedora2spdx tool, it will tell you the correct licence name.


We also have some errors from rpmlint:


> hexcurse.x86_64: W: spurious-executable-perm /usr/share/doc/hexcurse/AUTHORS
> hexcurse.x86_64: W: spurious-executable-perm /usr/share/doc/hexcurse/ChangeLog
> hexcurse.x86_64: W: spurious-executable-perm /usr/share/doc/hexcurse/NEWS
> hexcurse.x86_64: W: spurious-executable-perm /usr/share/doc/hexcurse/README.md

For some reason, these files have an executable bit. When I install your package and do this:

    $ ls -l /usr/share/doc/hexcurse/README.md
    -rwxr-xr-x. 1 root root 2643 Feb 13 01:00 /usr/share/doc/hexcurse/README.md

It is there. However, I have no idea where it came from because the permissions in the upstream tarball are correct.


> hexcurse.src: W: no-url-tag

The spec file should have an URL attribute pointing to the upstream project:

    URL: https://github.com/prso/hexcurse-ng


> hexcurse.x86_64: W: incoherent-version-in-changelog 1.60 ['1.60-1.fc43', '1.60-1']

The version number in the changelog must contain also the release number. In your case 1.60-1



Otherwise, the package builds and installs correctly.

Comment 6 Davide Bosio 2025-04-01 12:18:33 UTC
# Update
I made a few changed based on your report.


Since a 1.70 already exists and it was published about a year ago, I updated to 1.80.0.


Added an URL to the upstream next-gen project.


Checked again all BuildRequires and versions of them, removed the duplicated one.


Fixed %changelog forgotten version manifest and added a couple of logs of past events.


Fixed some old build env configurations.


I found the right SPDX license and added it in the right place.


Theoretically this should be our last check, try to check it:


Spec URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08844218-hexcurse/hexcurse.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/08844218-hexcurse/hexcurse-1.80.0-1.fc43.src.rpm

Comment 7 Jakub Kadlčík 2025-04-04 10:50:45 UTC
Thank you for the update Davide,

The project licensing is quite messy. The licensecheck tool shows:


    GNU General Public License v2.0 or later [obsolete FSF postal address (Mass Ave)]
    ---------------------------------------------------------------------------------
    hexcurse-1.80.0-build/hexcurse-ng/include/hgetopt.h
    hexcurse-1.80.0-build/hexcurse-ng/src/getopt.c

    GNU General Public License v2.0 or later [obsolete FSF postal address (Temple Place)]
    -------------------------------------------------------------------------------------
    hexcurse-1.80.0-build/hexcurse-ng/include/hex.h
    hexcurse-1.80.0-build/hexcurse-ng/src/acceptch.c
    hexcurse-1.80.0-build/hexcurse-ng/src/file.c
    hexcurse-1.80.0-build/hexcurse-ng/src/hexcurse.c
    hexcurse-1.80.0-build/hexcurse-ng/src/llist.c
    hexcurse-1.80.0-build/hexcurse-ng/src/screen.c
    hexcurse-1.80.0-build/hexcurse-ng/src/stack.c

    GNU General Public License v3.0 or later
    ----------------------------------------
    hexcurse-1.80.0-build/hexcurse-ng/config.guess
    hexcurse-1.80.0-build/hexcurse-ng/config.sub

    GNU General Public License, Version 2
    -------------------------------------
    hexcurse-1.80.0-build/hexcurse-ng/COPYING

    GNU Library General Public License, Version 2.0
    -----------------------------------------------
    hexcurse-1.80.0-build/hexcurse-ng/LICENSE.txt


So I think the License should look something like this (with the comment):

    # The project is available under the LGPL-2.0-or-later license except for the
    # following files.
    # These files are licensed under GPL-2.0-or-later:
        include/hex.h
        src/acceptch.c
        src/file.c
        src/hexcurse.c
        src/llist.c
        src/screen.c
        src/stack.c
    # These files are licensed under GPL-3.0-or-later:
        config.guess
        config.sub
    License: LGPL-2.0-or-later AND GPL-2.0-or-later AND GPL-3.0-or-later



Also, we still have the problem with executable documentation files. I am not sure why it happens. If you don't want to debug the issue, it's fine to just chmod -x on those files in the %build or %install section.
You can see the errors for example here - https://download.copr.fedorainfracloud.org/results/frostyx/fedora-review-2346284-hexcurse/fedora-rawhide-x86_64/08860368-hexcurse/fedora-review/review.txt
If you want to reproduce them for yourself, you can simply enable fedora-review for your Copr project
https://frostyx.cz/posts/running-fedora-review-after-copr-build

Comment 8 Davide Bosio 2025-05-22 07:27:40 UTC
# Update

Hi jakub, it's been a lot of time since i haven't updated you.

In the meanwhile, my commits have been checked and marged into master of https://github.com/prso/hexcurse-ng,
unfortunately they are minor changes related to the whole project so no new version has been released, so i modified a little the spec file:
The source now relates to the GitHub's repo compressed image and after unpacking it, the spec cd into the right directory.

I don't know if this is an "elegant" way to do it but since the last release doesn't even compile, I think it's the only way.

Spec URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/09071756-hexcurse/hexcurse.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/brosion/hexcurse/fedora-rawhide-x86_64/09071756-hexcurse/hexcurse-1.70.0-1.fc43.src.rpm

Comment 9 Fedora Review Service 2025-05-22 10:46:46 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9072358
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346284-hexcurse/fedora-rawhide-x86_64/09072358-hexcurse/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 10 Jakub Kadlčík 2025-05-29 13:58:57 UTC
Thank you for the update. Final touches, and its done.

> %doc README.md NEWS ChangeLog INSTALL AUTHORS

Can you please remove the INSTALL file from this line?

> %changelog
> %changelog
> * Sun May 18 2025 Davide Bosio <bosiodavide2006> - 1.70.0-1

There should be only one %changelog line

Other than that, I think we are good to go.

Comment 12 Fedora Review Service 2025-05-30 11:16:46 UTC
Created attachment 2092221 [details]
The .spec file difference from Copr build 9072358 to 9104307

Comment 13 Fedora Review Service 2025-05-30 11:16:49 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9104307
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346284-hexcurse/fedora-rawhide-x86_64/09104307-hexcurse/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.


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