I'm thinking of taking this review. I can see a couple issues though:

1- Some places in your spec you account for "if" upstream ever makes a release with how the project version shows, and some places you don't. For example:

Release: %autorelease -s %{date}git%{shortcommit}

%autosetup -p1 -n %{name}-%{commit}

(If/when the project makes an official release, those wont work)

2- The license "ASL 2.0" is the legacy naming and should be changed. See: https://docs.fedoraproject.org/en-US/legal/allowed-licenses/ for the SPDX expression.

3- `%{_libdir}/cmake/GKlib/` should be in the devel subpackage

4- The Apache 2.0 license is not applied properly and most likely wouldn't be allowed in fedora as-is. They only put the header inside the LICENSE.txt, while that should go in the source file headers. The Apache 2.0 license itself should be in the LICENSE.txt, along with, optionally, a NOTICE file pointing to it. As-is, the Apache 2.0 license is not enforced. So this is a big no-no and should get fixed upstream.

Oh sorry, I forgot to note in point #4 of my last post to the text at the bottom of https://www.apache.org/licenses/LICENSE-2.0 of how the ASL 2.0 license has to be applied.

Hi Steve, I was around the discussions of the project and if you take it, just want to point out a few major points that I want to raise about this:
- The Apache-2.0 license is not the only license of the project. See issue [1] where this was highlighted
- Upstream is quite unresponsive and there were multiple forks and attempts to help [2-4]. I would suggest to have a copr repo to review that this and the metis update are actually compatible with each other
  @Antonio are you able to get in touch with upstream and get any communication channel established to actually be able to contribute much of these changes. I have offered my help to review the CMake parts and others in the community are eager to help.

Otherwise, I do have some general packaging suggestions:
- (non-blocking) consider using %forgemeta macros [5]. It abstracts much of the commit and such logic, and makes it much easier to move from a tagged to a commit snapshot
- Please avoid picking up a dependency like valgrind-devel
- Please make the `sed` operation a patch. Proper patch is to make it use GNUInstallDirs variables
- Please remove the following variables passed `CMAKE_VERBOSE_MAKEFILE`, `CMAKE_INSTALL_PREFIX`. These are handled by the %cmake macro
- Consider using the %conf section if you do not need to backport this to too old epels
- Be careful with the snippet
  ```
  %ifnarch %{?x86}
   -DNO_X86:BOOL=ON
  %endif
  ```
  whitespace/blank lines is important and it might not be handled well on i686 in that form
- The `PCRE_LDFLAGS=-lpcreposix` is very suspicious. Please elaborate on it. Likely upstream should do a `find_package(PCRE)` instead and specify if they want PCRE2
- `%undefine _ld_as_needed` is also suspicious
- Please abstract the soversion in a variable

[1]: https://github.com/KarypisLab/GKlib/issues/30
[2]: https://github.com/KarypisLab/GKlib/pull/34
[3]: https://github.com/KarypisLab/GKlib/pull/33
[4]: https://github.com/KarypisLab/GKlib/pull/28
[5]: example on my package https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec

