github-actions[bot] commented on code in PR #40508: URL: https://github.com/apache/beam/pull/40508#discussion_r4367520723
########## sdks/rust/gradle/cargo.gradle: ########## @@ -0,0 +1,58 @@ +/* + * 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. + */ + +// Lifecycle tasks: build, check (rustfmt + clippy), test, fmt and clean. +// +// The tasks always run cargo. Cargo has its own incremental build, so a run with nothing to +// do is fast, and its result is always the real result of the command. + +ext.rustDir = file("${rootDir}/sdks/rust") + +// -Ppkg=<crate> narrows `test` to one crate; the default is the whole workspace. Review Comment: **🟢 Low**: In Groovy/Java, calling `.split(/\s+/)` on an empty string `""` returns a single-element array `[""]` rather than an empty array. If a user invokes Gradle with `-PtestArgs=""` (or whitespace), `testArgs` evaluates as truthy and passes `['--', '']` to `cargo test`, which filters test names by an empty argument or fails argument parsing. Guarding on a non-empty trimmed string avoids this. ```suggestion ? (project.property('testArgs').toString().trim() ? project.property('testArgs').toString().trim().split(/\s+/) as List : []) ``` ########## sdks/rust/scripts/set_version.sh: ########## @@ -0,0 +1,65 @@ +#!/bin/bash +# +# 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. +# + +# Sets the version of every crate in the Rust SDK workspace. +# +# Usage: set_version.sh <version> +# +# Called by release/src/main/scripts/set_version.sh with a release version +# (X.Y.Z) or a snapshot version (X.Y.Z-SNAPSHOT). Cargo requires semver, so +# snapshots use a -SNAPSHOT pre-release suffix. +# +# Updates: +# - [workspace.package] version in Cargo.toml, +# - the version on every internal path dependency in any Cargo.toml, +# - workspace member entries in Cargo.lock ([[package]] blocks without a +# `source`), so `--locked` builds work without running cargo. + +set -euo pipefail + +if [[ $# -ne 1 || -z "$1" ]]; then + echo "Usage: $0 <version>" >&2 + exit 1 +fi + +VERSION="$1" +RUST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +# Writes the output of a filter over a file back to that file. +rewrite() { + local file="$1" + shift + "$@" < "$file" > "$file.tmp" + mv "$file.tmp" "$file" +} + +find "$RUST_ROOT" -name Cargo.toml -not -path '*/target/*' -not -path '*/build/*' -print \ + | while IFS= read -r manifest; do + rewrite "$manifest" sed -E \ + -e "s/^version = \"[^\"]*\"$/version = \"$VERSION\"/" \ + -e "s/(path = \"[^\"]*\", version = )\"[^\"]*\"/\1\"$VERSION\"/" + done Review Comment: **🟡 Medium**: This regex requires `path` to appear immediately before `version` (`path = "...", version = "..."`). If a workspace/local dependency is written with `version` before `path` (e.g. `{ version = "...", path = "..." }`, which `cargo add` produces by default) or has another key in between (such as `default-features = false`), `set_version.sh` will silently skip updating its version. Using a line address match on `path = "` before substituting `version = "..."` handles any key ordering in inline path dependency tables. ```suggestion -e "/path = \"/s/version = \"[^\"]*\"/version = \"$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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
