Bug 2246561 - Review Request: helix - A post-modern modal text editor written in Rust
Summary: Review Request: helix - A post-modern modal text editor written in Rust
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://helix-editor.com/
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-10-27 13:36 UTC by blinxen
Modified: 2023-11-09 10:06 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-11-09 10:06:33 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6581577 to 6589210 (11.41 KB, patch)
2023-11-01 21:48 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6589210 to 6596401 (20.86 KB, patch)
2023-11-04 01:11 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6596401 to 6613596 (5.10 KB, patch)
2023-11-08 18:51 UTC, Fedora Review Service
no flags Details | Diff

Description blinxen 2023-10-27 13:36:59 UTC
Spec URL: https://blinxen.fedorapeople.org/helix/helix.spec
SRPM URL: https://blinxen.fedorapeople.org/helix/helix-23.10-1.fc40.src.rpm

Description:
A Kakoune / Neovim inspired editor, written in Rust.

Fedora Account System Username: blinxen

Comment 1 blinxen 2023-10-27 13:37:02 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=108176067

Comment 2 Fedora Review Service 2023-10-27 23:21:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6575033
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246561-helix/srpm-builds/06575033/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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 3 blinxen 2023-10-29 15:38:29 UTC
[fedora-review-service-build]

Comment 4 Fabio Valentini 2023-10-30 13:18:43 UTC
The packaging itself looks fine, but looking at the sources themselves, they are *a mess*:

- The "runtime/themes/*" contents are covered under different licenses - are they copied from different projects?

- The "runtime/grammars/*" contents appear to be bundled copies of tree-sitter-* crates and / or other tree-sitter subprojects, and they are covered under different licenses. Notably, the "hcl" grammar contains lots of example / data files that are covered under various licenses. I don't think they are included in the final package, but that needs to be verified.

Especially bundling all the tree-sitter stuff (including some tree-sitter sources / headers themselves) looks very suspicious to me. Shouldn't they be part of the tree-sitter package? Or do they work in a way where every package that uses tree-sitter grammars installs their own .so files? If this is the case, I think this package would at least need to specify that the tree-sitter grammars are indeed bundled (i.e. add something like "Provides: bundled(tree-sitter-astro) = 0.0.1" for every bundled grammar, and "Provides: bundled(tree-sitter) = x.y.z" for whatever version of tree-sitter that the sources are taken from).

Comment 5 Fedora Review Service 2023-10-30 15:22:07 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6581577
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246561-helix/fedora-rawhide-x86_64/06581577-helix/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 6 blinxen 2023-10-30 16:00:45 UTC
> - The "runtime/themes/*" contents are covered under different licenses - are they copied from different projects?

According to themes README [1], some themes are user submissions. The others are probably ports from other projects. Do their license files also have to be installed?

> I don't think they are included in the final package, but that needs to be verified.

Exactly, the source files for the grammars are not included. They are only added to the helix tarball so we are able to build and add them in the final package. Or else the user has to do `hx --grammar fetch` and `hx --grammar build` after the initial installation (which I would like to prevent).

> Especially bundling all the tree-sitter stuff (including some tree-sitter sources / headers themselves) looks very suspicious to me

Well that goes back to how helix works. Language-specific settings and settings for language servers are configured in `languages.toml`. In that file, the grammars + their sources are also declared. Helix builds the specified version of the tree-sitter parsers and puts it in the configured runtime directory. The build step for the parsers can be skipped but helix needs the ".so" files to be in the runtime directory. So the only thing we can do here is either bundle them or package each tree-sitter parser and add a symlink of all ".so" files to the runtime directory.

[1] https://github.com/helix-editor/helix/blob/master/runtime/themes/README.md
[2] https://tree-sitter.github.io/tree-sitter/using-parsers

