From 568bad7e7f54df5a4c6c9c27352fe838875c34bc Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sat, 6 Jul 2024 16:48:49 +0800 Subject: [PATCH 1/9] chore: Use nightly toolchain for check --- Makefile | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index ff01e1807..de180d2da 100644 --- a/Makefile +++ b/Makefile @@ -19,25 +19,31 @@ RUST_LOG = debug +RUST_NIGHTLY_CHANNEL = nightly-2024-06-10 + build: - cargo build + cargo build --all-targets --all-features --workspace + +install-night-toolchain: + rustup toolchain install $(RUST_NIGHTLY_CHANNEL) -check-fmt: - cargo fmt --all -- --check +check-fmt: install-night-toolchain + cargo +$(RUST_NIGHTLY_CHANNEL) fmt --all -- --check -check-clippy: - cargo clippy --all-targets --all-features --workspace -- -D warnings +check-clippy: install-night-toolchain + cargo +$(RUST_NIGHTLY_CHANNEL) clippy --all-targets --all-features --workspace -- -D warnings cargo-sort: cargo install cargo-sort cargo sort -c -w -fix-toml: +install-taplo-cli: cargo install taplo-cli --locked + +fix-toml: install-taplo-cli taplo fmt -check-toml: - cargo install taplo-cli --locked +check-toml: install-taplo-cli taplo check check: check-fmt check-clippy cargo-sort check-toml From bd78137de69d754bede52b82f06587c71c569bcc Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sat, 6 Jul 2024 16:58:27 +0800 Subject: [PATCH 2/9] Fix check --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index de180d2da..21baba03c 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ build: cargo build --all-targets --all-features --workspace install-night-toolchain: - rustup toolchain install $(RUST_NIGHTLY_CHANNEL) + rustup toolchain install $(RUST_NIGHTLY_CHANNEL) -c rustfmt -c clippy check-fmt: install-night-toolchain cargo +$(RUST_NIGHTLY_CHANNEL) fmt --all -- --check From 3322d3c3ee38e5eb4d48b7750876f38bcb452fe7 Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sat, 6 Jul 2024 17:10:14 +0800 Subject: [PATCH 3/9] Fix clippy finds --- crates/iceberg/src/avro/schema.rs | 327 +++++++++--------- .../src/writer/file_writer/parquet_writer.rs | 3 - crates/integrations/datafusion/src/schema.rs | 2 +- 3 files changed, 165 insertions(+), 167 deletions(-) diff --git a/crates/iceberg/src/avro/schema.rs b/crates/iceberg/src/avro/schema.rs index d83cdc36b..79cbbcc23 100644 --- a/crates/iceberg/src/avro/schema.rs +++ b/crates/iceberg/src/avro/schema.rs @@ -19,10 +19,10 @@ use std::collections::BTreeMap; use crate::spec::{ - visit_schema, ListType, MapType, NestedField, NestedFieldRef, PrimitiveType, Schema, - SchemaVisitor, StructType, Type, + visit_schema, ListType, MapType, NestedFieldRef, PrimitiveType, Schema, SchemaVisitor, + StructType }; -use crate::{ensure_data_valid, Error, ErrorKind, Result}; +use crate::{Error, ErrorKind, Result}; use apache_avro::schema::{ DecimalSchema, FixedSchema, Name, RecordField as AvroRecordField, RecordFieldOrder, RecordSchema, UnionSchema, @@ -272,205 +272,206 @@ fn avro_optional(avro_schema: AvroSchema) -> Result { ])?)) } -fn is_avro_optional(avro_schema: &AvroSchema) -> bool { - match avro_schema { - AvroSchema::Union(union) => union.is_nullable(), - _ => false, + +#[cfg(test)] +mod tests { + use super::*; + use crate::spec::{ListType, MapType, NestedField, PrimitiveType, Schema, StructType, Type}; + use apache_avro::schema::{Namespace, UnionSchema}; + use apache_avro::Schema as AvroSchema; + use std::fs::read_to_string; + use crate::ensure_data_valid; + + fn is_avro_optional(avro_schema: &AvroSchema) -> bool { + match avro_schema { + AvroSchema::Union(union) => union.is_nullable(), + _ => false, + } } -} -/// Post order avro schema visitor. -pub(crate) trait AvroSchemaVisitor { - type T; + /// Post order avro schema visitor. + pub(crate) trait AvroSchemaVisitor { + type T; - fn record(&mut self, record: &RecordSchema, fields: Vec) -> Result; + fn record(&mut self, record: &RecordSchema, fields: Vec) -> Result; - fn union(&mut self, union: &UnionSchema, options: Vec) -> Result; + fn union(&mut self, union: &UnionSchema, options: Vec) -> Result; - fn array(&mut self, array: &AvroSchema, item: Self::T) -> Result; - fn map(&mut self, map: &AvroSchema, value: Self::T) -> Result; + fn array(&mut self, array: &AvroSchema, item: Self::T) -> Result; + fn map(&mut self, map: &AvroSchema, value: Self::T) -> Result; - fn primitive(&mut self, schema: &AvroSchema) -> Result; -} + fn primitive(&mut self, schema: &AvroSchema) -> Result; + } -struct AvroSchemaToSchema { - next_id: i32, -} + struct AvroSchemaToSchema { + next_id: i32, + } -impl AvroSchemaToSchema { - fn next_field_id(&mut self) -> i32 { - self.next_id += 1; - self.next_id + impl AvroSchemaToSchema { + fn next_field_id(&mut self) -> i32 { + self.next_id += 1; + self.next_id + } } -} -impl AvroSchemaVisitor for AvroSchemaToSchema { - // Only `AvroSchema::Null` will return `None` - type T = Option; + impl AvroSchemaVisitor for AvroSchemaToSchema { + // Only `AvroSchema::Null` will return `None` + type T = Option; + + fn record( + &mut self, + record: &RecordSchema, + field_types: Vec>, + ) -> Result> { + let mut fields = Vec::with_capacity(field_types.len()); + for (avro_field, typ) in record.fields.iter().zip_eq(field_types) { + let field_id = avro_field + .custom_attributes + .get(FILED_ID_PROP) + .and_then(Value::as_i64) + .ok_or_else(|| { + Error::new( + ErrorKind::DataInvalid, + format!("Can't convert field, missing field id: {avro_field:?}"), + ) + })?; - fn record( - &mut self, - record: &RecordSchema, - field_types: Vec>, - ) -> Result> { - let mut fields = Vec::with_capacity(field_types.len()); - for (avro_field, typ) in record.fields.iter().zip_eq(field_types) { - let field_id = avro_field - .custom_attributes - .get(FILED_ID_PROP) - .and_then(Value::as_i64) - .ok_or_else(|| { - Error::new( - ErrorKind::DataInvalid, - format!("Can't convert field, missing field id: {avro_field:?}"), - ) - })?; + let optional = is_avro_optional(&avro_field.schema); - let optional = is_avro_optional(&avro_field.schema); + let mut field = if optional { + NestedField::optional(field_id as i32, &avro_field.name, typ.unwrap()) + } else { + NestedField::required(field_id as i32, &avro_field.name, typ.unwrap()) + }; - let mut field = if optional { - NestedField::optional(field_id as i32, &avro_field.name, typ.unwrap()) - } else { - NestedField::required(field_id as i32, &avro_field.name, typ.unwrap()) - }; + if let Some(doc) = &avro_field.doc { + field = field.with_doc(doc); + } - if let Some(doc) = &avro_field.doc { - field = field.with_doc(doc); + fields.push(field.into()); } - fields.push(field.into()); + Ok(Some(Type::Struct(StructType::new(fields)))) } - Ok(Some(Type::Struct(StructType::new(fields)))) - } - - fn union( - &mut self, - union: &UnionSchema, - mut options: Vec>, - ) -> Result> { - ensure_data_valid!( + fn union( + &mut self, + union: &UnionSchema, + mut options: Vec>, + ) -> Result> { + ensure_data_valid!( options.len() <= 2 && !options.is_empty(), "Can't convert avro union type {:?} to iceberg.", union ); - if options.len() > 1 { - ensure_data_valid!( + if options.len() > 1 { + ensure_data_valid!( options[0].is_none(), "Can't convert avro union type {:?} to iceberg.", union ); - } + } - if options.len() == 1 { - Ok(Some(options.remove(0).unwrap())) - } else { - Ok(Some(options.remove(1).unwrap())) + if options.len() == 1 { + Ok(Some(options.remove(0).unwrap())) + } else { + Ok(Some(options.remove(1).unwrap())) + } } - } - fn array(&mut self, array: &AvroSchema, item: Option) -> Result { - if let AvroSchema::Array(item_schema) = array { - let element_field = NestedField::list_element( - self.next_field_id(), - item.unwrap(), - !is_avro_optional(item_schema), - ) - .into(); - Ok(Some(Type::List(ListType { element_field }))) - } else { - Err(Error::new( - ErrorKind::Unexpected, - "Expected avro array schema, but {array}", - )) + fn array(&mut self, array: &AvroSchema, item: Option) -> Result { + if let AvroSchema::Array(item_schema) = array { + let element_field = NestedField::list_element( + self.next_field_id(), + item.unwrap(), + !is_avro_optional(item_schema), + ) + .into(); + Ok(Some(Type::List(ListType { element_field }))) + } else { + Err(Error::new( + ErrorKind::Unexpected, + "Expected avro array schema, but {array}", + )) + } } - } - fn map(&mut self, map: &AvroSchema, value: Option) -> Result> { - if let AvroSchema::Map(value_schema) = map { - // Due to avro rust implementation's limitation, we can't store attributes in map schema, - // we will fix it later when it has been resolved. - let key_field = NestedField::map_key_element( - self.next_field_id(), - Type::Primitive(PrimitiveType::String), - ); - let value_field = NestedField::map_value_element( - self.next_field_id(), - value.unwrap(), - !is_avro_optional(value_schema), - ); - Ok(Some(Type::Map(MapType { - key_field: key_field.into(), - value_field: value_field.into(), - }))) - } else { - Err(Error::new( - ErrorKind::Unexpected, - "Expected avro map schema, but {map}", - )) + fn map(&mut self, map: &AvroSchema, value: Option) -> Result> { + if let AvroSchema::Map(value_schema) = map { + // Due to avro rust implementation's limitation, we can't store attributes in map schema, + // we will fix it later when it has been resolved. + let key_field = NestedField::map_key_element( + self.next_field_id(), + Type::Primitive(PrimitiveType::String), + ); + let value_field = NestedField::map_value_element( + self.next_field_id(), + value.unwrap(), + !is_avro_optional(value_schema), + ); + Ok(Some(Type::Map(MapType { + key_field: key_field.into(), + value_field: value_field.into(), + }))) + } else { + Err(Error::new( + ErrorKind::Unexpected, + "Expected avro map schema, but {map}", + )) + } } - } - fn primitive(&mut self, schema: &AvroSchema) -> Result> { - let typ = match schema { - AvroSchema::Decimal(decimal) => { - Type::decimal(decimal.precision as u32, decimal.scale as u32)? - } - AvroSchema::Date => Type::Primitive(PrimitiveType::Date), - AvroSchema::TimeMicros => Type::Primitive(PrimitiveType::Time), - AvroSchema::TimestampMicros => Type::Primitive(PrimitiveType::Timestamp), - AvroSchema::Boolean => Type::Primitive(PrimitiveType::Boolean), - AvroSchema::Int => Type::Primitive(PrimitiveType::Int), - AvroSchema::Long => Type::Primitive(PrimitiveType::Long), - AvroSchema::Float => Type::Primitive(PrimitiveType::Float), - AvroSchema::Double => Type::Primitive(PrimitiveType::Double), - AvroSchema::String | AvroSchema::Enum(_) => Type::Primitive(PrimitiveType::String), - AvroSchema::Fixed(fixed) => { - if let Some(logical_type) = fixed.attributes.get(LOGICAL_TYPE) { - let logical_type = logical_type.as_str().ok_or_else(|| { - Error::new( - ErrorKind::DataInvalid, - "logicalType in attributes of avro schema is not a string type", - ) - })?; - match logical_type { - UUID_LOGICAL_TYPE => Type::Primitive(PrimitiveType::Uuid), - ty => { - return Err(Error::new( - ErrorKind::FeatureUnsupported, - format!( - "Logical type {ty} is not support in iceberg primitive type.", - ), - )) + fn primitive(&mut self, schema: &AvroSchema) -> Result> { + let typ = match schema { + AvroSchema::Decimal(decimal) => { + Type::decimal(decimal.precision as u32, decimal.scale as u32)? + } + AvroSchema::Date => Type::Primitive(PrimitiveType::Date), + AvroSchema::TimeMicros => Type::Primitive(PrimitiveType::Time), + AvroSchema::TimestampMicros => Type::Primitive(PrimitiveType::Timestamp), + AvroSchema::Boolean => Type::Primitive(PrimitiveType::Boolean), + AvroSchema::Int => Type::Primitive(PrimitiveType::Int), + AvroSchema::Long => Type::Primitive(PrimitiveType::Long), + AvroSchema::Float => Type::Primitive(PrimitiveType::Float), + AvroSchema::Double => Type::Primitive(PrimitiveType::Double), + AvroSchema::String | AvroSchema::Enum(_) => Type::Primitive(PrimitiveType::String), + AvroSchema::Fixed(fixed) => { + if let Some(logical_type) = fixed.attributes.get(LOGICAL_TYPE) { + let logical_type = logical_type.as_str().ok_or_else(|| { + Error::new( + ErrorKind::DataInvalid, + "logicalType in attributes of avro schema is not a string type", + ) + })?; + match logical_type { + UUID_LOGICAL_TYPE => Type::Primitive(PrimitiveType::Uuid), + ty => { + return Err(Error::new( + ErrorKind::FeatureUnsupported, + format!( + "Logical type {ty} is not support in iceberg primitive type.", + ), + )) + } } + } else { + Type::Primitive(PrimitiveType::Fixed(fixed.size as u64)) } - } else { - Type::Primitive(PrimitiveType::Fixed(fixed.size as u64)) } - } - AvroSchema::Bytes => Type::Primitive(PrimitiveType::Binary), - AvroSchema::Null => return Ok(None), - _ => { - return Err(Error::new( - ErrorKind::Unexpected, - "Unable to convert avro {schema} to iceberg primitive type.", - )) - } - }; + AvroSchema::Bytes => Type::Primitive(PrimitiveType::Binary), + AvroSchema::Null => return Ok(None), + _ => { + return Err(Error::new( + ErrorKind::Unexpected, + "Unable to convert avro {schema} to iceberg primitive type.", + )) + } + }; - Ok(Some(typ)) + Ok(Some(typ)) + } } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::avro::schema::AvroSchemaToSchema; - use crate::spec::{ListType, MapType, NestedField, PrimitiveType, Schema, StructType, Type}; - use apache_avro::schema::{Namespace, UnionSchema}; - use apache_avro::Schema as AvroSchema; - use std::fs::read_to_string; /// Visit avro schema in post order visitor. pub(crate) fn visit( diff --git a/crates/iceberg/src/writer/file_writer/parquet_writer.rs b/crates/iceberg/src/writer/file_writer/parquet_writer.rs index 43e49fc8b..5f50e417d 100644 --- a/crates/iceberg/src/writer/file_writer/parquet_writer.rs +++ b/crates/iceberg/src/writer/file_writer/parquet_writer.rs @@ -627,9 +627,6 @@ mod tests { use crate::writer::file_writer::location_generator::DefaultFileNameGenerator; use crate::writer::tests::check_parquet_data_file; - #[derive(Clone)] - struct TestLocationGen; - fn schema_for_all_type() -> Schema { Schema::builder() .with_schema_id(1) diff --git a/crates/integrations/datafusion/src/schema.rs b/crates/integrations/datafusion/src/schema.rs index 2ba69621a..f7b1a21d2 100644 --- a/crates/integrations/datafusion/src/schema.rs +++ b/crates/integrations/datafusion/src/schema.rs @@ -89,7 +89,7 @@ impl SchemaProvider for IcebergSchemaProvider { } fn table_exist(&self, name: &str) -> bool { - self.tables.get(name).is_some() + self.tables.contains_key(name) } async fn table(&self, name: &str) -> DFResult>> { From f707cae1d35ba80451034c92f66cb6201d14e8a1 Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sat, 6 Jul 2024 17:13:39 +0800 Subject: [PATCH 4/9] Make rustfmt happy --- crates/iceberg/src/avro/schema.rs | 37 ++++++++++++++----------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/crates/iceberg/src/avro/schema.rs b/crates/iceberg/src/avro/schema.rs index 79cbbcc23..f38ef6507 100644 --- a/crates/iceberg/src/avro/schema.rs +++ b/crates/iceberg/src/avro/schema.rs @@ -20,7 +20,7 @@ use std::collections::BTreeMap; use crate::spec::{ visit_schema, ListType, MapType, NestedFieldRef, PrimitiveType, Schema, SchemaVisitor, - StructType + StructType, }; use crate::{Error, ErrorKind, Result}; use apache_avro::schema::{ @@ -272,15 +272,14 @@ fn avro_optional(avro_schema: AvroSchema) -> Result { ])?)) } - #[cfg(test)] mod tests { use super::*; + use crate::ensure_data_valid; use crate::spec::{ListType, MapType, NestedField, PrimitiveType, Schema, StructType, Type}; use apache_avro::schema::{Namespace, UnionSchema}; use apache_avro::Schema as AvroSchema; use std::fs::read_to_string; - use crate::ensure_data_valid; fn is_avro_optional(avro_schema: &AvroSchema) -> bool { match avro_schema { @@ -360,17 +359,17 @@ mod tests { mut options: Vec>, ) -> Result> { ensure_data_valid!( - options.len() <= 2 && !options.is_empty(), - "Can't convert avro union type {:?} to iceberg.", - union - ); - - if options.len() > 1 { - ensure_data_valid!( - options[0].is_none(), + options.len() <= 2 && !options.is_empty(), "Can't convert avro union type {:?} to iceberg.", union ); + + if options.len() > 1 { + ensure_data_valid!( + options[0].is_none(), + "Can't convert avro union type {:?} to iceberg.", + union + ); } if options.len() == 1 { @@ -387,7 +386,7 @@ mod tests { item.unwrap(), !is_avro_optional(item_schema), ) - .into(); + .into(); Ok(Some(Type::List(ListType { element_field }))) } else { Err(Error::new( @@ -446,14 +445,12 @@ mod tests { })?; match logical_type { UUID_LOGICAL_TYPE => Type::Primitive(PrimitiveType::Uuid), - ty => { - return Err(Error::new( - ErrorKind::FeatureUnsupported, - format!( - "Logical type {ty} is not support in iceberg primitive type.", - ), - )) - } + ty => return Err(Error::new( + ErrorKind::FeatureUnsupported, + format!( + "Logical type {ty} is not support in iceberg primitive type.", + ), + )), } } else { Type::Primitive(PrimitiveType::Fixed(fixed.size as u64)) From a0ab8e6d23e349fcdb7aaf6c80e365b2fbf0c0a6 Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sat, 6 Jul 2024 17:15:33 +0800 Subject: [PATCH 5/9] Make rustfmt happy --- crates/iceberg/src/avro/schema.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/avro/schema.rs b/crates/iceberg/src/avro/schema.rs index f38ef6507..11d000cc5 100644 --- a/crates/iceberg/src/avro/schema.rs +++ b/crates/iceberg/src/avro/schema.rs @@ -445,12 +445,14 @@ mod tests { })?; match logical_type { UUID_LOGICAL_TYPE => Type::Primitive(PrimitiveType::Uuid), - ty => return Err(Error::new( - ErrorKind::FeatureUnsupported, - format!( + ty => { + return Err(Error::new( + ErrorKind::FeatureUnsupported, + format!( "Logical type {ty} is not support in iceberg primitive type.", ), - )), + )) + } } } else { Type::Primitive(PrimitiveType::Fixed(fixed.size as u64)) From dc430da999ef2d029bb94520743fd11e4c82df71 Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sun, 7 Jul 2024 11:25:40 +0800 Subject: [PATCH 6/9] Update github actions --- .github/workflows/ci.yml | 40 ++++++++++++++++++++++++++++++++++- .github/workflows/publish.yml | 12 +++++++++++ Makefile | 19 +++++++---------- rust-toolchain.toml | 2 +- 4 files changed, 60 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5be609c0b..db1261297 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,10 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }} cancel-in-progress: true +env: + rust_nightly: "nightly-2024-06-10" + rust_min: "1.77.1" + jobs: check: runs-on: ubuntu-latest @@ -38,6 +42,25 @@ jobs: - name: Check License Header uses: apache/skywalking-eyes/header@v0.6.0 + - name: Install Rust ${{ env.rust_nightly }} + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ env.rust_nightly }} + + - name: Install cargo-sort + uses: baptiste0928/cargo-install@v3 + with: + crate: cargo-sort + version: '1.0.9' + + - name: Install taplo-cli + uses: baptiste0928/cargo-install@v3 + with: + crate: taplo-cli + version: '0.9.0' + + - uses: Swatinem/rust-cache@v2 + - name: Cargo format run: make check-fmt @@ -61,14 +84,29 @@ jobs: - windows-latest steps: - uses: actions/checkout@v4 + + - name: Install Rust ${{ env.rust_min }} + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ env.rust_min }} + + - uses: Swatinem/rust-cache@v2 + - name: Build - run: cargo build + run: make build unit: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - name: Install Rust ${{ env.rust_min }} + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ env.rust_min }} + + - uses: Swatinem/rust-cache@v2 + - name: Test run: cargo test --no-fail-fast --all-targets --all-features --workspace diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 9289d3e10..7dc6910fe 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -28,6 +28,10 @@ on: - ".github/workflows/publish.yml" workflow_dispatch: + +env: + rust_min: "1.77.1" + jobs: publish: runs-on: ubuntu-latest @@ -42,6 +46,14 @@ jobs: - "crates/catalog/rest" steps: - uses: actions/checkout@v4 + + - name: Install Rust ${{ env.rust_min }} + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ env.rust_min }} + + - uses: Swatinem/rust-cache@v2 + - name: Dryrun ${{ matrix.package }} working-directory: ${{ matrix.package }} run: cargo publish --all-features --dry-run diff --git a/Makefile b/Makefile index 21baba03c..9f988263d 100644 --- a/Makefile +++ b/Makefile @@ -19,26 +19,23 @@ RUST_LOG = debug -RUST_NIGHTLY_CHANNEL = nightly-2024-06-10 - build: cargo build --all-targets --all-features --workspace -install-night-toolchain: - rustup toolchain install $(RUST_NIGHTLY_CHANNEL) -c rustfmt -c clippy +check-fmt: + cargo fmt --all -- --check -check-fmt: install-night-toolchain - cargo +$(RUST_NIGHTLY_CHANNEL) fmt --all -- --check +check-clippy: + cargo clippy --all-targets --all-features --workspace -- -D warnings -check-clippy: install-night-toolchain - cargo +$(RUST_NIGHTLY_CHANNEL) clippy --all-targets --all-features --workspace -- -D warnings +install-cargo-sort: + cargo install cargo-sort@1.0.9 -cargo-sort: - cargo install cargo-sort +cargo-sort: install-cargo-sort cargo sort -c -w install-taplo-cli: - cargo install taplo-cli --locked + cargo install taplo-cli@0.9.0 fix-toml: install-taplo-cli taplo fmt diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 6685489a6..7b10a8692 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -16,5 +16,5 @@ # under the License. [toolchain] -channel = "1.77.1" +channel = "nightly-2024-06-10" components = ["rustfmt", "clippy"] From 486214463600e8b80dc61ff9e088b6c7e589dd8a Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Sun, 7 Jul 2024 11:48:06 +0800 Subject: [PATCH 7/9] Use action builder since apache doesn't allow external actions --- .github/actions/setup-builder/action.yml | 38 ++++++++++++++++++++++++ .github/workflows/ci.yml | 34 +++++++-------------- .github/workflows/publish.yml | 8 ++--- 3 files changed, 52 insertions(+), 28 deletions(-) create mode 100644 .github/actions/setup-builder/action.yml diff --git a/.github/actions/setup-builder/action.yml b/.github/actions/setup-builder/action.yml new file mode 100644 index 000000000..0af9ecf93 --- /dev/null +++ b/.github/actions/setup-builder/action.yml @@ -0,0 +1,38 @@ +# 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. + +name: Prepare Rust Builder +description: 'Prepare Rust Build Environment' +inputs: + rust-version: + description: 'version of rust to install (e.g. stable)' + required: true + default: 'stable' +runs: + using: "composite" + steps: + - name: Setup Rust toolchain + shell: bash + run: | + echo "Installing ${{ inputs.rust-version }}" + rustup toolchain install ${{ inputs.rust-version }} + rustup default ${{ inputs.rust-version }} + rustup component add rustfmt clippy + - name: Fixup git permissions + # https://github.com/actions/checkout/issues/766 + shell: bash + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db1261297..f54604728 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,24 +42,16 @@ jobs: - name: Check License Header uses: apache/skywalking-eyes/header@v0.6.0 - - name: Install Rust ${{ env.rust_nightly }} - uses: dtolnay/rust-toolchain@stable + - name: Setup Rust toolchain + uses: ./.github/actions/setup-builder with: - toolchain: ${{ env.rust_nightly }} + rust-version: ${{ env.rust_nightly }} - name: Install cargo-sort - uses: baptiste0928/cargo-install@v3 - with: - crate: cargo-sort - version: '1.0.9' + run: make install-cargo-sort - name: Install taplo-cli - uses: baptiste0928/cargo-install@v3 - with: - crate: taplo-cli - version: '0.9.0' - - - uses: Swatinem/rust-cache@v2 + run: make install-taplo-cli - name: Cargo format run: make check-fmt @@ -85,12 +77,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install Rust ${{ env.rust_min }} - uses: dtolnay/rust-toolchain@stable + - name: Setup Rust toolchain + uses: ./.github/actions/setup-builder with: - toolchain: ${{ env.rust_min }} - - - uses: Swatinem/rust-cache@v2 + rust-version: ${{ env.rust_min }} - name: Build run: make build @@ -100,12 +90,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install Rust ${{ env.rust_min }} - uses: dtolnay/rust-toolchain@stable + - name: Setup Rust toolchain + uses: ./.github/actions/setup-builder with: - toolchain: ${{ env.rust_min }} - - - uses: Swatinem/rust-cache@v2 + rust-version: ${{ env.rust_min }} - name: Test run: cargo test --no-fail-fast --all-targets --all-features --workspace diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 7dc6910fe..7693a570a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -47,13 +47,11 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install Rust ${{ env.rust_min }} - uses: dtolnay/rust-toolchain@stable + - name: Setup Rust toolchain + uses: ./.github/actions/setup-builder with: - toolchain: ${{ env.rust_min }} + rust-version: ${{ env.rust_min }} - - uses: Swatinem/rust-cache@v2 - - name: Dryrun ${{ matrix.package }} working-directory: ${{ matrix.package }} run: cargo publish --all-features --dry-run From 9e9f9c6cb9c51f6fcee9c39b0b304abecc195411 Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Mon, 8 Jul 2024 20:51:51 +0800 Subject: [PATCH 8/9] Fix comments --- .github/actions/setup-builder/action.yml | 2 ++ .github/workflows/ci.yml | 12 +++--------- .github/workflows/publish.yml | 14 ++------------ 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/.github/actions/setup-builder/action.yml b/.github/actions/setup-builder/action.yml index 0af9ecf93..43de1cbaa 100644 --- a/.github/actions/setup-builder/action.yml +++ b/.github/actions/setup-builder/action.yml @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +# This file is heavily inspired by +# [datafusion](https://github.com/apache/datafusion/blob/main/.github/actions/setup-builder/action.yaml). name: Prepare Rust Builder description: 'Prepare Rust Build Environment' inputs: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f54604728..35d848e12 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,8 +30,7 @@ concurrency: cancel-in-progress: true env: - rust_nightly: "nightly-2024-06-10" - rust_min: "1.77.1" + rust_msrv: "1.77.1" jobs: check: @@ -42,11 +41,6 @@ jobs: - name: Check License Header uses: apache/skywalking-eyes/header@v0.6.0 - - name: Setup Rust toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.rust_nightly }} - - name: Install cargo-sort run: make install-cargo-sort @@ -80,7 +74,7 @@ jobs: - name: Setup Rust toolchain uses: ./.github/actions/setup-builder with: - rust-version: ${{ env.rust_min }} + rust-version: ${{ env.rust_msrv }} - name: Build run: make build @@ -93,7 +87,7 @@ jobs: - name: Setup Rust toolchain uses: ./.github/actions/setup-builder with: - rust-version: ${{ env.rust_min }} + rust-version: ${{ env.rust_msrv }} - name: Test run: cargo test --no-fail-fast --all-targets --all-features --workspace diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 7693a570a..a57aa612f 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -21,16 +21,10 @@ on: push: tags: - '*' - pull_request: - branches: - - main - paths: - - ".github/workflows/publish.yml" workflow_dispatch: - env: - rust_min: "1.77.1" + rust_msrv: "1.77.1" jobs: publish: @@ -50,11 +44,7 @@ jobs: - name: Setup Rust toolchain uses: ./.github/actions/setup-builder with: - rust-version: ${{ env.rust_min }} - - - name: Dryrun ${{ matrix.package }} - working-directory: ${{ matrix.package }} - run: cargo publish --all-features --dry-run + rust-version: ${{ env.rust_msrv }} - name: Publish ${{ matrix.package }} working-directory: ${{ matrix.package }} From 7896c83ded120196c0e39046be385f9faf81959e Mon Sep 17 00:00:00 2001 From: liurenjie1024 Date: Mon, 8 Jul 2024 21:02:01 +0800 Subject: [PATCH 9/9] Fix README.md --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index 3f4f7a334..4f8265b79 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,15 @@ The Apache Iceberg Rust project is composed of the following components: [iceberg-catalog-rest link]: https://crates.io/crates/iceberg-catalog-rest [iceberg-catalog-rest release docs]: https://docs.rs/iceberg-catalog-rest +## Supported Rust Version + +Iceberg Rust is built and tested with stable rust, and will keep a rolling MSRV(minimum supported rust version). The +current MSRV is 1.77.1. + +Also, we use unstable rust to run linters, such as `clippy` and `rustfmt`. But this will not affect downstream users, +and only MSRV is required. + + ## Contribute Apache Iceberg is an active open-source project, governed under the Apache Software Foundation (ASF). We are always open to people who want to use or contribute to it. Here are some ways to get involved.