Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FTBFS with rust-1.71.0 #5861

Closed
vashirov opened this issue Jul 25, 2023 · 8 comments · Fixed by #5854
Closed

FTBFS with rust-1.71.0 #5861

vashirov opened this issue Jul 25, 2023 · 8 comments · Fixed by #5854
Labels
needs triage The issue will be triaged during scrum

Comments

@vashirov
Copy link
Member

Issue Description

Reported in https://bugzilla.redhat.com/show_bug.cgi?id=2225532

error: using `.borrow()` on a double reference, which returns `&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type
  --> librslapd/src/cache.rs:60:28
   |
60 |     let stats = stat_rguard.borrow();
   |                            ^^^^^^^^^
   |
note: the lint level is defined here
  --> librslapd/src/lib.rs:1:9
   |
1  | #![deny(warnings)]
   |         ^^^^^^^^
   = note: `#[deny(suspicious_double_ref_op)]` implied by `#[deny(warnings)]`
error: could not compile `librslapd` (lib) due to previous error

error: variable does not need to be mutable
   --> slapi_r_plugin/src/value.rs:185:13
    |
185 |         let mut v = unsafe { slapi_value_new() };
    |             ----^
    |             |
    |             help: remove this `mut`
    |
note: the lint level is defined here
   --> slapi_r_plugin/src/lib.rs:1:9
    |
1   | #![deny(warnings)]
    |         ^^^^^^^^
    = note: `#[deny(unused_mut)]` implied by `#[deny(warnings)]`
error: could not compile `slapi_r_plugin` (lib) due to previous error
@vashirov vashirov added the needs triage The issue will be triaged during scrum label Jul 25, 2023
@droideck
Copy link
Member

Fixed here:
#5854

droideck added a commit to droideck/389-ds-base that referenced this issue Jul 25, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.

Related: 389ds#5853
Fixes: 389ds#5861

Reviewed by: ?
droideck added a commit to droideck/389-ds-base that referenced this issue Jul 26, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.

Related: 389ds#5853
Fixes: 389ds#5861

Reviewed by: ?
droideck added a commit to droideck/389-ds-base that referenced this issue Jul 27, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.

Related: 389ds#5853
Fixes: 389ds#5861

Reviewed by: ?
droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
@vashirov
Copy link
Member Author

vashirov commented Aug 1, 2023

@droideck, could you please backport it down to 1.4.3? rust-1.71 is already in 8.9.

droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
droideck added a commit that referenced this issue Aug 1, 2023
Description: Run cargo update --manifest-path=./src/Cargo.toml.
Add minimum supported rust version field to the manifests.
Fix minor 'variable does not need to be mutable' error.

Another error:
error: using `.borrow()` on a double reference, which returns
`&concread::cowcell::CowCellReadTxn<CacheStats>` instead of borrowing the inner type

We're getting the error about borrowing a double reference because
we're trying to borrow a type that is already a reference.
Fix - use the type directly.
Set rust-version to 1.70 for better compatibility.

Related: #5853
Fixes: #5861

Reviewed by: @vashirov (Thanks!), @Firstyear (Thanks for the rust-version idea!)
@droideck
Copy link
Member

droideck commented Aug 1, 2023

3b43a7b..bd3140f 389-ds-base-2.3 -> 389-ds-base-2.3
5e49564..8d74381 389-ds-base-2.2 -> 389-ds-base-2.2
98b5df6..86c736b 389-ds-base-2.1 -> 389-ds-base-2.1
3470b8e..744cb41 389-ds-base-2.0 -> 389-ds-base-2.0
7e07c95..e0ac93b 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

@aivanov389
Copy link

aivanov389 commented Aug 21, 2023

This commit changes minimum rust-version to 1.70 . It results in a fail to build on RHEL9.2 (rust 1.66.1):

-----------------------------> Compiling the 389 Directory Server minimum core...
./ldap/servers/slapd/mkDBErrStrs.py -i ./ldap/servers/slapd/back-ldbm -o .
RUST_BACKTRACE=1 RUSTC_BOOTSTRAP=1 \
CARGO_TARGET_DIR=/Admin/BUILD/389_Ldap/389-ds-base-2.2.9p1/rs/rslapd \
SLAPD_DYLIB_DIR=/Admin/BUILD/389_Ldap/389-ds-base-2.2.9p1/ \
SLAPD_HEADER_DIR=/Admin/BUILD/389_Ldap/389-ds-base-2.2.9p1/ \
        cargo rustc  --manifest-path=./src/librslapd/Cargo.toml \
        --release --verbose -- -C debuginfo=2 
    Updating crates.io index
  Downloaded scopeguard v1.2.0
  Downloaded fastrand v2.0.0
  Downloaded memoffset v0.9.0
  Downloaded log v0.4.19
  Downloaded pin-project-lite v0.2.10
  Downloaded unicode-ident v1.0.11
  Downloaded smallvec v1.11.0
  Downloaded once_cell v1.18.0
  Downloaded lock_api v0.4.10
  Downloaded getrandom v0.2.10
  Downloaded tempfile v3.7.0
  Downloaded ryu v1.0.15
  Downloaded quote v1.0.32
  Downloaded proc-macro2 v1.0.66
  Downloaded bitflags v2.3.3
  Downloaded itoa v1.0.9
  Downloaded crossbeam-epoch v0.9.15
  Downloaded crossbeam-utils v0.8.16
  Downloaded serde v1.0.175
  Downloaded serde_json v1.0.103
  Downloaded rustix v0.38.4
  Downloaded syn v2.0.27
  Downloaded serde_derive v1.0.175
  Downloaded libc v0.2.147
  Downloaded tokio v1.29.1
  Downloaded linux-raw-sys v0.4.3
  Downloaded 26 crates (4.2 MB) in 0.43s (largest was `linux-raw-sys` at 1.1 MB)
