Skip to content
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

chore: add version updating script in preparation for auto-release CI #16

Merged
merged 2 commits into from
Jul 20, 2022
Merged
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
8 changes: 8 additions & 0 deletions .github/workflows/misc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: editorconfig-checker/action-editorconfig-checker@v1

check-version-patch:
name: Check version update patch file
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Check version update patch file
run: python3 ci/version.py check
5 changes: 5 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ important:
by corrosion, but good to synchronize with the version of the main crate.
- `tests/Cargo.toml`: can be ignored.

You (or CI) can use `ci/version.py` to update the version automatically, but
this automation is based on a patchfile that may go out of date. You may have
to regenerate it (using the same tool) if you change a file that includes the
version number.

Relation of `substrait-validator` crate version to the Substrait specification
version is TBD.

Expand Down
5 changes: 5 additions & 0 deletions ci/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CI-related scripts
==================

This directory contains some assorted scripts related to CI and the release
process.
1 change: 1 addition & 0 deletions ci/version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0.0.1
132 changes: 132 additions & 0 deletions ci/version-diff-template
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
diff --git a/c/Cargo.toml b/c/Cargo.toml
index 0e9b476..7165025 100644
--- a/c/Cargo.toml
+++ b/c/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "substrait-validator-c"
-version = "{frm}"
+version = "{to}"
edition = "2021"
license = "Apache-2.0"

@@ -12,6 +12,6 @@ doc = false
cbindgen = "0.20.0"

[dependencies]
-substrait-validator = {{ path = "../rs", version = "{frm}" }}
+substrait-validator = {{ path = "../rs", version = "{to}" }}
libc = "0.2"
thiserror = "1.0"
diff --git a/derive/Cargo.toml b/derive/Cargo.toml
index 7a8af00..539e170 100644
--- a/derive/Cargo.toml
+++ b/derive/Cargo.toml
@@ -4,7 +4,7 @@ description = "Procedural macros for substrait-validator"
homepage = "https://substrait.io/"
repository = "https://github.com/substrait-io/substrait"
readme = "README.md"
-version = "{frm}"
+version = "{to}"
edition = "2021"
license = "Apache-2.0"

