Bug 273579

Summary: games/veloren-weekly: fails to build with rust 1.72
Product: Ports & Packages Reporter: Mikael Urankar <mikael>
Component: Individual Port(s)Assignee: Jan Beich <jbeich>
Status: Closed FIXED    
Severity: Affects Only Me Flags: jbeich: maintainer-feedback+
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://gitlab.com/veloren/veloren/-/issues/1875
Attachments:
Description Flags
v0 none

Description Mikael Urankar freebsd_committer freebsd_triage 2023-09-05 11:39:41 UTC
Created attachment 244658 [details]
v0

veloren uses 2 unstables features that were removed in rust 1.72, drain_filter [1] and array_zip [2]. drain_filter is easy to patch as it's just a rename but I can't fix array_zip:

error[E0599]: `[f32; 3]` is not an iterator
  --> voxygen/src/render/pipelines/trail.rs:42:27
   |
42 |             pos: self.pos.zip(other.pos).map(|(a, b)| a + b),
   |                           ^^^ `[f32; 3]` is not an iterator; try calling `.into_iter()` or `.iter()`
...

Upstream hasn't fixed this error yet.

[1] https://github.com/rust-lang/rfcs/issues/2140
[2] https://github.com/rust-lang/rust/pull/112096
Comment 1 Jan Beich freebsd_committer freebsd_triage 2023-09-06 00:40:45 UTC
According to https://github.com/rust-lang/rust/pull/112096 (see auto_vectorize_array_from_fn)

  self.pos.zip(other.pos).map(|(a, b)| a + b)

can be converted to

  std::array::from_fn(|i| self.pos[i] + other.pos[i])

but it doesn't work with

  let bloom_binds = bloom.map(|bloom| {
      bloom
          .src_views
          .zip(bloom.locals) // zip arrays
          .map(|(view, locals)| layouts.bloom.bind(device, view, sampler, locals))
  });

converted to

  let bloom_binds = bloom.map(|bloom| {
          std::array::from_fn(|i| layouts.bloom.bind(
              device, bloom.src_views[i], sampler, bloom.locals[i]))
  });

due to

  error[E0507]: cannot move out of `bloom.locals[_]`, as `bloom.locals` is a captured variable in an `FnMut` closure
    --> voxygen/src/render/renderer/locals.rs:62:58
     |
  60 |  let bloom_binds = bloom.map(|bloom| {
     |                               ----- captured outer variable
  61 |          std::array::from_fn(|i| layouts.bloom.bind(
     |                              --- captured by this `FnMut` closure
  62 |              device, bloom.src_views[i], sampler, bloom.locals[i]))
     |                                                   ^^^^^^^^^^^^^^^ move occurs because `bloom.locals[_]` has type `Consts<bloom::Locals>`, which does not implement the `Copy` trait


Unfortunately, my Rust knowledge is rudimentary. I'd recommend to file a bug upstream to ask when the next toolchain update is planned. In 2022 September was about time when April rust-toolchain file was updated.

For now, feel free to mark the port as BROKEN as the majority of users are on /quarterly.

2023Q4 will be branched soon, so /quarterly may inherit BROKEN. Delaying rust-1.72 may help *but* if Veloren upstream updates rust-toolchain then /quarterly would benefit from rust-1.72 (to avoid having to maintain rust-1.71 compat). Cherry-picking rust-1.72 into 2023Q4 later is probably too much PITA due to PORTREVISION bumps.
Comment 2 commit-hook freebsd_committer freebsd_triage 2023-09-08 09:07:56 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=362de6dd83786ea3668f3519b3dae049a9cb42f3

commit 362de6dd83786ea3668f3519b3dae049a9cb42f3
Author:     Mikael Urankar <mikael@FreeBSD.org>
AuthorDate: 2023-09-08 09:03:34 +0000
Commit:     Mikael Urankar <mikael@FreeBSD.org>
CommitDate: 2023-09-08 09:05:43 +0000

    games/veloren-weekly: Mark as broken with rust 1.72

    PR:             273579
    Approved by:    jbeich (maintainer)

 games/veloren-weekly/Makefile | 1 +
 1 file changed, 1 insertion(+)
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-09-14 07:55:03 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=b55fc6a70267ca1f9638d9eee36ed38559cdc2fd

commit b55fc6a70267ca1f9638d9eee36ed38559cdc2fd
Author:     Jan Beich <jbeich@FreeBSD.org>
AuthorDate: 2023-09-14 07:13:34 +0000
Commit:     Jan Beich <jbeich@FreeBSD.org>
CommitDate: 2023-09-14 07:54:10 +0000

    games/veloren-weekly: unbreak build after fa874813924c

    PR:             273579
    Submitted by:   mikael (drain_filter)
    Obtained from:  upstream (array_zip)

 games/veloren-weekly/Makefile                    |   1 -
 games/veloren-weekly/files/patch-rust-1.72 (new) | 179 +++++++++++++++++++++++
 2 files changed, 179 insertions(+), 1 deletion(-)
Comment 4 Jan Beich freebsd_committer freebsd_triage 2023-09-14 07:56:48 UTC
After checking https://gitlab.com/veloren/dev/veloren/-/commits/christof/toolchain_update here's what was missing to fix the build:

--- voxygen/src/render/pipelines/bloom.rs.orig	unmodified
+++ voxygen/src/render/pipelines/bloom.rs	upstream
@@ -112,7 +112,7 @@ impl BloomLayout {
         device: &wgpu::Device,
         src_color: &wgpu::TextureView,
         sampler: &wgpu::Sampler,
-        half_pixel: Consts<Locals>,
+        half_pixel: &Consts<Locals>,
     ) -> BindGroup {
         let bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
             label: None,
--- voxygen/src/render/renderer/locals.rs.orig	comment 1
+++ voxygen/src/render/renderer/locals.rs	upstream (except style)
@@ -59,7 +59,7 @@ impl Locals {
 
         let bloom_binds = bloom.map(|bloom| {
             std::array::from_fn(|i| layouts.bloom.bind(
-                device, bloom.src_views[i], sampler, bloom.locals[i]))
+                device, bloom.src_views[i], sampler, &bloom.locals[i]))
         });
 
         Self {
@@ -106,7 +106,7 @@ impl Locals {
         );
         self.bloom_binds = bloom.map(|bloom| {
             std::array::from_fn(|i| layouts.bloom.bind(
-                device, bloom.src_views[i], sampler, bloom.locals[i]))
+                device, bloom.src_views[i], sampler, &bloom.locals[i]))
         });
     }
 }
Comment 5 Mikael Urankar freebsd_committer freebsd_triage 2023-09-14 08:10:29 UTC
(In reply to Jan Beich from comment #4)
There is also:
voxygen/src/render/pipelines/trail.rs
voxygen/src/render/renderer/locals.rs
Comment 6 Jan Beich freebsd_committer freebsd_triage 2023-10-16 13:48:28 UTC
Can you close upstream issue after ports b51d169384d5? It contains https://gitlab.com/veloren/veloren/-/commit/394c8892bb2f
Comment 7 Mikael Urankar freebsd_committer freebsd_triage 2023-10-16 13:51:43 UTC
(In reply to Jan Beich from comment #6)
done