Comment 7 Fabio Valentini 2023-10-31 12:36:11 UTC
(In reply to blinxen from comment #6)
> > - The "runtime/themes/*" contents are covered under different licenses - are they copied from different projects?
> 
> According to themes README [1], some themes are user submissions. The others
> are probably ports from other projects. Do their license files also have to
> be installed?

Yes - the MIT license requires license text to be included with distributed sources.
But helix sources don't include the license texts for these themes:

./everforest_dark.toml: *No copyright* MIT License
./everforest_light.toml: *No copyright* MIT License
./serika-dark.toml: *No copyright* MIT License
./serika-light.toml: *No copyright* MIT License
./sonokai.toml: *No copyright* MIT License

> > I don't think they are included in the final package, but that needs to be verified.
> 
> Exactly, the source files for the grammars are not included. They are only
> added to the helix tarball so we are able to build and add them in the final
> package. Or else the user has to do `hx --grammar fetch` and `hx --grammar
> build` after the initial installation (which I would like to prevent).

I meant the contents of the "runtime/grammars/sources/hcl/example/" directory.
They are from other projects, and are covered under various licenses.
But I assume these are example files that are not included when building the "hcl" grammar?
In that case, their licenses don't need to be taken into account for the helix package.

> > Especially bundling all the tree-sitter stuff (including some tree-sitter sources / headers themselves) looks very suspicious to me
> 
> Well that goes back to how helix works. Language-specific settings and
> settings for language servers are configured in `languages.toml`. In that
> file, the grammars + their sources are also declared. Helix builds the
> specified version of the tree-sitter parsers and puts it in the configured
> runtime directory. The build step for the parsers can be skipped but helix
> needs the ".so" files to be in the runtime directory. So the only thing we
> can do here is either bundle them or package each tree-sitter parser and add
> a symlink of all ".so" files to the runtime directory.
> 
> [1]
> https://github.com/helix-editor/helix/blob/master/runtime/themes/README.md
> [2] https://tree-sitter.github.io/tree-sitter/using-parsers

Ok, if that's how helix + tree-sitter works, then we'll have to deal with that.

I don't think unbundling all the tree-sitter grammars as separate projects (163 projects?) would be a reasonable solution, so building them from the bundled sources is OK. However, they will need to be specified as bundled dependencies, and their licenses taken into account in the License tag for the package.

We will need something like "Provides: bundled(tree-sitter-astro) = 0.0.1" for all bundled grammars.

I'm not sure how to best deal with the licenses for all the tree sitter grammars - it looks like they all only depend on the "tree-sitter" crate that provides bindings for the tree-sitter C library, but I haven't checked *all of them* if they also depend on other Rust crates. But at least the tree-sitter crate is already a dependency of helix-core, so it's already taken into account by the %cargo_license macro.

However, not all included tree-sitter grammars ship their own license files, which might be a problem for redistributing them.

I also noticed that some grammars require "tree-sitter = 0.19", which is an unsatisfiable dependency in Fedora - are those grammars just not built?

Comment 8 blinxen 2023-10-31 15:11:59 UTC
> Yes - the MIT license requires license text to be included with distributed sources.

I will look into this and create a PR upstream.

> I meant the contents of the "runtime/grammars/sources/hcl/example/" directory.

Ah, yeah they are not used during compilation. This can be checked by grepping (`rg example --glob='!example' --glob='!.git'`) the source. 

> I don't think unbundling all the tree-sitter grammars as separate projects (163 projects?) would be a reasonable solution, so building them from the bundled sources is OK. However, they will need to be specified as bundled dependencies, and their licenses taken into account in the License tag for the package.

:+1:

> I'm not sure how to best deal with the licenses for all the tree sitter grammars - it looks like they all only depend on the "tree-sitter" crate that provides bindings for the tree-sitter C library, but I haven't checked *all of them* if they also depend on other Rust crates

I created a file (https://blinxen.fedorapeople.org/helix/cargo_toml_of_grammars.toml) that contains all `Cargo.toml` files of all grammars. There is only one crate that requires other dependencies than `tree-sitter`. The additional dependencies are `encoding_rs` and `encoding_rs_io`. Both licenses should be covered in the package license tag.

> But at least the tree-sitter crate is already a dependency of helix-core, so it's already taken into account by the %cargo_license macro.

:+1:

> However, not all included tree-sitter grammars ship their own license files, which might be a problem for redistributing them.

Did you check this manually? Do you have an example grammar?

> I also noticed that some grammars require "tree-sitter = 0.19", which is an unsatisfiable dependency in Fedora - are those grammars just not built?

Hm, yeah that's weird. They are built but I am unsure how. Will have to investigate how this actually works. See https://blinxen.fedorapeople.org/helix/tree-sitter-versions-unique for all `tree-sitter` versions that are used.

Comment 9 Fabio Valentini 2023-10-31 15:29:32 UTC
(In reply to blinxen from comment #8)
> > Yes - the MIT license requires license text to be included with distributed sources.
> 
> I will look into this and create a PR upstream.

Great, thanks!

> > However, not all included tree-sitter grammars ship their own license files, which might be a problem for redistributing them.
> 
> Did you check this manually? Do you have an example grammar?

I just did a spot-check, and the lexicographically first one already doesn't contain a license file (tree-sitter-astro, in runtime/grammars/sources/astro). There are others that are missing license files as well.

> > I also noticed that some grammars require "tree-sitter = 0.19", which is an unsatisfiable dependency in Fedora - are those grammars just not built?
> 
> Hm, yeah that's weird. They are built but I am unsure how. Will have to
> investigate how this actually works. See
> https://blinxen.fedorapeople.org/helix/tree-sitter-versions-unique for all
> `tree-sitter` versions that are used.

I think I know what's happening. Those grammars are built as C objects by the helix build scripts, not as Rust projects. So the dependencies specified in Cargo.toml don't even factor into it, from what I can tell. It also looks like the "tree-sitter-afl-fuzzer" (the only crate that has more than just the "tree-sitter" crate dependency) is not an actual tree-sitter grammar, but a subproject of the SSH client configuration grammar, so it isn't used at all.

Comment 10 blinxen 2023-10-31 19:10:40 UTC
> > I will look into this and create a PR upstream.

> Great, thanks!

Here is the PR --> https://github.com/helix-editor/helix/pull/8684

I assume I have to install the license files for the themes too. Is that correct?

> I just did a spot-check, and the lexicographically first one already doesn't contain a license file (tree-sitter-astro, in runtime/grammars/sources/astro). There are others that are missing license files as well.

I did some python magic and got the following list of grammars that don't have a license file.

```
sql
llvm
eex
clojure
twig
forth
comment
vala
beancount
uxntal
ungrammar
purescript
vhs
perl
hare
astro
pod
lean
unison
smithy
rego
```

Some of these do have LICENSE files but the commit that is used in `languages.toml` does not have it --> They (the license files) were added in a later commit. Besides adding "bundled(...) = ..", do I also have to install the license files for the grammars?

> I think I know what's happening. Those grammars are built as C objects by the helix build scripts, not as Rust projects. So the dependencies specified in Cargo.toml don't even factor into it, from what I can tell. It also looks like the "tree-sitter-afl-fuzzer" (the only crate that has more than just the "tree-sitter" crate dependency) is not an actual tree-sitter grammar, but a subproject of the SSH client configuration grammar, so it isn't  used at all.

That sounds about right, here is the answer from upstream:

```
Yes helix does not require tree sitter whole building grammar it just invokes the c compiler. I need to check tough some headers may be needed but I am not sure about that.

For a package manager I would generally recommend to invoke the c compiler yourself to properly handle cross compilation and make sure all flags are respected (the sourcecode for every grammsr is provided in the source archive of every release).
I don't think tree sitter versions matter much for the grammars. Tbh I didn't even know they could specify a specific TS version (grammars are just one or Teo c source files that my or may not be auto generated).
A slight tangent: Some distros (like alpine) ship the grammars as separate packages and share them across editors like nvim.
Whole this may seem attractive in theory it's incompatible with the reality of TS. The queries that ship with the editor are specific to one specific version of the grammar and will (sometimes subtley) break when used with a different version. Helix should only be packaged with the exact grammar commits that we pine d in our languages.toml/ship in the source archive
```

Comment 11 Fabio Valentini 2023-10-31 22:42:26 UTC
(In reply to blinxen from comment #10)
> > > I will look into this and create a PR upstream.
> 
> > Great, thanks!
> 
> Here is the PR --> https://github.com/helix-editor/helix/pull/8684

Thanks!

> I assume I have to install the license files for the themes too. Is that
> correct?

Yes. The themes are getting installed, so the associated license file(s) should get installed too.

> > I just did a spot-check, and the lexicographically first one already doesn't contain a license file (tree-sitter-astro, in runtime/grammars/sources/astro). There are others that are missing license files as well.
> 
> I did some python magic and got the following list of grammars that don't
> have a license file.
> 
> ```
> sql
> llvm
> eex
> clojure
> twig
> forth
> comment
> vala
> beancount
> uxntal
> ungrammar
> purescript
> vhs
> perl
> hare
> astro
> pod
> lean
> unison
> smithy
> rego
> ```
> 
> Some of these do have LICENSE files but the commit that is used in
> `languages.toml` does not have it --> They (the license files) were added in
> a later commit. Besides adding "bundled(...) = ..", do I also have to
> install the license files for the grammars?

These projects are bundled and get build + shipped by the helix package, so yes, I think so.
You might need to rename the files to avoid name clashes with the %license macro.

For example, do something like "cp -p runtime/grammars/foo/LICENSE -> LICENSE-tree-sitter-foo" for all grammar modules, and then do "%license LICENSE-tree-sitter-*" in %files.

> > I think I know what's happening. Those grammars are built as C objects by the helix build scripts, not as Rust projects. So the dependencies specified in Cargo.toml don't even factor into it, from what I can tell. It also looks like the "tree-sitter-afl-fuzzer" (the only crate that has more than just the "tree-sitter" crate dependency) is not an actual tree-sitter grammar, but a subproject of the SSH client configuration grammar, so it isn't  used at all.
> 
> That sounds about right, here is the answer from upstream:
> 
> ```
> Yes helix does not require tree sitter whole building grammar it just
> invokes the c compiler. I need to check tough some headers may be needed but
> I am not sure about that.
> 
> For a package manager I would generally recommend to invoke the c compiler
> yourself to properly handle cross compilation and make sure all flags are
> respected (the sourcecode for every grammsr is provided in the source
> archive of every release).
> I don't think tree sitter versions matter much for the grammars. Tbh I
> didn't even know they could specify a specific TS version (grammars are just
> one or Teo c source files that my or may not be auto generated).
> A slight tangent: Some distros (like alpine) ship the grammars as separate
> packages and share them across editors like nvim.
> Whole this may seem attractive in theory it's incompatible with the reality
> of TS. The queries that ship with the editor are specific to one specific
> version of the grammar and will (sometimes subtley) break when used with a
> different version. Helix should only be packaged with the exact grammar
> commits that we pine d in our languages.toml/ship in the source archive
> ```

Sounds about right to me.
That makes it even more clear that packaging all the tree-sitter grammars separately is unrealistic. :)

Comment 12 blinxen 2023-11-01 21:19:32 UTC
> Yes. The themes are getting installed, so the associated license file(s) should get installed too.

Theme license files are now being manually installed (The PR for the license files has been merged upstream :D).

> For example, do something like "cp -p runtime/grammars/foo/LICENSE -> LICENSE-tree-sitter-foo" for all grammar modules, and then do "%license LICENSE-tree-sitter-*" in %files.

This was a bit tricky. There are some grammars that already contained license files but helix uses an older version (commit SHA) for them, so the files are missing. I manually added those and created https://github.com/helix-editor/helix/pull/8691. There is a small subset of grammars that doesn't have license files. I filed PRs for them and manually added the license files from my forks. I hope that is OK.
Since helix uses commits SHAs to download the grammars from GitHub, I can't really define a version for the bundled provides so I omitted that (should be OK according to the packaging guidelines).

Here are the updated files:
Spec URL: https://blinxen.fedorapeople.org/helix/helix.spec
SRPM URL: https://blinxen.fedorapeople.org/helix/helix-23.10-1.fc40.src.rpm

Comment 13 Fedora Review Service 2023-11-01 21:48:10 UTC
Created attachment 1996642 [details]
The .spec file difference from Copr build 6581577 to 6589210

Comment 14 Fedora Review Service 2023-11-01 21:48:13 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6589210
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246561-helix/fedora-rawhide-x86_64/06589210-helix/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 15 Fabio Valentini 2023-11-03 22:16:20 UTC
(In reply to blinxen from comment #12)
> > Yes. The themes are getting installed, so the associated license file(s) should get installed too.
> 
> Theme license files are now being manually installed (The PR for the license
> files has been merged upstream :D).

Great, thanks!

> > For example, do something like "cp -p runtime/grammars/foo/LICENSE -> LICENSE-tree-sitter-foo" for all grammar modules, and then do "%license LICENSE-tree-sitter-*" in %files.
> 
> This was a bit tricky. There are some grammars that already contained
> license files but helix uses an older version (commit SHA) for them, so the
> files are missing. I manually added those and created
> https://github.com/helix-editor/helix/pull/8691.

Thanks!

> There is a small subset of
> grammars that doesn't have license files. I filed PRs for them and manually
> added the license files from my forks. I hope that is OK.

It's OK in the short-term, but please switch back to upstream sources as soon as the projects merge your PRs.

> Since helix uses commits SHAs to download the grammars from GitHub, I can't
> really define a version for the bundled provides so I omitted that (should
> be OK according to the packaging guidelines).

It's OK, but I think you might be able to use pseudo-snapshot versioning.
For example, if the snapshot for commit 1a2b3cd happened after version 1.2.3 was released,
you could add something like "Provides: bundled(...) = 1.2.3^git1a2b3cd".
That would make identifying the versions a little bit easier, but it's not required by the guidelines, so I won't require it either. :)

> Here are the updated files:
> Spec URL: https://blinxen.fedorapeople.org/helix/helix.spec
> SRPM URL: https://blinxen.fedorapeople.org/helix/helix-23.10-1.fc40.src.rpm

Thanks! I'll do a full review now.

PS: I might ask somebody else for a second opinion later since this is a rather complicated package with many components, and I'm not sure I'm getting 100% of the things right here.

Comment 16 Fabio Valentini 2023-11-03 22:28:33 UTC
A few minor things for now (still going over the package):

1. Looks like there's a file "/usr/share/licenses/helix/LICENSE-tree-sitter-." getting installed, this looks like a mistake?

2. It seems that the grammar plugins that are getting compiled with "cc" link libm and libgcc_s even if they don't have to:

helix.x86_64: W: unused-direct-shlib-dependency /usr/lib64/helix/runtime/grammars/astro.so /lib64/libm.so.6
helix.x86_64: W: unused-direct-shlib-dependency /usr/lib64/helix/runtime/grammars/astro.so /lib64/libgcc_s.so.1

This might or might not be a false positive - but usually plugins don't need to be linked against libraries directly, as the "host" program that loads the plugins is expected to do that. But since the "host" program is Rust here ... not sure about that. helix *does* link against both libm.so and libgcc_s.so, so it should be fine if the plugins didn't do that themselves - but I'm not sure the "cc" crate can be configured to compile objects like that. So I don't consider this to be a blocker, just a warning for you (or maybe upstream).

3. There appear to be some files that have executable permissions that should not have them:

helix.x86_64: E: script-without-shebang /usr/share/licenses/helix/LICENSE-tree-sitter-svelte

4. These shell completion scripts have shebangs but don't need them (I think)?

helix.x86_64: E: non-executable-script /usr/share/bash-completion/completions/hx 644 /usr/bin/env bash
helix.x86_64: E: non-executable-script /usr/share/fish/vendor_completions.d/hx.fish 644 /usr/bin/env fish

Comment 17 Fabio Valentini 2023-11-03 22:44:40 UTC
A few more minor issues with the spec file:

1. Inconsistent indentation: The tags from Source100 up to the Provides for bundled tree-sitter grammars are indented inconsistently from the rest of the spec file, which uses 16 columns of indentation. For the bundled Provides, you can just tweak the command that you used to generate them (i.e. add the appropriate amount of spaces in the command at the right position).

2. The list of bundled Provides for tree-sitter grammars is unsorted. I suspect that if you regenerate this list with the next version, it will have a different order. I suggest to add something like "| sort" at the end of command that you used to generate the list.

3. The "%{_libdir}/helix/" directory ends up unowned.
The package only owns "%{runtime_directory_path}", which is a subdirectory of "%{_libdir}/helix/", but it doesn't own the parent directory.
Please add something like "%dir %{_libdir}/helix" to the %files list as well.

4. You install a .desktop file for Helix, but it is not validated, which is a mandatory requirement.
You will need to add "BuildRequires: desktop-file-utils" (which is already there, because you use desktop-file-install),
and run something like this in %check (possibly unconditionally, whether %bcond check is enabled or not), similar to this:

"""
%check
desktop-file-validate %{buildroot}/%{_datadir}/applications/Helix.desktop
%if %{with check}
# Grammars are already built
export HELIX_DISABLE_AUTO_GRAMMAR_BUILD=true
%cargo_test
%{buildroot}%{_bindir}/%{binary_name} --health
%endif
"""

5. The other steps for preparing license files for installation all happen in %prep, but this one happens in %install, even though it does not *install* anything, just moves things around in the build directory:

> # Rename license files for tree-sitter grammars so they can be installed

I think it would make sense to move that to %prep as well.

Comment 18 blinxen 2023-11-04 00:31:38 UTC
> PS: I might ask somebody else for a second opinion later since this is a rather complicated package with many components, and I'm not sure I'm getting 100% of the things right here.

Sure, no problem :D

> It's OK in the short-term, but please switch back to upstream sources as soon as the projects merge your PRs.

I am already doing that :D I am also creating PRs [1] for upstream to use the newest version that includes the license commits.

> 1. Looks like there's a file "/usr/share/licenses/helix/LICENSE-tree-sitter-." getting installed, this looks like a mistake?

Should be fixed now.

> 2. It seems that the grammar plugins that are getting compiled with "cc" link libm and libgcc_s even if they don't have to:

Will report this upstream.

> 3. There appear to be some files that have executable permissions that should not have them:

Should be fixed now

> 4. These shell completion scripts have shebangs but don't need them (I think)?

I can report this upstream but it seems that this is not required but does not really do any damage if kept.

> 1. Inconsistent indentation: The tags from Source100 up to the Provides for bundled tree-sitter grammars are indented inconsistently from the rest of the spec file, which uses 16 columns of indentation. For the bundled Provides, you can just tweak the command that you used to generate them (i.e. add the appropriate amount of spaces in the command at the right position).

Should be fixed now

> 4. You install a .desktop file for Helix, but it is not validated, which is a mandatory requirement.

Desktop file validation has been added

> 5. The other steps for preparing license files for installation all happen in %prep, but this one happens in %install, even though it does not *install* anything, just moves things around in the build directory:

Makes sense

https://github.com/helix-editor/helix/pull/8691

Comment 19 blinxen 2023-11-04 00:32:09 UTC
Here are the updated files:
Spec URL: https://blinxen.fedorapeople.org/helix/helix.spec
SRPM URL: https://blinxen.fedorapeople.org/helix/helix-23.10-1.fc40.src.rpm

Comment 20 Fedora Review Service 2023-11-04 01:11:47 UTC
Created attachment 1997059 [details]
The .spec file difference from Copr build 6589210 to 6596401

Comment 21 Fedora Review Service 2023-11-04 01:11:50 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6596401
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246561-helix/fedora-rawhide-x86_64/06596401-helix/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 22 Fabio Valentini 2023-11-08 13:20:59 UTC
Thank you, the package looks pretty good to me now.
It's a pretty complicated package, so I'll ask for a second opinion or quick review.

Comment 23 Neal Gompa 2023-11-08 13:30:10 UTC
Looking over the spec:

> BuildRequires:  git
>
> # Required to allow users to fetch and build grammars
> Requires:       git

This should be "git-core" to avoid pulling in unnecessary stuff

I would also recommend adding an AppStream metainfo file since it does have a desktop file, so that it can be easily found in software centers.

Comment 24 Seth Maurice-Brant 2023-11-08 13:59:52 UTC
This is quite a big package, and for the most part it does look good to me, building locally works fine.

However there are a couple of things that may be worth considering.

- There is no man page for hx, which is what most users will be running, maybe add an alias to the helix manpage?
- rpmlint goes pretty crazy, but most of it seems like it can be ignored.
- helix.spec has permission 600, this seems a bit odd, is there a specific reason for that?
- fedora-review believes there is an uncommitted change,might be worth checking that out.
- Other minor issues I found have been discussed already

===

Other than couple of small problems, I believe this is in good working order.

I can't wait to replace my copr version of helix with an official one. 

Good luck and congratulations on getting this far!

Comment 25 Fabio Valentini 2023-11-08 14:02:59 UTC
(In reply to Seth Maurice-Brant from comment #24)

> - helix.spec has permission 600, this seems a bit odd, is there a specific
> reason for that?

This is a bug in rpmautospec that should be fixed with the next version.
It can be ignored, the actual spec file should have correct permissions.

> - fedora-review believes there is an uncommitted change,might be worth
> checking that out.

Another artifact of using rpmautospec for a package that's not in dist-git yet.

Comment 26 blinxen 2023-11-08 18:25:27 UTC
> This should be "git-core" to avoid pulling in unnecessary stuff

Thanks for the tip!!

> I would also recommend adding an AppStream metainfo file since it does have a desktop file, so that it can be easily found in software centers.

Added the appdata.xml file from upstream

Links to the updated files:
Spec URL: https://blinxen.fedorapeople.org/helix/helix.spec
SRPM URL: https://blinxen.fedorapeople.org/helix/helix-23.10-1.fc40.src.rpm

Comment 27 Fedora Review Service 2023-11-08 18:51:59 UTC
Created attachment 1997909 [details]
The .spec file difference from Copr build 6596401 to 6613596

Comment 28 Fedora Review Service 2023-11-08 18:52:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6613596
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246561-helix/fedora-rawhide-x86_64/06613596-helix/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 29 Fabio Valentini 2023-11-08 20:57:30 UTC
Thank you all. Package looks good to me. Full review below.

=======================================================================

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

===== MUST items =====

C/C++:
[-]: Provides: bundled(gnulib) in place as required.
[x]: Package does not contain kernel modules.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: The License field must be a valid SPDX expression.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[-]: Spec file according to URL is the same as in SRPM.
[x]: Rpmlint is run on debuginfo package(s).
[x]: Rpmlint is run on all installed packages.
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Checking: helix-23.10-1.fc40.x86_64.rpm
          helix-debuginfo-23.10-1.fc40.x86_64.rpm
          helix-debugsource-23.10-1.fc40.x86_64.rpm
          helix-23.10-1.fc40.src.rpm
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmp7haz8vh3')]
checks: 31, packages: 4

helix.x86_64: E: zero-length /usr/lib64/helix/runtime/queries/bibtex/locals.scm
helix.x86_64: E: zero-length /usr/lib64/helix/runtime/queries/bibtex/tags.scm

These look harmless.

helix.spec:83: W: unversioned-explicit-provides bundled(*)

This is OK if there's no real version to assign.

helix.src: W: strange-permission helix.spec 600

This is an artefact of a bug in rpmautospec.

helix.x86_64: E: non-executable-script /usr/share/bash-completion/completions/hx 644 /usr/bin/env bash
helix.x86_64: E: non-executable-script /usr/share/fish/vendor_completions.d/hx.fish 644 /usr/bin/env fish

This is OK.

helix.x86_64: W: no-manual-page-for-binary hx

This is OK too.

helix-debugsource.x86_64: E: files-duplicated-waste 978826

This is OK. Deduplicating these files is not really feasible.

 4 packages and 0 specfiles checked; 5 errors, 208 warnings, 5 badness; has taken 15.5 s 

Rpmlint (debuginfo)
-------------------
Checking: helix-debuginfo-23.10-1.fc40.x86_64.rpm
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmp6cbfxkfg')]
checks: 31, packages: 1

 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 5.2 s 

Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 3

helix.x86_64: E: zero-length /usr/lib64/helix/runtime/queries/bibtex/locals.scm
helix.x86_64: E: zero-length /usr/lib64/helix/runtime/queries/bibtex/tags.scm

These are OK. These files are empty on purpose, I assume.

helix.x86_64: W: unused-direct-shlib-dependency /usr/lib64/helix/runtime/grammars/*.so /lib64/libm.so.6
helix.x86_64: W: unused-direct-shlib-dependency /usr/lib64/helix/runtime/grammars/*.so /lib64/libgcc_s.so.1

This looks like an artifact of these files being compiled with the "cc" crate.
Not sure if it's possible to skip linking with libm and libgcc_s.

helix.x86_64: E: non-executable-script /usr/share/bash-completion/completions/hx 644 /usr/bin/env bash
helix.x86_64: E: non-executable-script /usr/share/fish/vendor_completions.d/hx.fish 644 /usr/bin/env fish

These are OK.

helix.x86_64: W: no-manual-page-for-binary hx

OK as well.

helix-debugsource.x86_64: E: files-duplicated-waste 978826

Not really feasible to deduplicate these files, as far as I can tell.

 3 packages and 0 specfiles checked; 5 errors, 510 warnings, 5 badness; has taken 35.9 s 

Unversioned so-files
--------------------
helix: /usr/lib64/helix/runtime/grammars/*.so

These are plugins, they are supposed to be unversioned.

Source checksums
----------------
https://raw.githubusercontent.com/blinxen/tree-sitter-smithy/main/LICENSE#/LICENSE-tree-sitter-smithy :
  CHECKSUM(SHA256) this package     : c3f5bd7f82227a941b6d619d0d3f637bf94cbe0e13e9fdd5e060995c63f3a760
  CHECKSUM(SHA256) upstream package : c3f5bd7f82227a941b6d619d0d3f637bf94cbe0e13e9fdd5e060995c63f3a760
https://raw.githubusercontent.com/blinxen/tree-sitter-vhs/main/LICENSE#/LICENSE-tree-sitter-vhs :
  CHECKSUM(SHA256) this package     : 3417ae865a9c4b305e83425268a4a06ee1f389c7f4738ba0d72ddba7e9bf8f50
  CHECKSUM(SHA256) upstream package : 3417ae865a9c4b305e83425268a4a06ee1f389c7f4738ba0d72ddba7e9bf8f50
https://raw.githubusercontent.com/blinxen/tree-sitter-pod/main/LICENSE#/LICENSE-tree-sitter-pod :
  CHECKSUM(SHA256) this package     : b759fd95c70b136ab34d4aba06c580c8048bd9b5e2991fe6914e461662ef0218
  CHECKSUM(SHA256) upstream package : b759fd95c70b136ab34d4aba06c580c8048bd9b5e2991fe6914e461662ef0218
https://raw.githubusercontent.com/blinxen/tree-sitter-eex/main/LICENSE#/LICENSE-tree-sitter-eex :
  CHECKSUM(SHA256) this package     : 4bfe090c69931f7dbb26546f6bac9117d881bede4600c8648de34ed1b4725308
  CHECKSUM(SHA256) upstream package : 4bfe090c69931f7dbb26546f6bac9117d881bede4600c8648de34ed1b4725308
https://raw.githubusercontent.com/Maskhjarna/tree-sitter-purescript/main/LICENSE#/LICENSE-tree-sitter-purescript :
  CHECKSUM(SHA256) this package     : e6708d41a3bb4757528ba7903f6cbef66bd8a453d341ac77500f02db1ad42ef8
  CHECKSUM(SHA256) upstream package : e6708d41a3bb4757528ba7903f6cbef66bd8a453d341ac77500f02db1ad42ef8
https://raw.githubusercontent.com/kylegoetz/tree-sitter-unison/master/LICENSE#/LICENSE-tree-sitter-unison :
  CHECKSUM(SHA256) this package     : cee7dcc846a5cc89b160ce3ddd23ec6fb606f1a90b7e1b4c815b28940a117968
  CHECKSUM(SHA256) upstream package : cee7dcc846a5cc89b160ce3ddd23ec6fb606f1a90b7e1b4c815b28940a117968
https://raw.githubusercontent.com/FallenAngel97/tree-sitter-rego/master/LICENSE#/LICENSE-tree-sitter-rego :
  CHECKSUM(SHA256) this package     : 7eeb3c3b1789d235ea1866fe1e0790233a9679b7ef8415ce93ab454775ebcb4f
  CHECKSUM(SHA256) upstream package : 7eeb3c3b1789d235ea1866fe1e0790233a9679b7ef8415ce93ab454775ebcb4f
https://raw.githubusercontent.com/AlexanderBrevig/tree-sitter-forth/main/LICENSE#/LICENSE-tree-sitter-forth :
  CHECKSUM(SHA256) this package     : 616f6dd042987a369f7962ee7a1f4fd9ab136c89f0677c6b2f4f1a5a34a30a2b
  CHECKSUM(SHA256) upstream package : 616f6dd042987a369f7962ee7a1f4fd9ab136c89f0677c6b2f4f1a5a34a30a2b
https://raw.githubusercontent.com/Jummit/tree-sitter-uxntal/master/LICENSE#/LICENSE-tree-sitter-uxntal :
  CHECKSUM(SHA256) this package     : dbda82e6febb4184d74ed80d1c85f658ec3f431b3cc40e7908ac92e09a485a0d
  CHECKSUM(SHA256) upstream package : dbda82e6febb4184d74ed80d1c85f658ec3f431b3cc40e7908ac92e09a485a0d
https://raw.githubusercontent.com/virchau13/tree-sitter-astro/master/LICENSE#/LICENSE-tree-sitter-astro :
  CHECKSUM(SHA256) this package     : 9ec95a90150fa19e2cee78faf49138fc480f6f7a2354006a0ab150c44239447e
  CHECKSUM(SHA256) upstream package : 9ec95a90150fa19e2cee78faf49138fc480f6f7a2354006a0ab150c44239447e
https://raw.githubusercontent.com/tree-sitter-perl/tree-sitter-perl/master/LICENSE#/LICENSE-tree-sitter-perl :
  CHECKSUM(SHA256) this package     : 19e2e0f2079ea1ce1576eb4ecc0575b33fe45b2b8e71f4aa589d6bedd1da4e0a
  CHECKSUM(SHA256) upstream package : 19e2e0f2079ea1ce1576eb4ecc0575b33fe45b2b8e71f4aa589d6bedd1da4e0a
https://raw.githubusercontent.com/Philipp-M/tree-sitter-ungrammar/main/LICENSE#/LICENSE-tree-sitter-ungrammar :
  CHECKSUM(SHA256) this package     : 6fe9d0b41450b02419e92daab3cef9413300a0cd3d9fc650d474804d646cdcec
  CHECKSUM(SHA256) upstream package : 6fe9d0b41450b02419e92daab3cef9413300a0cd3d9fc650d474804d646cdcec
https://raw.githubusercontent.com/polarmutex/tree-sitter-beancount/master/LICENSE#/LICENSE-tree-sitter-beancount :
  CHECKSUM(SHA256) this package     : 8976806a7d187d8e152d49146fae44d8a98bb15be05099e63843756c116efcf0
  CHECKSUM(SHA256) upstream package : 8976806a7d187d8e152d49146fae44d8a98bb15be05099e63843756c116efcf0
https://raw.githubusercontent.com/stsewd/tree-sitter-comment/master/LICENSE#/LICENSE-tree-sitter-comment :
  CHECKSUM(SHA256) this package     : 711e47fad3c2f298c4fdd21cc54b147ab1a73648b530c9940a0327e2cc905248
  CHECKSUM(SHA256) upstream package : 711e47fad3c2f298c4fdd21cc54b147ab1a73648b530c9940a0327e2cc905248
https://raw.githubusercontent.com/benwilliamgraham/tree-sitter-llvm/main/LICENSE#/LICENSE-tree-sitter-llvm :
  CHECKSUM(SHA256) this package     : a292ff39c53d0d2af3a0804ec291737508f1c888efb148f8f2bd3030e7cfdc48
  CHECKSUM(SHA256) upstream package : a292ff39c53d0d2af3a0804ec291737508f1c888efb148f8f2bd3030e7cfdc48
https://raw.githubusercontent.com/DerekStride/tree-sitter-sql/main/LICENSE#/LICENSE-tree-sitter-sql :
  CHECKSUM(SHA256) this package     : 3b3e4e4252d5d7d1c18e1257005f23242bf0580ad619204fd093c99b8c56748d
  CHECKSUM(SHA256) upstream package : 3b3e4e4252d5d7d1c18e1257005f23242bf0580ad619204fd093c99b8c56748d
https://raw.githubusercontent.com/helix-editor/helix/master/runtime/themes/licenses/sonokai.LICENSE :
  CHECKSUM(SHA256) this package     : 1578c1d9fe713f0ee479ff822b82baddbd2dfecf1f247a3f72bbdae5e0162e7e
  CHECKSUM(SHA256) upstream package : 1578c1d9fe713f0ee479ff822b82baddbd2dfecf1f247a3f72bbdae5e0162e7e
https://raw.githubusercontent.com/helix-editor/helix/master/runtime/themes/licenses/serika-syntax.LICENSE :
  CHECKSUM(SHA256) this package     : c0a2e97702d1e06a613e53fd7286b9fd2a260d93a29368d98e0a44bbd4f028eb
  CHECKSUM(SHA256) upstream package : c0a2e97702d1e06a613e53fd7286b9fd2a260d93a29368d98e0a44bbd4f028eb
https://raw.githubusercontent.com/helix-editor/helix/master/runtime/themes/licenses/everforest.LICENSE :
  CHECKSUM(SHA256) this package     : 6504b9e62794cb58b61c90ea5fe598274086119f2126b8d8245708b2bfcc0e36
  CHECKSUM(SHA256) upstream package : 6504b9e62794cb58b61c90ea5fe598274086119f2126b8d8245708b2bfcc0e36
https://github.com/helix-editor/helix/releases/download/23.10/helix-23.10-source.tar.xz :
  CHECKSUM(SHA256) this package     : 4e7bcac200b1a15bc9f196bdfd161e4e448dc670359349ae14c18ccc512153e8
  CHECKSUM(SHA256) upstream package : 4e7bcac200b1a15bc9f196bdfd161e4e448dc670359349ae14c18ccc512153e8

Requires
--------
helix (rpmlib, GLIBC filtered):
    gcc-c++
    git-core
    ld-linux-x86-64.so.2()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3)(64bit)
    libgcc_s.so.1(GCC_4.2.0)(64bit)
    libm.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

helix-debuginfo (rpmlib, GLIBC filtered):

helix-debugsource (rpmlib, GLIBC filtered):

Provides
--------
helix:
    application()
    application(Helix.desktop)
    bundled(tree-sitter-astro)
    bundled(tree-sitter-awk)
    bundled(tree-sitter-bash)
    bundled(tree-sitter-bass)
    bundled(tree-sitter-beancount)
    bundled(tree-sitter-bibtex)
    bundled(tree-sitter-bicep)
    bundled(tree-sitter-blueprint)
    bundled(tree-sitter-c)
    bundled(tree-sitter-c-sharp)
    bundled(tree-sitter-capnp)
    bundled(tree-sitter-cassette)
    bundled(tree-sitter-clojure)
    bundled(tree-sitter-cmake)
    bundled(tree-sitter-comment)
    bundled(tree-sitter-cpon)
    bundled(tree-sitter-cpp)
    bundled(tree-sitter-css)
    bundled(tree-sitter-cue)
    bundled(tree-sitter-d)
    bundled(tree-sitter-dart)
    bundled(tree-sitter-devicetree)
    bundled(tree-sitter-dhall)
    bundled(tree-sitter-diff)
    bundled(tree-sitter-dockerfile)
    bundled(tree-sitter-dot)
    bundled(tree-sitter-dtd)
    bundled(tree-sitter-edoc)
    bundled(tree-sitter-eex)
    bundled(tree-sitter-elixir)
    bundled(tree-sitter-elm)
    bundled(tree-sitter-elvish)
    bundled(tree-sitter-embedded-template)
    bundled(tree-sitter-erlang)
    bundled(tree-sitter-esdl)
    bundled(tree-sitter-fish)
    bundled(tree-sitter-forth)
    bundled(tree-sitter-fortran)
    bundled(tree-sitter-fsharp)
    bundled(tree-sitter-gas)
    bundled(tree-sitter-gdscript)
    bundled(tree-sitter-gemini)
    bundled(tree-sitter-git-config)
    bundled(tree-sitter-gitattributes)
    bundled(tree-sitter-gitcommit)
    bundled(tree-sitter-gitignore)
    bundled(tree-sitter-gleam)
    bundled(tree-sitter-glsl)
    bundled(tree-sitter-go)
    bundled(tree-sitter-go-mod)
    bundled(tree-sitter-go-template)
    bundled(tree-sitter-go-work)
    bundled(tree-sitter-godot-resource)
    bundled(tree-sitter-graphql)
    bundled(tree-sitter-hare)
    bundled(tree-sitter-haskell)
    bundled(tree-sitter-haskell-persistent)
    bundled(tree-sitter-hcl)
    bundled(tree-sitter-heex)
    bundled(tree-sitter-hosts)
    bundled(tree-sitter-html)
    bundled(tree-sitter-hurl)
    bundled(tree-sitter-iex)
    bundled(tree-sitter-ini)
    bundled(tree-sitter-java)
    bundled(tree-sitter-javascript)
    bundled(tree-sitter-jinja2)
    bundled(tree-sitter-jsdoc)
    bundled(tree-sitter-json)
    bundled(tree-sitter-json5)
    bundled(tree-sitter-jsonnet)
    bundled(tree-sitter-julia)
    bundled(tree-sitter-just)
    bundled(tree-sitter-kdl)
    bundled(tree-sitter-kotlin)
    bundled(tree-sitter-latex)
    bundled(tree-sitter-lean)
    bundled(tree-sitter-ledger)
    bundled(tree-sitter-llvm)
    bundled(tree-sitter-llvm-mir)
    bundled(tree-sitter-lua)
    bundled(tree-sitter-make)
    bundled(tree-sitter-markdoc)
    bundled(tree-sitter-markdown)
    bundled(tree-sitter-matlab)
    bundled(tree-sitter-mermaid)
    bundled(tree-sitter-meson)
    bundled(tree-sitter-nasm)
    bundled(tree-sitter-nickel)
    bundled(tree-sitter-nim)
    bundled(tree-sitter-nix)
    bundled(tree-sitter-nu)
    bundled(tree-sitter-ocaml)
    bundled(tree-sitter-odin)
    bundled(tree-sitter-opencl)
    bundled(tree-sitter-openscad)
    bundled(tree-sitter-org)
    bundled(tree-sitter-pascal)
    bundled(tree-sitter-passwd)
    bundled(tree-sitter-pem)
    bundled(tree-sitter-perl)
    bundled(tree-sitter-php)
    bundled(tree-sitter-po)
    bundled(tree-sitter-pod)
    bundled(tree-sitter-ponylang)
    bundled(tree-sitter-prisma)
    bundled(tree-sitter-protobuf)
    bundled(tree-sitter-prql)
    bundled(tree-sitter-purescript)
    bundled(tree-sitter-python)
    bundled(tree-sitter-qmljs)
    bundled(tree-sitter-r)
    bundled(tree-sitter-rebase)
    bundled(tree-sitter-regex)
    bundled(tree-sitter-rego)
    bundled(tree-sitter-rescript)
    bundled(tree-sitter-robot)
    bundled(tree-sitter-rst)
    bundled(tree-sitter-ruby)
    bundled(tree-sitter-rust)
    bundled(tree-sitter-scala)
    bundled(tree-sitter-scheme)
    bundled(tree-sitter-scss)
    bundled(tree-sitter-slint)
    bundled(tree-sitter-smithy)
    bundled(tree-sitter-sml)
    bundled(tree-sitter-solidity)
    bundled(tree-sitter-sql)
    bundled(tree-sitter-ssh-client-config)
    bundled(tree-sitter-strace)
    bundled(tree-sitter-svelte)
    bundled(tree-sitter-sway)
    bundled(tree-sitter-swift)
    bundled(tree-sitter-t32)
    bundled(tree-sitter-tablegen)
    bundled(tree-sitter-task)
    bundled(tree-sitter-templ)
    bundled(tree-sitter-todotxt)
    bundled(tree-sitter-toml)
    bundled(tree-sitter-tsq)
    bundled(tree-sitter-twig)
    bundled(tree-sitter-typescript)
    bundled(tree-sitter-ungrammar)
    bundled(tree-sitter-unison)
    bundled(tree-sitter-uxntal)
    bundled(tree-sitter-v)
    bundled(tree-sitter-vala)
    bundled(tree-sitter-verilog)
    bundled(tree-sitter-vhdl)
    bundled(tree-sitter-vue)
    bundled(tree-sitter-wasm)
    bundled(tree-sitter-wgsl)
    bundled(tree-sitter-wit)
    bundled(tree-sitter-wren)
    bundled(tree-sitter-xit)
    bundled(tree-sitter-xml)
    bundled(tree-sitter-yaml)
    bundled(tree-sitter-yuck)
    bundled(tree-sitter-zig)
    helix
    helix(x86-64)
    mimehandler(application/x-shellscript)
    mimehandler(text/english)
    mimehandler(text/plain)
    mimehandler(text/x-c)
    mimehandler(text/x-c++)
    mimehandler(text/x-c++hdr)
    mimehandler(text/x-c++src)
    mimehandler(text/x-chdr)
    mimehandler(text/x-csrc)
    mimehandler(text/x-java)
    mimehandler(text/x-makefile)
    mimehandler(text/x-moc)
    mimehandler(text/x-pascal)
    mimehandler(text/x-tcl)
    mimehandler(text/x-tex)

helix-debuginfo:
    debuginfo(build-id)
    helix-debuginfo
    helix-debuginfo(x86-64)

helix-debugsource:
    helix-debugsource
    helix-debugsource(x86-64)

Diff spec file in url and in SRPM
---------------------------------
--- /home/deca/Downloads/2246561-helix/srpm/helix.spec	2023-11-08 13:33:03.997028791 +0100
+++ /home/deca/Downloads/2246561-helix/srpm-unpacked/helix.spec	2023-11-07 01:00:00.000000000 +0100
@@ -1,2 +1,12 @@
+## START: Set by rpmautospec
+## (rpmautospec version 0.3.5)
+## RPMAUTOSPEC: autorelease, autochangelog
+%define autorelease(e:s:pb:n) %{?-p:0.}%{lua:
+    release_number = 1;
+    base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}"));
+    print(release_number + base_release_number - 1);
+}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}}
+## END: Set by rpmautospec
+
 %bcond_without check
 
@@ -327,3 +337,4 @@
 
 %changelog
-%autochangelog
+* Tue Nov 07 2023 John Doe <packager> - 23.10-1
+- Uncommitted changes

This is an artifact of using "fedpkg srpm" instead of "rpmbuild -bs" for building the SRPM file.

Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24
Command line :/usr/bin/fedora-review -b 2246561
Buildroot used: fedora-rawhide-x86_64
Active plugins: C/C++, Shell-api, Generic
Disabled plugins: R, Python, Haskell, fonts, Perl, Java, Ocaml, SugarActivity, PHP
Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH

Comment 30 Neal Gompa 2023-11-08 22:20:27 UTC
> appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/Helix.appdata.xml

Please put this in %check instead of %install

Comment 31 blinxen 2023-11-09 08:05:42 UTC
> These are OK. These files are empty on purpose, I assume.

Exactly

> Not sure if it's possible to skip linking with libm and libgcc_s.

Will ask in the matrix room of helix, maybe the devs know more

> Please put this in %check instead of %install

Will do :+1:


Thanks for the review :D I know it was **a lot** of work.

Comment 32 Fedora Admin user for bugzilla script actions 2023-11-09 08:06:27 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/helix

Comment 33 Fedora Update System 2023-11-09 10:03:14 UTC
FEDORA-2023-18ec5df09f has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-18ec5df09f

Comment 34 Fedora Update System 2023-11-09 10:06:33 UTC
FEDORA-2023-18ec5df09f has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.


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