[Bug 2278709] New: Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

Bug ID: 2278709
           Summary: Review Request: rust-buddy-alloc - Memory allocator
                    for no-std Rust, used for embedded environments
           Product: Fedora
           Version: rawhide
            Status: NEW
         Component: Package Review
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://jordalgo.fedorapeople.org/review/rust-buddy-alloc/rust-buddy-alloc.s...
SRPM URL:
https://jordalgo.fedorapeople.org/review/rust-buddy-alloc/rust-buddy-alloc-0...

Description:
Buddy-alloc is a memory allocator for no-std Rust, used for embedded
environments.

Fedora Account System Username: jordalgo

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #1 from Jordan Rome  ---
This package built on koji: 
https://koji.fedoraproject.org/koji/taskinfo?taskID=117164443

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

Fedora Review Service  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://crates.io/crates/bu
                   |                            |ddy-alloc

--- Comment #2 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7399587
(succeeded)

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

Found issues:

- License file LICENSE is not marked as %license
  Read more:
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline...

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.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Doc Type|---                         |If docs needed, set a value
                 CC|                            |[e-mail]

--- Comment #3 from Fabio Valentini  ---
This fails to build for me:
> File not found: /builddir/build/BUILDROOT/rust-buddy-alloc-0.5.1-1.fc41.x86_64/usr/share/cargo/registry/buddy-alloc-0.5.1/LICENSE-MIT.txt
>    File not found: /builddir/build/BUILDROOT/rust-buddy-alloc-0.5.1-1.fc41.x86_64/usr/share/cargo/registry/buddy-alloc-0.5.1/Readme.md

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #4 from Jordan Rome  ---
Sorry for the delay. Do you mind giving it another try?

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]

--- Comment #5 from Fabio Valentini  ---
Package builds now, thanks!

Two small issues:

1. Please don't include the rpmlintrc file in the spec / SRPM though. It's
unrelated to the RPM build.
   You can include it in the dist-git repo after importing the package though.

2. I cannot reproduce the test failures.
> # fast_alloc tests end in a segfault on x86
> %cargo_test -- -- --skip failing_tests --skip tests::fast_alloc

I can run "cargo test" and "cargo test --release" in the sources on my x86
machine just fine.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #6 from Jordan Rome  ---
> I can run "cargo test" and "cargo test --release" in the sources on my x86 machine just fine.

The comment in the spec might just be wrong. I ran `mock -r
fedora-rawhide-x86_64 --sources . --spec rust-buddy-alloc.spec --postinstall
` locally on my aarch64 (Mac) with Parallels and I got that failure (maybe
because of the architecture mismatch?). https://pastebin.com/NYejgAYp

If I run `cargo test` from the buddy-alloc repo directly it works just fine.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #7 from Jordan Rome  ---
Test failure is still happening with `mock -r fedora-rawhide-aarch64 --sources
. --spec rust-buddy-alloc.spec --postinstall` - maybe it has something to do
with being in a VM.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #8 from Jordan Rome  ---
> Please don't include the rpmlintrc file in the spec / SRPM though. It's unrelated to the RPM build. You can include it in the dist-git repo after importing the package though.

Done - removed it from the rpm and spec files.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #9 from Fabio Valentini  ---
> Test failure is still happening with `mock -r fedora-rawhide-aarch64 --sources . --spec rust-buddy-alloc.spec --postinstall` - maybe it has something to do with being in a VM.

OK, if you want to skip tests because koji builds happen either on "bare metal"
or in VMs, and they will fail if they're scheduled to run in a VM, please
document that. Right now there is no documented reason why some tests are
skipped.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #10 from Jordan Rome  ---
> please document that

Makes sense. I updated the spec file with a comment.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #11 from Fabio Valentini  ---
Sorry, the new comment is only more confusing to me.
I don't see how Fedora 38 is relevant, it is EOL.

Additionally, the files behind the spec and SRPM links now mismatch.

I ran a rawhide scratch build of your package (without skipped tests):
https://koji.fedoraproject.org/koji/taskinfo?taskID=118963473

Tests crashed on *all* architectures with SIGSEGV.
So it looks like something is seriously wrong with the code of this crate ...

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #12 from Jordan Rome  ---
No worries, I updated the comment and also synced the uploaded spec file and
SRPM.
> Tests crashed on *all* architectures with SIGSEGV. So it looks like something is seriously wrong with the code of this crate ...

What's strange is that when I run `cargo test` on this package on my same
machine (where the mock build fails), they pass as normal.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #13 from Jordan Rome  ---
No worries, I updated the comment and also synced the uploaded spec file and
SRPM.
> Tests crashed on *all* architectures with SIGSEGV. So it looks like something is seriously wrong with the code of this crate ...

What's strange is that when I run `cargo test` on this package on my same
machine (where the mock build fails), they pass as normal.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #14 from Fabio Valentini  ---
Can you try running "cargo test --release"?

Tests are run and built in "release" mode during package builds.

If the presence of "--release" makes a difference, then it sounds like a
soundness bug that is exposed by turning on optimizations.

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #15 from Jordan Rome  ---
> Can you try running "cargo test --release"?

Ah ha. Now I see the failure. I guess this is something we need to fix upstream
before packaging this?

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #16 from Fabio Valentini  ---
If this is indeed a soundness issue (which is what it looks like), then yes, I
would prefer if it was fixed before we package this for Fedora.
Can you file bug against upstream for this issue?

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #17 from Jordan Rome  ---
Done: https://github.com/jjyr/buddy-alloc/issues/16

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

--- Comment #18 from Fabio Valentini  ---
Looks like the issues have been fixed upstream!
Can you update this review for the new version that was published?

[Bug 2278709] Review Request: rust-buddy-alloc - Memory allocator for no-std Rust, used for embedded environments

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

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(jordan@jordanrome
                   |                            |.com)