You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "mbrobbel (via GitHub)" <gi...@apache.org> on 2023/08/05 18:42:43 UTC

[GitHub] [arrow-rs] mbrobbel opened a new pull request, #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

mbrobbel opened a new pull request, #4653:
URL: https://github.com/apache/arrow-rs/pull/4653

   # Which issue does this PR close?
   
   None.
   
   # Rationale for this change
   
   Some crates use the `workspace.rust-version` which is set to `1.62`. However, for example `arrow-buffer` won't compile with `1.62`, because it depends on `half`, which requires `1.70`. 
   
   # What changes are included in this PR?
   
   This bumps the `workspace.rust-version` to `1.70.0`, and sets all members (which didn't already do this) to use the `workspace.rust-version`.
   
   # Are there any user-facing changes?
   
   Yes, the `rust-version` field for all crates in the workspace is now correct and the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] mbrobbel commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.
mbrobbel commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667642369

   > Looks okay. Just one question. I think half only requires 1.70 since [v2.3.0](https://github.com/starkat99/half-rs/releases/tag/v2.3.0). But arrow-buffer uses half 2.1, cannot it be compiled using 1.62?
   
   `arrow-buffer` requires half `2.1` so that is `>=2.1.0, <3.0.0`, and the REAMDE of `half-rs` states: [*Requires Rust 1.70 or greater.* If you need support for older versions of Rust, use 1.x versions of this crate.](https://github.com/starkat99/half-rs/blob/cabfc74e2a48b44b4556780f9d1550dd50a708be/README.md?plain=1#L18-L19). We would have to use [tilde requirements](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements) to pin to a minor version.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1670155006

   Having thought about this I think I would rather keep the MSRV set as currently, this is based off the following rationale:
   
   * The crate can be compiled with 1.62, although this does require manual intervention
   * If someone is on a version < 1.70 they will get effectively same message by as they would if we updated the MSRV to 1.70, just reported by half instead of this crate
   
   As such bumping the MSRV does not improve the UX for users on old versions, they get the same error regardless, but does reduce their options for how to deal with it, by forcing them to update the Rust toolchain. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] mbrobbel commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.
mbrobbel commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667677733

   > > Hm I ran into this (after adding arrow-buffer as dependency) [here](https://github.com/mbrobbel/narrow/actions/runs/5772132679/job/15646790593#step:5:48):
   > 
   > Is it possible your workspace contains a version constraint or lockfile that is forcing it to use v2.3.1 of half?
   
   I have no `Cargo.lock` file (library crate), and no other dependencies that depend on `half`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] mbrobbel commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.
mbrobbel commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667661412

   > The MSRV CI job appears to be failing as a result of this change?
   
   Looks like `workspace.rust-version` is unsupported until the next release of `cargo msrv`: https://github.com/foresterre/cargo-msrv/issues/590#issuecomment-1396887424. That probably explains why these were not using the workspace `rust.version`. Maybe this PR should just bump the `workspace.rust-version` then?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667671400

   > Hm I ran into this (after adding arrow-buffer as dependency) [here](https://github.com/mbrobbel/narrow/actions/runs/5772132679/job/15646790593#step:5:48):
   
   Is it possible your workspace contains a version constraint or lockfile that is forcing it to use v2.3.1 of half?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] mbrobbel closed pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.
mbrobbel closed pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members
URL: https://github.com/apache/arrow-rs/pull/4653


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1666599581

   The MSRV CI job appears to be failing as a result of this change?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667965288

   It would appear I am incorrect and cargo does not perform MSRV-aware resolution. There is a ticket https://github.com/rust-lang/cargo/issues/9930 to track supporting this.
   
   I confirmed this with
   
   ```
   $ cargo new msrv_test
   $ cd msrv_test
   $ rustup override set 1.62
   $ cargo --version
   cargo 1.62.1 (a748cf5a3 2022-06-08)
   $ echo 'arrow-buffer = "45.0"' >> Cargo.toml
   $ cargo build
   error: package `half v2.3.1` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.62.1
   $ cargo update -p half --precise v2.1.0
   $ cargo build
      Compiling autocfg v1.1.0
      Compiling half v2.1.0
      Compiling num-traits v0.2.16
      Compiling num-integer v0.1.45
      Compiling num-bigint v0.4.3
      Compiling num-iter v0.1.43
      Compiling num-rational v0.4.1
      Compiling num-complex v0.4.3
      Compiling num v0.4.1
      Compiling arrow-buffer v45.0.0
      Compiling msrv_test v0.1.0 (/home/raphael/repos/external/msrv_test)
       Finished dev [unoptimized + debuginfo] target(s) in 5.11s
   ```
   
   That being said I don't actually know what the correct policy here is, I would expect the MSRV to track the minimum MSRV where a compatible set of crate versions exists, as opposed to the MSRV of the latest version of all crates... I'll see if I can't ping some other crate maintainers who may be able to weigh in with greater authority
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] mbrobbel commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.
mbrobbel commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667669541

   > > We would have to use [tilde requirements](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements) to pin to a minor version.
   > 
   > Unless I am mistaken, cargo will not select a crate version that is not compatible with the selected version of Rust. Therefore if someone is running 1.62 it will select an older version of half to satisfy this. Provided this compiles, which the CI job would indicate it does, we are perfectly correct to state the MSRV as 1.62. This would only change if we were to specify a half version constraint that could not be satisfied by a version compatible with 1.62
   
   Hm I ran into this (after adding `arrow-buffer` as dependency) [here](https://github.com/mbrobbel/narrow/actions/runs/5772132679/job/15646790593#step:5:48):
   ```
   error: package `half v2.3.1` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.65.0
   Either upgrade to rustc 1.70 or newer, or use
   cargo update -p half@2.3.1 --precise ver
   where `ver` is the latest version of `half` supporting rustc 1.65.0
   Error: Process completed with exit code 101.
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on pull request #4653: Bump `workspace.rust-version` to `1.70.0` and use for all members

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #4653:
URL: https://github.com/apache/arrow-rs/pull/4653#issuecomment-1667664362

   > We would have to use [tilde requirements](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements) to pin to a minor version.
   
   Unless I am mistaken, cargo will not select a crate version that is not compatible with the selected version of Rust. Therefore if someone is running 1.62 it will select an older version of half to satisfy this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org