Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 61 additions & 5 deletions .github/release/partial_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,20 @@ def main(ctx):
@main.command()
@click.option(
"--artifact-ids",
required=True,
required=False,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The --artifact-ids option is now optional, but it lacks a default value. If this option is omitted and --sdk-platform-java is not set, artifact_ids will be None, which will cause an AttributeError when bump_version attempts to call artifact_ids.split(","). Adding default="" ensures the variable is always a string and avoids the crash.

Suggested change
required=False,
required=False,
default="",

type=str,
help="""
Artifact IDs whose version needs to update, separated by comma.
""",
)
@click.option(
"--sdk-platform-java",
is_flag=True,
default=False,
help="""
If set, bump versions for all modules in sdk-platform-java.
""",
)
@click.option(
"--versions",
required=False,
Expand All @@ -53,18 +61,29 @@ def main(ctx):
The path to the versions.txt.
""",
)
def bump_snapshot_version(artifact_ids: str, versions: str) -> None:
def bump_snapshot_version(artifact_ids: str, sdk_platform_java: bool, versions: str) -> None:
if sdk_platform_java:
artifact_ids = _get_sdk_platform_java_artifacts(artifact_ids)
bump_version(artifact_ids, "snapshot", versions)


@main.command()
@click.option(
"--artifact-ids",
required=True,
required=False,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Similar to the bump-snapshot-version command, the --artifact-ids option here should have a default value to prevent a NoneType error in bump_version when the flag is omitted.

Suggested change
required=False,
required=False,
default="",

type=str,
help="""
Artifact IDs whose version needs to update, separated by comma.
""",
)
@click.option(
"--sdk-platform-java",
is_flag=True,
default=False,
help="""
If set, bump versions for all modules in sdk-platform-java.
""",
)
@click.option(
"--version-type",
required=False,
Expand All @@ -83,9 +102,45 @@ def bump_snapshot_version(artifact_ids: str, versions: str) -> None:
The path to the versions.txt.
""",
)
def bump_released_version(artifact_ids: str, version_type: str, versions: str) -> None:
def bump_released_version(
artifact_ids: str, sdk_platform_java: bool, version_type: str, versions: str
) -> None:
if sdk_platform_java:
artifact_ids = _get_sdk_platform_java_artifacts(artifact_ids)
bump_version(artifact_ids, version_type, versions)


def _get_sdk_platform_java_artifacts(artifact_ids: str) -> str:
sdk_artifacts = [
"gapic-generator-java",
"api-common",
"gax",
"gax-grpc",
"gax-httpjson",
"proto-google-common-protos",
"grpc-google-common-protos",
"proto-google-iam-v1",
"grpc-google-iam-v1",
"proto-google-iam-v2beta",
"grpc-google-iam-v2beta",
"proto-google-iam-v2",
"grpc-google-iam-v2",
"proto-google-iam-v3",
"grpc-google-iam-v3",
"proto-google-iam-v3beta",
"grpc-google-iam-v3beta",
"google-cloud-core",
"google-cloud-shared-dependencies",
"google-iam-policy",
"gapic-showcase",
"proto-gapic-showcase-v1beta1",
"grpc-gapic-showcase-v1beta1",
]
if artifact_ids:
return ",".join(sdk_artifacts + artifact_ids.split(","))
return ",".join(sdk_artifacts)


def bump_version(artifact_ids: str, version_type: str, versions: str) -> None:
target_artifact_ids = set(artifact_ids.split(","))
version_enum = _parse_type_or_raise(version_type)
Expand Down Expand Up @@ -130,7 +185,8 @@ def bump_version(artifact_ids: str, version_type: str, versions: str) -> None:
f"{artifact_id}:{major}.{minor}.{patch}:{major}.{minor}.{patch}"
)
with open(versions, "w") as versions_file:
versions_file.writelines("\n".join(newlines))
for line in newlines:
versions_file.write(f"{line}\n")