diff --git a/py/Cargo.toml b/py/Cargo.toml
index c095a2d..32108ad 100644
--- a/py/Cargo.toml
+++ b/py/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "substrait-validator-py"
-version = "{frm}"
+version = "{to}"
edition = "2018"
license = "Apache-2.0"
include = [
@@ -29,7 +29,7 @@ name = "substrait_validator"
doc = false

[dependencies]
-substrait-validator = {{ path = "../rs", version = "{frm}" }}
+substrait-validator = {{ path = "../rs", version = "{to}" }}
pyo3 = {{ version = "0.15.1", features = ["extension-module"] }}

[build-dependencies]
diff --git a/py/pyproject.toml b/py/pyproject.toml
index 8481d2c..60d0453 100644
--- a/py/pyproject.toml
+++ b/py/pyproject.toml
@@ -5,7 +5,7 @@ backend-path = ["."]

[project]
name = "substrait-validator"
-version = "{frm}"
+version = "{to}"
description = "Validator for Substrait query plans"
readme = "README.md"
license = {{file = "LICENSE"}}
diff --git a/rs/Cargo.toml b/rs/Cargo.toml
index 6dbf6a8..d7e0c22 100644
--- a/rs/Cargo.toml
+++ b/rs/Cargo.toml
@@ -4,7 +4,7 @@ description = "Substrait validator"
homepage = "https://substrait.io/"
repository = "https://github.com/substrait-io/substrait"
readme = "README.md"
-version = "{frm}"
+version = "{to}"
edition = "2021"
license = "Apache-2.0"
include = ["src", "build.rs", "README.md"]
@@ -17,7 +17,7 @@ prost-types = "0.10"

# Prost doesn't generate any introspection stuff, so we hack that stuff in with
# our own procedural macros.
-substrait-validator-derive = {{ path = "../derive", version = "{frm}" }}
+substrait-validator-derive = {{ path = "../derive", version = "{to}" }}

# Google/protobuf has a funny idea about case conventions (it converts them all
# over the place) and prost remaps to Rust's conventions to boot. So, to
diff --git a/rs/README.md b/rs/README.md
index 1ea785f..553fd82 100644
--- a/rs/README.md
+++ b/rs/README.md
@@ -6,7 +6,7 @@ plans.

```
[dependencies]
-substrait-validator = "{frm}"
+substrait-validator = "{to}"
```

YAML file resolution
@@ -20,7 +20,7 @@ dependency:

```
[dependencies]
-substrait-validator = {{ version = "{frm}", features = ["curl"] }}
+substrait-validator = {{ version = "{to}", features = ["curl"] }}
```

This adds the `substrait_validator::Config::add_curl_yaml_uri_resolver()`
diff --git a/tests/Cargo.toml b/tests/Cargo.toml
index ae095fd..a341e1c 100644
--- a/tests/Cargo.toml
+++ b/tests/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "test-runner"
-version = "{frm}"
+version = "{to}"
edition = "2018"
license = "Apache-2.0"
default-run = "runner"
@@ -14,7 +14,7 @@ name = "find_protoc"
path = "src/find_protoc.rs"

[dependencies]
-substrait-validator = {{ path = "../rs", version = "{frm}" }}
+substrait-validator = {{ path = "../rs", version = "{to}" }}
serde = {{ version = "1.0", features = ["derive"] }}
serde_json = "1.0"
walkdir = "2"
178 changes: 178 additions & 0 deletions ci/version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
#!/usr/bin/python3
# SPDX-License-Identifier: Apache-2.0

import subprocess
import sys
import os


def set_version(current_version, new_version, dry_run=False):
"""Tries to update the version from current_version to new_version."""
print(f"Modifying version from {current_version} to {new_version}...")
if dry_run:
print(" (dry run, won't actually make the change)")

# Read the template.
with open("version-diff-template", "r") as f:
template = f.read()

# Fill out the template.
patch = template.format(frm=current_version, to=new_version).encode("utf-8")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use something like cargo bump instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't aware of it's existence, but...

thread 'main' panicked at 'Workspaces are not supported yet.'

Also, it still wouldn't bump the version information for the Python bindings, in documentation, etc.

Copy link
Contributor

@cpcloud cpcloud Jun 23, 2022

Choose a reason for hiding this comment

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

Right, but that should all be handled using the tools to do such tasks for each ecosystem, or with a replacement tool such as https://github.com/google/semantic-release-replace-plugin.

For cargo bump it seems like you can handle its lack of workspace support by listing the crates and looping over them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also tried from the subdirectories; it still fails. Also the issue tracking this has been stale for 3 1/2 years. In fact, all of cargo bump has been stale for 3 years. It's not exactly a standard component of cargo.

I'm perfectly fine with replacing the script with that semantic release plugin, in which case this PR can just be closed. Though to be honest it seems like a lot of complexity for what looks to be a regex replacement, which in itself seems more likely to break than a patchfile... Anyway, do you have time to help me hammer out the semantic release logic sometime, or at least to get me started with it? I remember being very confused about the logic on the main repo.


# Try to apply the patch.
output = subprocess.run(["git", "apply", "--check"], cwd="..", input=patch)
if output.returncode:
print("Patch is invalid. Something is out of sync: check the version")
print("file and try regenerating the patch template.")
sys.exit(1)
print("Generated patch seems valid.")

# Stop here if this is a dry run.
if dry_run:
return

# Apply the patch.
subprocess.run(["git", "apply"], cwd="..", input=patch, check=True)

# Update the current version file.
with open("version", "w") as f:
f.write(new_version)

print("Version modified.")


def escape(s):
"""Escapes { and } sequences in a diff template file."""
return s.replace("{", "{{").replace("}", "}}")


if __name__ == "__main__":

# The rest of the script assumes that we're running from the script
# directory, so make sure that's where we are.
os.chdir(os.path.dirname(os.path.realpath(__file__)))

# Read the current version.
with open("version", "r") as f:
current_version = f.read().strip()

if len(sys.argv) == 2 and sys.argv[1] == "get":
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use argparse for all of this argument handling. It handles subcommands and will make the handling much easier to read.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair enough. Was just being lazy to be honest.

print(current_version)
sys.exit(0)

if len(sys.argv) == 3 and sys.argv[1] == "set":
new_version = sys.argv[2]
if new_version == current_version:
print("Error: that's already the current version.")
sys.exit(1)
set_version(current_version, new_version)
sys.exit(0)

if len(sys.argv) == 2 and sys.argv[1] == "check":
test_version = "3.1.4"
if current_version == test_version:
test_version = "2.7.1"
set_version(current_version, test_version, dry_run=True)
sys.exit(0)

if len(sys.argv) == 2 and sys.argv[1] == "update":
print("Change all the version numbers in the repository, EXCEPT the")
print("current_version file in this directory, such that git diff")
print("will give a valid diff (make sure the repo is clean before")
print("doing this). The new version doesn't matter; it just serves")
print("as a marker and will be reverted. When done, enter the version")
print("you changed the current version into here.")
print()
dummy_version = input("New/dummy version: ").strip()
print()
if dummy_version == current_version:
print("Current and new/dummy version must differ")
sys.exit(1)

# Ask git for the diff, expected to correspond to a version change from
# current_version to dummy_version.
output = subprocess.run(["git", "diff"], capture_output=True)
if output.returncode:
error = output.stderr.decode("utf-8")
print(f"Failed to get diff:\n{error}")
sys.exit(1)
diff = output.stdout.decode("utf-8")

# Parse the diff and convert it to a format-style template.
lines = diff.strip().split("\n")
template_lines = []
num_changes = 0
skip = False
for index, line in enumerate(lines):
if skip:
skip = False
continue
if line.startswith("-") and not line.startswith("---"):
if index == len(lines) - 1:
print("Error: - line at end of `git diff`")
sys.exit(1)
frm = line[1:]
if not lines[index + 1].startswith("+"):
print(
f"Error: expecting -+ line pairs on lines {index+1} "
f"and {index+2} of `git diff`"
)
sys.exit(1)
to = lines[index + 1][1:]
if (
frm.replace(current_version, dummy_version) != to
or to.replace(dummy_version, current_version) != frm
):
print(
f"Error: diff at lines {index+1} and {index+2} of git "
f"diff is not just a change from {current_version} to "
f"{dummy_version}"
)
sys.exit(1)
template_lines.append(
"-" + escape(frm).replace(current_version, "{frm}")
)
template_lines.append("+" + escape(to).replace(dummy_version, "{to}"))
skip = True
num_changes += 1
else:
template_lines.append(escape(line))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is all of this code doing something other than bumping the crates in the workspace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See https://github.com/substrait-io/substrait-validator/blob/main/RELEASE.md#incrementing-version-numbers. tl;dr, various duplicates of the version number in the various cargo.toml files, a reference in Rust's readme/docs for the customary copypasta to include a library as a dependency, and importantly pyproject.toml. It also helps update and sanity-check the patch file, which would be a massive pain otherwise.

Note that this is not exactly my idea of a clean solution, but (Python script implementation aside) I'm not sure how to do it better without making the whole thing massively more complex by special-casing everything or generating all these files.

template = "\n".join(template_lines) + "\n"
print(f"Found {num_changes} version instances.")

# If no lines changed, the user probably didn't understand how this
# works.
if not num_changes:
print("Error: `git diff` returned empty or invalid diff.")
print(
"Did you manually change the version from "
f"{current_version} to {dummy_version}?"
)
sys.exit(1)

# Write the template.
with open("version-diff-template", "w") as f:
f.write(template)
print("Wrote updated template.")

# Try to apply the reverse diff.
set_version(dummy_version, current_version)
sys.exit(0)

# Arguments invalid, print usage.
me = sys.argv[0] if sys.argv else "version.py"
print("Usage:")
print()
print(f"{me} get")
print(" Returns the current version.")
print()
print(f"{me} set <version>")
print(" Updates the version to the given value.")
print()
print(f"{me} check")
print(" Checks patch template consistency.")
print()
print(f"{me} update")
print(" Interactively updates the patchfile template used to update")
print(" the version")
Copy link
Contributor

Choose a reason for hiding this comment

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

All of this can be specified using argparse.

sys.exit(2)