(In reply to Cristian Le from comment #3)
> Hi Steve, I was around the discussions of the project and if you take it,
> just want to point out a few major points that I want to raise about this:
...
> - Upstream is quite unresponsive and there were multiple forks and attempts
> to help [2-4]. I would suggest to have a copr repo to review that this and
> the metis update are actually compatible with each other
> 
...
> - The `PCRE_LDFLAGS=-lpcreposix` is very suspicious. Please elaborate on it.

> - `%undefine _ld_as_needed` is also suspicious
> - Please abstract the soversion in a variable
> 
> [1]: https://github.com/KarypisLab/GKlib/issues/30
> [2]: https://github.com/KarypisLab/GKlib/pull/34
> [3]: https://github.com/KarypisLab/GKlib/pull/33
> [4]: https://github.com/KarypisLab/GKlib/pull/28
> [5]: example on my package
> https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec

Hi Cristian
Hi Steve

I think that, excepting all that is closely concerning to the packaging, these issues should be discussed/resolved with upstream.
All i want to do here is to make as best as possible the RPM of the code actually distributed.
I can't supersede upstream developers.

@Cristian
> [1]: https://github.com/KarypisLab/GKlib/issues/30
> [2]: https://github.com/KarypisLab/GKlib/pull/34
> [3]: https://github.com/KarypisLab/GKlib/pull/33
> [4]: https://github.com/KarypisLab/GKlib/pull/28

I did not understand if your changes have been already partially merged by upstream.

> Likely upstream should do a `find_package(PCRE)` instead and specify if they
> want PCRE2

`find_package(PCRE)` requires a PCRE CMake config file that does not exist; or am i wrong?

(In reply to Antonio T. sagitter from comment #4)
> 
> Hi Cristian
> Hi Steve
> 
> I think that, excepting all that is closely concerning to the packaging,
> these issues should be discussed/resolved with upstream.
> All i want to do here is to make as best as possible the RPM of the code
> actually distributed.
> I can't supersede upstream developers.

Most of it is patchable on our side, and there were some references of the patches already proposed. If you get stuck on any of them, let me know and I'll find some time to write a quick patch.

License should really get upstream to respond, but we can also make an executive decision to fix it for them since the state is rather clear.

But to clarify, fixing the license **IS** a blocker for review. On your side the simplest course of action is to move the content of that COPYING and put the actual license text file in there instead. If the full license is included in all headers, that can be skipped, but I have not checked the sources in a while.

> I did not understand if your changes have been already partially merged by
> upstream.

Not yet because upstream went silent on us again after that miraculous activity. @scivision might be interested in longer term maintenance, so I would pass the ball to them if they would be up for it.

I did ping upstream again to try and get some response at least https://github.com/KarypisLab/GKlib/issues/56

> > Likely upstream should do a `find_package(PCRE)` instead and specify if they
> > want PCRE2
> 
> `find_package(PCRE)` requires a PCRE CMake config file that does not exist;
> or am i wrong?

I should have said or equivalent. It has `.pc` file which can be used. For pcre2 it would be possible, but also it is not built with CMake: https://bugzilla.redhat.com/show_bug.cgi?id=2218125.

I have added some simple patches and fixed most of packaging lacks.

SPEC: https://download.copr.fedorainfracloud.org/results/sagitter/ForTesting/fedora-rawhide-x86_64/10129598-GKlib/GKlib.spec
SRPM: https://download.copr.fedorainfracloud.org/results/sagitter/ForTesting/fedora-rawhide-x86_64/10129598-GKlib/GKlib-5.1.1~20250705git6e79513-1.fc45.src.rpm

Cristian,
can you remake your patch for including the tests?

> I have added some simple patches and fixed most of packaging lacks.

Great. I can't look into it today because of CMake changes, but I'll try to get to it this week. In the meantime

> can you remake your patch for including the tests?

Which patch?

About the patches, can you clarify where they come from, if it's from a commit or otherwise. It would actually be good if you can submit those as individual PRs upstream as well (I should have done that initially, but I was too native at that time)

(In reply to Cristian Le from comment #7)
> > I have added some simple patches and fixed most of packaging lacks.
> 
> Great. I can't look into it today because of CMake changes, but I'll try to
> get to it this week. In the meantime
> 
> > can you remake your patch for including the tests?
> 
> Which patch?

https://github.com/KarypisLab/GKlib/pull/33/commits/c8ead82087a5c7931ce6962f394ca41f83922351

(In reply to Antonio T. sagitter from comment #8)
> (In reply to Cristian Le from comment #7)
> > > I have added some simple patches and fixed most of packaging lacks.
> > 
> > Great. I can't look into it today because of CMake changes, but I'll try to
> > get to it this week. In the meantime
> > 
> > > can you remake your patch for including the tests?
> > 
> > Which patch?
> 
> https://github.com/KarypisLab/GKlib/pull/33/commits/
> c8ead82087a5c7931ce6962f394ca41f83922351

Upstream has moved it to `apps` and gated it by `GKLIB_BUILD_APPS`. I have no idea what they want to do with that because it is not being installed. My suggestion is to pass `GKLIB_BUILD_APPS=OFF`.