Skip to content

Fix Rustdocs (amd64, nightly)" CI check #5727

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

Merged
merged 2 commits into from
May 7, 2024
Merged

Conversation

alamb
Copy link
Contributor

@alamb alamb commented May 6, 2024

Which issue does this PR close?

Closes #5725

Rationale for this change

Nightly clippy got more stringent

What changes are included in this PR?

  1. Remove some targets which don't exists
  2. Change how rustfmt is disabled for parquet format file

Are there any user-facing changes?

No

@github-actions github-actions bot added parquet Changes to the parquet crate arrow Changes to the arrow crate labels May 6, 2024
@alamb
Copy link
Contributor Author

alamb commented May 6, 2024

The failing test is seems to be fixed: https://github.com/apache/arrow-rs/actions/runs/8970828470/job/24635202345?pr=5727

The integration test failure is tracked via #5719

@alamb alamb added the development-process Related to development process of arrow-rs label May 6, 2024
@@ -80,15 +80,6 @@ pub const ALIGNMENT: usize = 1 << 5;
#[cfg(target_arch = "sparc64")]
pub const ALIGNMENT: usize = 1 << 6;

// On ARM cache line sizes are fixed. both v6 and v7.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed because these target_arch apparently do not exist

warning: unexpected `cfg` condition value: `thumbv7`
  --> arrow-buffer/src/alloc/alignment.rs:89:7
   |
89 | #[cfg(target_arch = "thumbv7")]
   |       ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: expected values for `target_arch` are: `aarch64`, `arm`, `arm64ec`, `avr`, `bpf`, `csky`, `hexagon`, `loongarch64`, `m68k`, `mips`, `mips32r6`, `mips64`, `mips64r6`, `msp430`, `nvptx64`, `powerpc`, `powerpc64`, `riscv32`, `riscv64`, `s390x`, `sparc`, `sparc64`, `wasm32`, `wasm64`, `x86`, `x86_64`
   = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. I tried searching up to understand more but can't really find much to point towards existence of those target_arch values, even though targets like thumbv6m-none-eabi exist

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file is very old ( so it may be something lost to the mists of time ) --

According to some archeology on 1be5358 it looks like it was introduced in apache/arrow#9495 when this file was still part of the arrow mono repo and @jorgecarleitao was working on arrow2

@@ -5,7 +5,8 @@
#![allow(unused_imports)]
#![allow(unused_extern_crates)]
#![allow(clippy::too_many_arguments, clippy::type_complexity, clippy::vec_box, clippy::wrong_self_convention)]
#![cfg_attr(rustfmt, rustfmt_skip)]
// Fix unexpected `cfg` condition name: `rustfmt` https://github.com/apache/arrow-rs/issues/5725
//#![cfg_attr(rustfmt, rustfmt_skip)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was failing like

                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     warning: unexpected `cfg` condition name: `rustfmt`
 --> parquet/src/format.rs:8:13
  |
8 | #![cfg_attr(rustfmt, rustfmt_skip)]
  |             ^^^^^^^
  |
  = help: expected names are: `clippy`, `debug_assertions`, `doc`, `docsrs`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
  = help: consider using a Cargo feature instead or adding `println!("cargo::rustc-check-cfg=cfg(rustfmt)");` to the top of the `build.rs`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration
  = note: `#[warn(unexpected_cfgs)]` on by default

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just remove this line if the permanent fix is the #[rustfmt::skip] in parquet/src/lib.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking it would be good to leave the comment explaining why the change as this file seems to be auto generated

// Autogenerated by Thrift Compiler (0.19.0)
// DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING

@alamb alamb marked this pull request as ready for review May 6, 2024 15:21
@@ -5,7 +5,8 @@
#![allow(unused_imports)]
#![allow(unused_extern_crates)]
#![allow(clippy::too_many_arguments, clippy::type_complexity, clippy::vec_box, clippy::wrong_self_convention)]
#![cfg_attr(rustfmt, rustfmt_skip)]
// Fix unexpected `cfg` condition name: `rustfmt` https://github.com/apache/arrow-rs/issues/5725
//#![cfg_attr(rustfmt, rustfmt_skip)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just remove this line if the permanent fix is the #[rustfmt::skip] in parquet/src/lib.rs?

@@ -80,15 +80,6 @@ pub const ALIGNMENT: usize = 1 << 5;
#[cfg(target_arch = "sparc64")]
pub const ALIGNMENT: usize = 1 << 6;

// On ARM cache line sizes are fixed. both v6 and v7.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. I tried searching up to understand more but can't really find much to point towards existence of those target_arch values, even though targets like thumbv6m-none-eabi exist

@alamb
Copy link
Contributor Author

alamb commented May 7, 2024

Thanks for the review @Jefffrey -- I am going to merge this one in to get the CI back a little cleaner

@alamb alamb merged commit 98784bd into apache:master May 7, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate development-process Related to development process of arrow-rs parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Rustdocs are clean (amd64, nightly)" CI check is failing
2 participants