Bug 2383530 - Review Request: grass-compiler - Sass compiler written purely in Rust
Summary: Review Request: grass-compiler - Sass compiler written purely in Rust
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 2383528 2383529
Blocks: 2373009
TreeView+ depends on / blocked
 
Reported: 2025-07-25 16:57 UTC by Yaakov Selkowitz
Modified: 2025-08-19 01:16 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2025-08-07 23:07:24 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description Yaakov Selkowitz 2025-07-25 16:57:44 UTC
Spec URL: https://yselkowitz.fedorapeople.org/grass-compiler.spec
SRPM URL: https://yselkowitz.fedorapeople.org/grass-compiler-0.13.4-1.fc43.src.rpm
Description: A Sass compiler written purely in Rust.
Fedora Account System Username: yselkowitz

Comment 1 Fabio Valentini 2025-07-31 16:14:45 UTC
Initial review
==============

1. These should both be unused / noops in this kind of package, please remove them:

> %global cargo_install_lib 0
> %global crate grass

2. Use the standard / documented SourceURL format for GitHub sources please:

> Source:         https://github.com/connorskees/grass/archive/refs/tags/%{version}/%{crate}-%{version}.tar.gz
> %autosetup -n %{crate}-%{version} -p1

becomes

> Source:         https://github.com/connorskees/grass/archive/%{version}/grass-%{version}.tar.gz
> %autosetup -n grass-%{version} -p1

3. Don't "cd" into some subdirectory, it shouldn't be necessary:

> %generate_buildrequires
> cd crates/lib
> %cargo_generate_buildrequires

becomes

> %generate_buildrequires
> %cargo_generate_buildrequires

etc. (and analogue changes in the other scriptlets and in the %files list)

Not sure why you thought this was necessary - it should be a noop and just looks confusing.
Running cargo at the top-level (i.e. where the top-level Cargo.toml with the workspace definition is) should work just fine.

Comment 2 Fabio Valentini 2025-07-31 16:16:17 UTC
Oh, and this one too (4., I guess):

> %bcond check 0

This means "don't pull in test dependencies, don't compile tests, don't run tests".

If this is due to an actual issue that prevents tests from being compiled or run, please document this issue. Otherwise, flip it to "1" please.

Comment 3 Yaakov Selkowitz 2025-07-31 19:51:53 UTC
Spec URL: https://yselkowitz.fedorapeople.org/grass-compiler.spec
SRPM URL: https://yselkowitz.fedorapeople.org/grass-compiler-0.13.4-1.fc43.src.rpm

It seems %cargo_install isn't usable in this case.

Comment 4 Fabio Valentini 2025-08-01 13:59:50 UTC
> It seems %cargo_install isn't usable in this case.

Yeah, that's fine. Even if it worked, it would just be a glorified wrapper around `install` in this case - so, equivalent to what you replaced it with.

===

With the latest changes, the package now looks good to me.

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
✅ test suite is run and all unit tests pass
✅ latest version of the project is packaged
✅ license matches upstream specification and is acceptable for Fedora
✅ licenses of statically linked dependencies are correctly taken into account
✅ license file is included with %license in %files
✅ package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

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

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

- add @rust-sig with "commit" access as package co-maintainer

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

Comment 5 Fedora Admin user for bugzilla script actions 2025-08-01 21:04:22 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/grass-compiler

Comment 6 Fedora Update System 2025-08-07 22:42:26 UTC
FEDORA-2025-dd211fa464 (grass-compiler-0.13.4-2.fc43) has been submitted as an update to Fedora 43.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-dd211fa464

Comment 7 Fedora Update System 2025-08-07 23:07:24 UTC
FEDORA-2025-dd211fa464 (grass-compiler-0.13.4-2.fc43) has been pushed to the Fedora 43 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 8 Fabio Valentini 2025-08-12 14:19:46 UTC
Please don't forget to file bugs for the compilation failures in the tests ...

Comment 9 Yaakov Selkowitz 2025-08-19 01:16:40 UTC
(In reply to Fabio Valentini from comment #8)
> Please don't forget to file bugs for the compilation failures in the tests ...

https://src.fedoraproject.org/rpms/grass-compiler/pull-request/1


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