error: package `librslapd v0.1.0 (/Admin/BUILD/389_Ldap/389-ds-base-2.2.9p1/src/librslapd)` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.66.1

@vashirov
Copy link
Member Author

@aivanov389, could you please try with this patch?

diff --git a/src/librnsslapd/Cargo.toml b/src/librnsslapd/Cargo.toml
index c18ab7fc8..11bb9afe7 100644
--- a/src/librnsslapd/Cargo.toml
+++ b/src/librnsslapd/Cargo.toml
@@ -2,7 +2,6 @@
 name = "librnsslapd"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 build = "build.rs"
 
diff --git a/src/librslapd/Cargo.toml b/src/librslapd/Cargo.toml
index fb445c251..15c00a47b 100644
--- a/src/librslapd/Cargo.toml
+++ b/src/librslapd/Cargo.toml
@@ -2,7 +2,6 @@
 name = "librslapd"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 build = "build.rs"
 
diff --git a/src/plugins/entryuuid/Cargo.toml b/src/plugins/entryuuid/Cargo.toml
index f0d8e9f2a..c43d7a771 100644
--- a/src/plugins/entryuuid/Cargo.toml
+++ b/src/plugins/entryuuid/Cargo.toml
@@ -2,7 +2,6 @@
 name = "entryuuid"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/src/plugins/entryuuid_syntax/Cargo.toml b/src/plugins/entryuuid_syntax/Cargo.toml
index d80b59bf1..f7d3d64c9 100644
--- a/src/plugins/entryuuid_syntax/Cargo.toml
+++ b/src/plugins/entryuuid_syntax/Cargo.toml
@@ -2,7 +2,6 @@
 name = "entryuuid_syntax"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/src/plugins/pwdchan/Cargo.toml b/src/plugins/pwdchan/Cargo.toml
index 3cda69f22..40d8a54aa 100644
--- a/src/plugins/pwdchan/Cargo.toml
+++ b/src/plugins/pwdchan/Cargo.toml
@@ -2,7 +2,6 @@
 name = "pwdchan"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/src/slapd/Cargo.toml b/src/slapd/Cargo.toml
index 39b6fdd1d..a18cb7626 100644
--- a/src/slapd/Cargo.toml
+++ b/src/slapd/Cargo.toml
@@ -2,7 +2,6 @@
 name = "slapd"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/src/slapi_r_plugin/Cargo.toml b/src/slapi_r_plugin/Cargo.toml
index 024bd464a..9d197ec85 100644
--- a/src/slapi_r_plugin/Cargo.toml
+++ b/src/slapi_r_plugin/Cargo.toml
@@ -2,7 +2,6 @@
 name = "slapi_r_plugin"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-rust-version = "1.70"
 edition = "2018"
 build = "build.rs"

@droideck, rust-1.71 won't land in 9.2.z, in fact we have the same issue there. As I mentioned in #5854 (comment), we should be careful with setting rust-version to this high version, when in fact nothing specifically requires it.

@aivanov389
Copy link

Yep, it works with your patch. And it does also compile correctly if i change the minimum version to 1.66:

sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/librnsslapd/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/librslapd/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/plugins/entryuuid/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/plugins/entryuuid_syntax/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/plugins/pwdchan/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/slapd/Cargo.toml
sed -i 's@rust-version = "1.70"@rust-version = "1.66"@g' src/slapi_r_plugin/Cargo.toml

So it's really just a rust version difference between RHEL production and CentOS9 Stream it seems...

@droideck
Copy link
Member

@droideck, rust-1.71 won't land in 9.2.z, in fact we have the same issue there. As I mentioned in #5854 (comment), we should be careful with setting rust-version to this high version, when in fact nothing specifically requires it.

Totally agree. I was planning to do that but the pre-PTO mindset had failed me... Sorry :(
I'll do the revert for 2.0, 2.1, and 2.2 tomorrow.

droideck added a commit to droideck/389-ds-base that referenced this issue Aug 22, 2023
Description: We should be careful with rust-version manifest field
on older 389-ds-base versions as it's harder to predict at which
point in time Rust was updated in some older environments.

Related: 389ds#5861

Reviewed by: ?
droideck added a commit that referenced this issue Aug 23, 2023
Description: We should be careful with rust-version manifest field
on older 389-ds-base versions as it's harder to predict at which
point in time Rust was updated in some older environments.

Related: #5861

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this issue Aug 23, 2023
Description: We should be careful with rust-version manifest field
on older 389-ds-base versions as it's harder to predict at which
point in time Rust was updated in some older environments.

Related: #5861

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this issue Aug 23, 2023
Description: We should be careful with rust-version manifest field
on older 389-ds-base versions as it's harder to predict at which
point in time Rust was updated in some older environments.

Related: #5861

Reviewed by: @vashirov (Thanks!)
@droideck
Copy link
Member

f1eb2f1..6687681 389-ds-base-2.2 -> 389-ds-base-2.2
0747f76..5584ef6 389-ds-base-2.1 -> 389-ds-base-2.1
5881562..faa725f 389-ds-base-2.0 -> 389-ds-base-2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage The issue will be triaged during scrum
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants