You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "EricccTaiwan (via GitHub)" <gi...@apache.org> on 2025/07/21 15:50:00 UTC

[PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   # Which issue does this PR close?
   
   None.
   
   # Rationale for this change
   
   Without a fixed toolchain, contributors may use different Rust versions, leading to inconsistent Clippy lints or build errors (e.g., from renamed lints). This change ensures that the project builds consistently across environments.
   
   # What changes are included in this PR?
   
   - Adds a `rust-toolchain.toml` file to pin the Rust toolchain to the `stable` channel.
   
   
   # Are these changes tested?
   
   This change affects toolchain configuration and does not modify runtime code. Behavior is implicitly tested via CI builds, which will now consistently use the pinned toolchain.
   
   # Are there any user-facing changes?
   
   No.


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   Here is an example of what happens without this PR:
   1. New clippy is released, and CI tests start failing
   2. First person to notice needs to make a PR to fix it
   1. https://github.com/apache/arrow-rs/pull/8078


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > > Hmm, looks like the CI jobs will need to be updated to account for the new toolchain somehow
   > 
   > Yes, I see -- I can give it a try to modify the CI part. By the way, is there a way for me to test the CI on my own fork? Or does it only run on the upstream repo?
   
   I think can make (another) PR into your own fork to test the CI
   
   
   


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > Hmm, looks like the CI jobs will need to be updated to account for the new toolchain somehow
   
   Yes, I see -- I can give it a try to modify the CI part.
   By the way, is there a way for me to test the CI on my own fork? Or does it only run on the upstream repo?


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   Thanks again @EricccTaiwan  -- since there has been no more comments on this PR I will merge it in


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   Hmm, looks like the CI jobs will need to be updated to account for the new toolchain somehow


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2223879000


##########
rust-toolchain.toml:
##########
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[toolchain]
+channel = "stable"

Review Comment:
   I think it is preferable to have to manually bump it to avoid CI randomly breaking when a new rust version is released. 
   - For example https://github.com/apache/arrow-rs/pull/7797



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > > Since the new upgrade just caused me annoyance I am going to try and push this PR through (It will be easier for me as I can trigger CI). Thanks you @EricccTaiwan
   > 
   > Hi @alamb , I’ve been a bit busy lately, and this change looks a bit more involved than I can tackle right now. I’m not sure how to move it forward at the moment, but I’m happy to help if there’s anything specific you need. Thanks!
   
   No worries -- thank you for the help. I will get this one over the line


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2220531169


##########
rust-toolchain.toml:
##########
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[toolchain]
+channel = "stable"

Review Comment:
   What do you think of adding a specific release here like https://github.com/apache/datafusion/blob/70e7eb3aaef221c8fefe2eee6198e9a61d68bc0f/rust-toolchain.toml#L22
   
   `1.88` 
   
   That will prevent random failure when new stable releases are released



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   By the way, would it make sense to also add `components = ["rustfmt", "clippy"]` like in DataFusion?
   https://github.com/apache/datafusion/blob/70e7eb3aaef221c8fefe2eee6198e9a61d68bc0f/rust-toolchain.toml#L23


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > Since the new upgrade just caused me annoyance I am going to try and push this PR through (It will be easier for me as I can trigger CI). Thanks you @EricccTaiwan
   
   Hi @alamb , I’ve been a bit busy lately, and this change looks a bit more involved than I can tackle right now. 
   I’m not sure how to move it forward at the moment, but I’m happy to help if there’s anything specific you need. Thanks!


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > By the way, would it make sense to also add `components = ["rustfmt", "clippy"]` like in DataFusion? https://github.com/apache/datafusion/blob/70e7eb3aaef221c8fefe2eee6198e9a61d68bc0f/rust-toolchain.toml#L23
   
   I am not sure


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > I'm a bit confused — on my fork, CI only seems to run on the main branch. When I sync with upstream, it works fine, but creating a new branch (like this PR) doesn’t trigger CI at all.
   > And just to confirm: on the upstream repo, do CI jobs always require approval before they start running?
   
   CI jobs on this (the upstream) repo need approval for first time contributors. Once we have merged a PR from you then the CI starts automatically on subsequent PRs


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2260665482


##########
.github/workflows/docs.yml:
##########
@@ -34,12 +34,8 @@ jobs:
   docs:
     name: Rustdocs are clean
     runs-on: ubuntu-latest
-    strategy:

Review Comment:
   This adds a few seconds to the job 
   
   <img width="1163" height="204" alt="Screenshot 2025-08-07 at 11 19 10 AM" src="https://github.com/user-attachments/assets/099c116d-11c0-412f-9002-f81f3f85fa77" />
   



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   THANK YOU @EricccTaiwan  -- this is super helpful


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > Thanks again @EricccTaiwan -- since there has been no more comments on this PR I will merge it in
   
   Thanks @alamb, for doing most of the work, lol!


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > > > Hmm, looks like the CI jobs will need to be updated to account for the new toolchain somehow
   > > 
   > > 
   > > Yes, I see -- I can give it a try to modify the CI part. By the way, is there a way for me to test the CI on my own fork? Or does it only run on the upstream repo?
   > 
   > I think can make (another) PR into your own fork to test the CI
   
   I'm a bit confused — on my fork, CI only seems to run on the main branch. When I sync with upstream, it works fine, but creating a new branch (like this PR) doesn’t trigger CI at all.
   And just to confirm: on the upstream repo, do CI jobs always require approval before they start running?


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2260654032


##########
.github/actions/setup-builder/action.yaml:
##########
@@ -17,15 +17,6 @@
 
 name: Prepare Rust Builder
 description: 'Prepare Rust Build Environment'
-inputs:

Review Comment:
   I removed these parameters as almost all uses installed stable 



##########
.github/workflows/docs.yml:
##########
@@ -34,12 +34,8 @@ jobs:
   docs:
     name: Rustdocs are clean
     runs-on: ubuntu-latest
-    strategy:

Review Comment:
   I think this is let over from a previous time when we checked docs on different configurations -- at the moment there is a single pair that is used, so I unparameterized this and made the version used explicit



##########
.github/workflows/arrow.yml:
##########
@@ -148,8 +148,10 @@ jobs:
           submodules: true
       - name: Setup Rust toolchain
         uses: ./.github/actions/setup-builder
-        with:
-          target: wasm32-unknown-unknown,wasm32-wasip1
+      - name: Install wasm32 targets
+        run: |
+          rustup target add wasm32-unknown-unknown

Review Comment:
   This adds 4 seconds to the job:
   
   <img width="1163" height="204" alt="Screenshot 2025-08-07 at 11 19 10 AM" src="https://github.com/user-attachments/assets/321495ff-9c6c-4123-8972-219b5e497742" />
   



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2260665482


##########
.github/workflows/docs.yml:
##########
@@ -34,12 +34,8 @@ jobs:
   docs:
     name: Rustdocs are clean
     runs-on: ubuntu-latest
-    strategy:

Review Comment:
   This adds 9s to the job 
   
   <img width="1137" height="64" alt="Screenshot 2025-08-07 at 11 23 24 AM" src="https://github.com/user-attachments/assets/c7126a89-bf41-43b2-98e2-c6f36e314edc" />
   



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   Since the new upgrade just caused me annoyance I am going to try and push this PR through (It will be easier for me as I can trigger CI). Thanks you @EricccTaiwan 


-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

Posted by "EricccTaiwan (via GitHub)" <gi...@apache.org>.
EricccTaiwan commented on code in PR #7972:
URL: https://github.com/apache/arrow-rs/pull/7972#discussion_r2220902407


##########
rust-toolchain.toml:
##########
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[toolchain]
+channel = "stable"

Review Comment:
   > What do you think of adding a specific release here like https://github.com/apache/datafusion/blob/70e7eb3aaef221c8fefe2eee6198e9a61d68bc0f/rust-toolchain.toml#L22
   > 
   > `1.88`
   > 
   > That will prevent random failure when new stable releases are released
   
   Makes sense. Using `1.88` avoids random breakage, though it means running rustup manually. 
   I'm okay with both. What do you prefer?
   



-- 
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


Re: [PR] chore: Add rust-toolchain.toml to ensure consistent toolchain version [arrow-rs]

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

   > > By the way, would it make sense to also add `components = ["rustfmt", "clippy"]` like in DataFusion? https://github.com/apache/datafusion/blob/70e7eb3aaef221c8fefe2eee6198e9a61d68bc0f/rust-toolchain.toml#L23
   > 
   > I am not sure
   
   Just thought it might be helpful for consistency, and possibly more contributor-friendly too -- especially for folks who rely on `cargo fmt` and `cargo clippy` out of the box.
   But I’m good either way. :+1:


-- 
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