Spec URL: https://download.copr.fedorainfracloud.org/results/solomoncyj/rust/fedora-rawhide-x86_64/08175084-rust-tree-sitter-bash/rust-tree-sitter-bash.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/solomoncyj/rust/fedora-rawhide-x86_64/08175084-rust-tree-sitter-bash/rust-tree-sitter-bash-0.23.1-1.fc42.src.rpm
Description: Tree-sitter Language type, used by the library and by language implementations
Fedora Account System Username: solomoncyj

Reproducible: Always

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8175131
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2318053-rust-tree-sitter-bash/fedora-rawhide-x86_64/08175131-rust-tree-sitter-bash/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.

Can/should this be built directly from the source rather than the crate, in such a way that the same source package outputs subpackages for both the C and Rust bindings (and, potentially, other languages too, such as Python and JavaScript)?

I have suggested some RPM macros to help build C bindings (https://src.fedoraproject.org/rpms/tree-sitter/pull-request/1).  We could enhance that to cover Rust too, if someone interested in Rust could work out that part of the recipe.

(In reply to Peter Oliver from comment #3)
> Can/should this be built directly from the source rather than the crate, in
> such a way that the same source package outputs subpackages for both the C
> and Rust bindings (and, potentially, other languages too, such as Python and
> JavaScript)?

I don't think that is advisable because each packaging environment have their own workflow and even in this package the build artifacts for each one is different. Synchronizing the versions across a `rust-tree-sitter-bash` and `python-tree-sitter-bash` would indeed be advisable, but probably we can use a different tooling for that.

> I have suggested some RPM macros to help build C bindings
> (https://src.fedoraproject.org/rpms/tree-sitter/pull-request/1).  We could
> enhance that to cover Rust too, if someone interested in Rust could work out
> that part of the recipe.

Unfortunately those have a strong assumption that the packaging project is using Makefile, but that would be incompatible with building the rust or python package. Do you have example projects that could be using that?

[fedora-review-service-build]

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8206505
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2318053-rust-tree-sitter-bash/fedora-rawhide-x86_64/08206505-rust-tree-sitter-bash/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

Please know that there can be false-positives.

---
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.

(In reply to Cristian Le from comment #4)
> (In reply to Peter Oliver from comment #3)
> > Can/should this be built directly from the source rather than the crate, in
> > such a way that the same source package outputs subpackages for both the C
> > and Rust bindings (and, potentially, other languages too, such as Python and
> > JavaScript)?
> 
> I don't think that is advisable because each packaging environment have
> their own workflow

I don't think that presents a problem.  See https://src.fedoraproject.org/rpms/tree-sitter-java/blob/rust/f/tree-sitter-java.spec for a spec file that builds binding for Rust as well as C and Python.  I don't know anything about Rust, but the package seems to build okay at least.

> > I have suggested some RPM macros to help build C bindings
> > (https://src.fedoraproject.org/rpms/tree-sitter/pull-request/1).  We could
> > enhance that to cover Rust too, if someone interested in Rust could work out
> > that part of the recipe.
> 
> Do you have example projects that could be using that?

Here's an example spec file:

---
Name:           tree-sitter-c
Version:        0.21.4

%global libname lib%{name}
%global summary C grammar for Tree-sitter
%global desc    Add support for C to Tree-sitter, an incremental parsing system for \
programming tools.


Release:        %autorelease
Summary:        %{summary}
License:        MIT
URL:            https://github.com/tree-sitter/%{name}

Source:         %{url}/archive/v%{version}/%{name}-%{version}.tar.gz

BuildSystem:    treesitter

%description
%{desc}


%package -n %{libname}
Summary:        %{summary}
Recommends:     libtree-sitter
Enhances:       libtree-sitter

%description -n %{libname}
%{desc}


%package -n %{libname}-devel
Summary:        Development files for %{libname}
Requires:       %{libname}%{?_isa} = %{version}-%{release}

%description -n %{libname}-devel
Libraries and header files for developing applications that use
%{name}.


%{?ldconfig_scriptlets}


%files -n %{libname}
%license LICENSE
%doc README.md
%{_libdir}/%{libname}.so.*

%files -n %{libname}-devel
%{_includedir}/tree_sitter/%{name}.h
%{_libdir}/%{libname}.so
%{_libdir}/pkgconfig/%{name}.pc


%changelog
%autochangelog
---

> I don't think that presents a problem.  See https://src.fedoraproject.org/rpms/tree-sitter-java/blob/rust/f/tree-sitter-java.spec for a spec file that builds binding for Rust as well as C and Python.  I don't know anything about Rust, but the package seems to build okay at least.

This package is in violation of the Rust Packaging Guidelines, notably:

> Projects from crates.io MUST be packaged from the sources that are published there (i.e. by using the %{crates_source} macro).

https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_package_sources

The exception from this section might apply, but I am surprised that this hasn't been discussed with the Rust SIG at all.

https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_crates_with_rust_library_interface

To elaborate on the rust packaging, it is quite difficult to maintain a package without running `rust2rpm` and then if you want to add other bindings like python and go, then the packaging becomes even more complicated to satisfy each packaging guidelines simultaneously. For example for rust packaging the whole repo source must be packaged for re-distribution since it works like a header-only library, but then we would have to filter out all of the bindings from the other languages.

What your `BuildSystem` approach would be useful for though is for distributing the C (and/or C++?) libraries as you are doing with the snippet you've shared. But I have one recommendation. Instead of using the Makefile, please use the CMake build system because that also generates the <Pkg>Config.cmake files needed to be imported by other CMake/meson projects.

(In reply to Cristian Le from comment #9)
> To elaborate on the rust packaging, it is quite difficult to maintain a
> package without running `rust2rpm`

Does the resulting .spec vary much from one Tree-sitter parser to another?

> and then if you want to add other
> bindings like python and go, then the packaging becomes even more
> complicated to satisfy each packaging guidelines simultaneously.

Assuming we want the bindings for multiple languages, is it really harder to do this in a single spec file rather than artificially splitting it over multiple source packages?

I think if we can get this right once, and make a suitable template, we can save a lot of effort in the long run.

> For example
> for rust packaging the whole repo source must be packaged for
> re-distribution since it works like a header-only library, but then we would
> have to filter out all of the bindings from the other languages.

I don't think this is true.  Calling %cargo_install on the source code fetched from GitHub does the right thing AFAICT.

> But I have one recommendation. Instead of using the Makefile,
> please use the CMake build system because that also generates the
> <Pkg>Config.cmake files needed to be imported by other CMake/meson projects.

Sounds good, but that appears to be a new feature of tree-sitter released only last month that we don't have in Fedora yet, and that not all parsers so-far support (ironically, tree-sitter-cmake included).

(In reply to Fabio Valentini from comment #8)

> The exception from this section might apply,

That was my reading, yes.

> but I am surprised that this hasn't been discussed with the Rust SIG at all.

I think Tree-sitter parser packaging would probably benefit from some discussion more generally.  I'll put together a proof of concept and post about it.

(In reply to Peter Oliver from comment #11)

> I think Tree-sitter parser packaging would probably benefit from some
> discussion more generally.  I'll put together a proof of concept and post
> about it.

I've asked about this on the Packaging mailing list. https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/thread/X3B2MYPPSWBGWRFD7K62NNLIMIL3O56F/

There was a lack on enthusiasm for the idea of packaging bindings for multiple languages from a common source.  This could be down to a lack of interest in Tree-sitter in general, but, in any case, let’s go ahead without it for now.

Would you like to swap this review for any of those listed at https://bugzilla.redhat.com/buglist.cgi?bug_id=2258924&bug_id_type=anddependson&bug_status=NEW&columnlist=product%2Ccomponent%2Cassigned_to%2Cbug_status%2Cshort_desc%2Cchangeddate%2Cbug_severity&component=Package%20Review&list_id=13541136&order=status%2C%20assigned_to%2C%20id%2C%20&product=Fedora&query_format=advanced ?

I need it for difftastic, and did not realize there is this review request until I was about to submit mine (sorry!)

I put up a PR to have upstream include the license file - https://github.com/tree-sitter/tree-sitter-bash/pull/286

Package was generated with rust2rpm, simplifying the review.

- package builds and installs without errors on rawhide
- test suite is run and all unit tests pass
! latest version of the crate is packaged
  => please update to 0.23.3 when importing
- license matches upstream specification (MIT) and is acceptable for Fedora
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks (use `fedora-sig-onboard onboard rust-$crate` to automate):

- add @rust-sig with "commit" access as package co-maintainer
  (should happen automatically)

- set bugzilla assignee overrides to @rust-sig (optional)

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- track package in koschei for all built branches
  (should happen automatically once rust-sig is co-maintainer)

The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-tree-sitter-bash

FEDORA-2025-77f48bda55 (rust-tree-sitter-bash-0.23.3-8.fc41) has been submitted as an update to Fedora 41.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-77f48bda55

FEDORA-2025-057be36c81 (rust-tree-sitter-bash-0.23.3-8.fc40) has been submitted as an update to Fedora 40.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-057be36c81

FEDORA-2025-77f48bda55 has been pushed to the Fedora 41 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-77f48bda55 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-77f48bda55

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2025-057be36c81 has been pushed to the Fedora 40 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-057be36c81 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-057be36c81

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2025-d985ff519b has been pushed to the Fedora 42 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-d985ff519b \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-d985ff519b

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2025-77f48bda55 (rust-tree-sitter-bash-0.23.3-8.fc41) has been pushed to the Fedora 41 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2025-057be36c81 (rust-tree-sitter-bash-0.23.3-8.fc40) has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2025-d985ff519b (rust-tree-sitter-bash-0.23.3-8.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.