def _parse_type_or_raise(version_type: str) -> VersionType:
Expand Down
31 changes: 22 additions & 9 deletions generation/apply_versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,38 @@ if [[ -z "$versions_file" || -z "$column_name" ]]; then
echo "Usage: $0 path/to/versions.txt (released|current)"
exit 1
fi
if [[ "$column_name" == "released" ]]; then
if [[ "$column_name" == "released" ]]; then
column_index=2
elif [[ "$column_name" == "current" ]]; then
column_index=3
elif "$column_name" != "current" ]]; then
else
echo "Error: column_name must be either 'released' or 'current'"
exit 1
fi

SED_SCRIPT=$(mktemp)
trap "rm -f $SED_SCRIPT" EXIT

SED_OPTIONS=""

# The second column is
for KV in $(cut -f1,"${column_index}" -d: $versions_file |grep -v "#"); do
K=${KV%:*}; V=${KV#*:}
echo Key:$K, Value:$V;
SED_OPTIONS="$SED_OPTIONS -e /x-version-update:$K:current/{s|<version>.*<\/version>|<version>$V<\/version>|;}"
echo "Key:$K, Value:$V";
# Pattern 1: XML <version> tags and property tags (e.g., <google-cloud-shared-dependencies.version>)
echo "/x-version-update:$K:\(current\|released\)/{s|>\([^<]*\)<|>$V<|;}" >> "$SED_SCRIPT"
# Pattern 2: YAML _VERSION: '...' or "..."
echo "/x-version-update:$K:\(current\|released\)/{s|: [\'\"][^\'\"]*[\'\"]|: \'$V\'|;}" >> "$SED_SCRIPT"
# Pattern 3: Java static final String VERSION = "..."
echo "/x-version-update-start:$K:\(current\|released\)/{n;s|\".*\"|\"$V\"|;}" >> "$SED_SCRIPT"
Comment on lines +40 to +44
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The sed replacement patterns for XML, YAML, and Java are quite generic and rely on the marker being the only relevant content on the line (or the next line for Java). For example, the XML pattern s|>\([^<]*\)<|>$V<| will replace the first tag's content on the line, which might not be the version tag if multiple tags exist (e.g., <groupId>foo</groupId><version>1.0</version>). \n\nConsider making these patterns more specific by anchoring them to the marker or the expected tag name (e.g., <version>) to avoid accidental replacements in complex files.

done


echo "Running sed command. It may take few minutes."
find . -maxdepth 3 -name pom.xml |sort --dictionary-order |xargs sed -i.bak $SED_OPTIONS
find . -maxdepth 3 -name pom.xml.bak |xargs rm
# Including pom.xml, *.yaml, and Version.java files.
FILES=$(find . \( -name pom.xml -o -name "*.yaml" -o -name "Version.java" \) | sort --dictionary-order)
TOTAL_FILES=$(echo "$FILES" | grep -c .)
CURRENT_FILE=0
while read -r FILE; do
CURRENT_FILE=$((CURRENT_FILE + 1))
echo -ne "Progress: $CURRENT_FILE/$TOTAL_FILES ($((CURRENT_FILE * 100 / TOTAL_FILES))%)\r"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If no files are found matching the search criteria, TOTAL_FILES will be 0, resulting in a division by zero error in the progress calculation. It is safer to wrap the progress output in a conditional check.

Suggested change
echo -ne "Progress: $CURRENT_FILE/$TOTAL_FILES ($((CURRENT_FILE * 100 / TOTAL_FILES))%)\r"
if [ "$TOTAL_FILES" -gt 0 ]; then
echo -ne "Progress: $CURRENT_FILE/$TOTAL_FILES ($((CURRENT_FILE * 100 / TOTAL_FILES))%)\r"
fi

sed -i -f "$SED_SCRIPT" "$FILE"
done <<< "$FILES"
Comment on lines +53 to +57
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Spawning a new sed process for every file in a loop is significantly slower than processing all files in a single sed invocation (e.g., using xargs sed -i -f "$SED_SCRIPT"). While the progress bar provides feedback, it introduces a performance bottleneck that may become substantial in large repositories. Consider batching the files or using xargs if execution time becomes a concern.

echo
Loading