[Bug 2245445] New: Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

            Bug ID: 2245445
           Summary: Review Request: rust-logos-codegen - Implementation
                    details for logos-codegen and logos-derive
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: nobody(a)fedoraproject.org
          Reporter: fedoraproject.kwudj(a)passinbox.com
        QA Contact: extras-qa(a)fedoraproject.org
                CC: package-review(a)lists.fedoraproject.org
  Target Milestone: ---
    Classification: Fedora



Spec URL:
https://gitlab.com/iSaluki/fedora-rust-logos-codegen/-/raw/main/rust-logo...
SRPM URL:
https://gitlab.com/iSaluki/fedora-rust-logos-codegen/-/raw/main/rust-logo...
Description: Implementation details for logos-codegen and logos-derive
Fedora Account System Username: saluki

Passing Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=107882800


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2245450





Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=2245450
[Bug 2245450] Review Request: rust-logos-derive - Create ridiculously fast
Lexers
-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2245451





Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=2245451
[Bug 2245451] Review Request: rust-logos - Create ridiculously fast Lexers
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Fabio Valentini <decathorpe(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Doc Type|---                         |If docs needed, set a value
              Flags|                            |fedora-review?
           Assignee|nobody(a)fedoraproject.org    |decathorpe(a)gmail.com
                 CC|                            |decathorpe(a)gmail.com



--- Comment #1 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
Package looks good, except that upstream doesn't ship license files.
Including license files in (re)distributed sources is mandatory for MIT and
Apache-2.0.

Please file a ticket with the upstream project to include license files in
published crates.
You can look at https://github.com/alexcrichton/ssh2-rs/issues/289 that I filed
for the same issue in a different crate.

Until this is resolved upstream, you can include the files as additional
sources from the upstream git repo.
Here's a package that can serve as an example of how to do that:
https://src.fedoraproject.org/rpms/rust-unic-locale/blob/rawhide/f/rust-u...


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #2 from Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; ---
Gotcha.

I've opened an issue regarding the licensing problem here:
https://github.com/maciejhirsz/logos/issues/343

I have also mentioned other crates stored in that repository that have the same
licence issue. Two of these are also pending review, blocked by this request.
Hopefully we can get them all sorted together.

I will address the licensing issue with my package and then update you here
once that is done.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #3 from Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt; ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6557793
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-...

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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #4 from Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; ---
The build error is likely caused by some changes I made at the source
repository. This should fix it.

Spec:
https://gitlab.com/iSaluki/fedora-rust-logos-codegen/-/blob/cbe3e567225b5...
SRPM:
https://gitlab.com/iSaluki/fedora-rust-logos-codegen/-/blob/cbe3e567225b5...


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #5 from Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt; ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6557877
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-...

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.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #6 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
Looks like there's some issues:

>  %license %{crate_instdir}/LICENSE-APACHE
>  %license %{crate_instdir}/LICENSE-MI 
These are in the wrong subpackage, they should be under "%files devel".
There's also a typo (trailing "T" missing from "LICENSE-MIT".

You also didn't add the files to the package anywhere - they need to be
separate "Source" entries, and need to be copied into the build directory in
the %prep scriptlet.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #7 from Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; ---
Thanks for the help Fabio. Looks like I managed to botch this one a bit. 

Anyway, here is a fixed version.

Spec:
https://gitlab.com/fedora-pkgs/rust-logos-codegen/-/raw/main/rust-logos-c...
SRPM:https://gitlab.com/fedora-pkgs/rust-logos-codegen/-/raw/main/rust-lo...


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://crates.io/crates/lo
                   |                            |gos-codegen



--- Comment #8 from Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt; ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6577368
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-...

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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #9 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
>  Source1:		%{crate_instdir}/LICENSE-APACHE
>  Source2:		%{crate_instdir}/LICENSE-MIT 
How is this supposed to work? Source and Patch lines are about telling RPM
where to find files *before* building the package.
These are absolute paths, and they don't exist.

You will need to replace these two lines with something like this, to tell RPM
that these files don't come from an absolute path on the filesystem, but insead
come from URLs and can be referenced by relative path:

Source1:  https://github.com/maciejhirsz/logos/raw/v0.13/LICENSE-APACHE
Source2:  https://github.com/maciejhirsz/logos/raw/v0.13/LICENSE-MIT

Additionally, please don't mix tabs and spaces in spec files. And since
rust2rpm defaults to spaces, please use spaces for indenting. :)

Looks like you also made a small mistake when updating the %files list here:

>  %files          devel
>  %doc %{crate_instdir}/README.md
>  %{crate_instdir}/
>  %license %{crate_instdir}/LICENSE-APACHE
>  %license %{crate_instdir}/LICENSE-MIT
>  %{crate_instdir}/ 
The %{crate_instdir}/ should not be listed twice, it should be something like:

%files          devel
%license %{crate_instdir}/LICENSE-APACHE
%license %{crate_instdir}/LICENSE-MIT
%doc %{crate_instdir}/README.md
%{crate_instdir}/

This matches what rust2rpm would produce if the files were included in the
upstream sources.

Please also take care not to introduce additional whitespace / newlines when
making manual changes. They will only result in additional non-useful "diff"
output when updating the package in the future.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #10 from Seth Maurice-Brant <fedoraproject.kwudj(a)passinbox.com&gt; ---
Thank you for this feedback. It definitely cleared up some confusion I had.

here are my ammended files:

Spec:
https://gitlab.com/fedora-pkgs/rust-logos-codegen/-/raw/main/rust-logos-c...
SRPM:https://gitlab.com/fedora-pkgs/rust-logos-codegen/-/raw/main/rust-lo...


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #11 from Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt;
---
Created attachment 1996718
  --> https://bugzilla.redhat.com/attachment.cgi?id=1996718&action=edit
The .spec file difference from Copr build 6577368 to 6591542


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #12 from Fedora Review Service <fedora-review-bot(a)fedoraproject.org&gt;
---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6591542
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-...

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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Fabio Valentini <decathorpe(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+
             Status|ASSIGNED                    |POST



--- Comment #13 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
Thank you, looks good to me now!

===

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
- license matches upstream specification (MIT OR Apache-2.0) and is acceptable
for Fedora
- license files are included with %license in %files (included manually from
upstream repo)
- 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: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

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

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

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


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445



--- Comment #14 from Fedora Admin user for bugzilla script actions
<fedora-admin-xmlrpc(a)fedoraproject.org&gt; ---
The Pagure repository was created at
https://src.fedoraproject.org/rpms/rust-logos-codegen


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Fedora Update System <updates(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED



--- Comment #15 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
FEDORA-2023-0797a6b19d has been submitted as an update to Fedora 40.
https://bodhi.fedoraproject.org/updates/FEDORA-2023-0797a6b19d


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...

[Bug 2245445] Review Request: rust-logos-codegen - Implementation details for logos-codegen and logos-derive

https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Fedora Update System <updates(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2023-11-03 23:43:26



--- Comment #16 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
FEDORA-2023-0797a6b19d has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2245445

Report this comment as SPAM:
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=rep...