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

[internal] Port dry release, publishing, and testing release from Bash to Python #11952

Merged
merged 6 commits into from
Apr 21, 2021

Conversation

Eric-Arellano
Copy link
Contributor

Makes more progress on #5551. Now all that is left in Bash is building PEXes.

This makes a couple of improvements from the original Bash implementation:

  • When building 3rdparty wheels, we run over the precise transitive deps of our two python_distributions, rather than the whole requiremenents.txt.
  • We now log which Python version is being used for things like building wheels. This is useful because we build wheels for Py37 x 38 x 39.
  • Reduces verbosity by capturing some output and using --quiet with pip where relevant.

This removes the -x debug option for release.sh, which is no longer very useful given how little Bash is left.

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
Comment on lines +352 to +355
banner(f"Validating Pants wheels for {CONSTANTS.python_version}.")
create_twine_venv()
subprocess.run([CONSTANTS.twine_venv_dir / "bin/twine", "check", dest / "*.whl"], check=True)
green(f"Validated Pants wheels for {CONSTANTS.python_version}.")
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 used to run for release.sh -t, which was incorrect to do because it always inspects the dist/ folder, whereas -t is only meant to test the live stable release on PyPI and we can make no assumptions that there are local wheels in dist/.

Now, it only runs when building wheels, which happens when building a PEX and when doing a dry release.

Comment on lines 363 to 385
deps = (
subprocess.run(
[
"./pants",
"--concurrent",
"--no-dynamic-ui",
"dependencies",
"--transitive",
"--type=3rdparty",
*pkg_tgts,
],
stdout=subprocess.PIPE,
check=True,
)
.stdout.decode()
.strip()
.splitlines()
)
if not deps:
raise AssertionError(
f"No 3rd-party dependencies detected for {pkg_tgts}. Is `./pants dependencies` "
"broken?"
)
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 the main change from Bash, which used to read our requirements file instead.

@@ -348,11 +531,86 @@ def check(i: int, pkg: Package) -> None:
)


def check_release_prereqs() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlined

check_pgp()
me = get_pypi_config("server-login", "username")
check_ownership({me})
def reversion_prebuilt_wheels() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from lower in the file, no changes made.

)


def tag_release() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from lower in the file, no changes made.

@@ -6,18 +6,6 @@ set -e

ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd "$(git rev-parse --show-toplevel)" && pwd)

function safe_curl() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved lower in file.

Comment on lines -198 to -212
if [[ "${pause_after_venv_creation}" == "true" ]]; then
cat << EOM

If you want to poke around with the new version of pants that has been built
and installed in a temporary virtualenv, fire up another shell window and type:

source ${VENV_DIR}/bin/activate
cd ${ROOT}

From there, you can run 'pants' (not './pants') to do some testing.

When you're done testing, press enter to continue.
EOM
read -r
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI we're losing this functionality. Fwict it wasn't really used.

Comment on lines -246 to -247
eval pkg_${package##*\.}_install_test "${PIP_ARGS[@]}" ||
die "Failed to install and test package ${package}-${VERSION}!"
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 line is when I realized how much I wanted to port from Bash to Python :) Glad to see it go

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
*args,
],
check=True,
stdout=subprocess.PIPE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this call also need capture_output=True? https://docs.python.org/3/library/subprocess.html#subprocess.run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that kwarg is a shorthand. I don't want to capture stderr because we want errors to be printed, and this current factoring avoids doing that.

If capture_output is true, stdout and stderr will be captured. When used, the internal Popen object is automatically created with stdout=PIPE and stderr=PIPE. The stdout and stderr arguments may not be supplied at the same time as capture_output. If you wish to capture and combine both streams into one, use stdout=PIPE and stderr=STDOUT instead of capture_output.

build-support/bin/packages.py Show resolved Hide resolved
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
@Eric-Arellano
Copy link
Contributor Author

Thanks for the review @tdyas! Ik it was a big PR, I appreciate it.

@Eric-Arellano Eric-Arellano merged commit e4333f4 into pantsbuild:main Apr 21, 2021
@Eric-Arellano Eric-Arellano deleted the release-3rdparty branch April 21, 2021 18:03
Eric-Arellano added a commit that referenced this pull request Apr 21, 2021
More progress on #11952. There is nothing left in Bash except for the processing of options, which then pipes to Python; and setting the Python interpreter to run the Python script. `release.sh` will be removed in a followup.

This adds lightweight validation to our PEX build process, which would have caught #11954.

Certainly, we should be building the PEX via `./pants package` instead of directly via PEX. But this is an incremental improvement from before.

[ci skip-rust]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants