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.

Sign up for GitHub

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

Jump to bottom

Auto-generate pants.ini with pinned pants_version if file is missing #45

Closed
wants to merge 12 commits into from
Closed

Auto-generate pants.ini with pinned pants_version if file is missing #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b163f43
Auto-generate pants.ini if missing
Eric-Arellano Mar 21, 2019
46ad8f5
Fix sed failing when file is missing
Eric-Arellano Mar 21, 2019
5c2f975
Add testing
Eric-Arellano Mar 21, 2019
7a83883
OSX Sierra should be testing 3.7
Eric-Arellano Mar 21, 2019
f8bae90
Improve wording and grammar.
Eric-Arellano Mar 21, 2019
4268a3f
Improve logging messages
Eric-Arellano Mar 21, 2019
a5e6253
improve message #2
Eric-Arellano Mar 21, 2019
a109dbf
Update documentation
Eric-Arellano Mar 21, 2019
91b841e
Implement review suggestions
Eric-Arellano Mar 21, 2019
9b10896
Improve bash with Danny's suggestions
Eric-Arellano Mar 21, 2019
a70fb03
Convert to unit test file
Eric-Arellano Mar 21, 2019
dc50bb2
Rename original, rather than deleting
Eric-Arellano Mar 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ script:
- ./build-support/ci.py
--pants-versions config
--python-versions unspecified 2.7 3.6 3.7
- ./build-support/test_pants_ini_autogen.py

osx_setup: &osx_setup
os: osx
Expand Down Expand Up @@ -90,8 +91,9 @@ matrix:
--skip-pantsd-tests
- ./build-support/ci.py
--pants-versions config
--python-versions unspecified 2.7 3.6
--python-versions unspecified 2.7 3.6 3.7
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 was accidentally removed in #42.

--skip-pantsd-tests
- ./build-support/test_pants_ini_autogen.py


- name: "OSX 10.13 - High Sierra"
Expand All @@ -114,6 +116,7 @@ matrix:
- ./build-support/ci.py
--pants-versions config
--python-versions unspecified 2.7 3.6
- ./build-support/test_pants_ini_autogen.py

- name: "Ubuntu 14.04 - Trusty"
<<: *linux_setup
Expand All @@ -127,6 +130,7 @@ matrix:
- ./build-support/ci.py
--pants-versions config
--python-versions unspecified 2.7 3.6
- ./build-support/test_pants_ini_autogen.py


- name: "Ubuntu 16.04 - Xenial"
Expand Down
10 changes: 10 additions & 0 deletions build-support/common.py
View file
Open in desktop
Expand Up @@ -96,3 +96,13 @@ def temporarily_rewrite_config(updated_config: configparser.ConfigParser):
yield
finally:
write_config(original_config)


@contextmanager
def temporarily_remove_config():
Path(PANTS_INI).rename("pants.ini.orig")
try:
yield
finally:
Path(PANTS_INI).unlink()
Path("pants.ini.orig").rename("pants.ini")
51 changes: 51 additions & 0 deletions build-support/test_pants_ini_autogen.py
View file
Open in desktop
@@ -0,0 +1,51 @@
#!/usr/bin/env python3
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

"""Test that auto-generation of pants.ini works as expected.

Note this is a separate file from ci.py because we only want to test the auto-generation
part of ./pants here, and leave it to ci.py to test the other behavior like correctly
parsing values from pants.ini and that the virtual env actually works. """

import configparser
import subprocess
import unittest
from contextlib import contextmanager
from pathlib import Path

from ci import PantsVersion, setup_pants_version
from common import (CONFIG_GLOBAL_SECTION, PANTS_INI, banner, read_config,
temporarily_remove_config, travis_section)


class TestPantsIniAutogen(unittest.TestCase):

@contextmanager
def autogen_pants_ini(self):
banner("Temporarily removing pants.ini.")
with temporarily_remove_config():
subprocess.run(["./pants"], check=True)
yield
banner("Restoring original pants.ini.")

def test_file_created(self) -> None:
with self.autogen_pants_ini():
self.assertTrue(Path(PANTS_INI).is_file())


def test_pants_versions_pinned_properly(self) -> None:
with self.autogen_pants_ini():
config = read_config()
self.assertIn("pants_version", config[CONFIG_GLOBAL_SECTION])
pinned_pants_version = config[CONFIG_GLOBAL_SECTION]["pants_version"]
with setup_pants_version(PantsVersion.unspecified):
unconfigured_pants_version = subprocess.run(
["./pants", "-V"], stdout=subprocess.PIPE, encoding="utf-8", check=True
).stdout.strip()
self.assertEqual(pinned_pants_version, unconfigured_pants_version)


if __name__ == "__main__":
with travis_section("PantsIniAutoGen", "Testing auto-generation of pants.ini."):
unittest.main()
25 changes: 10 additions & 15 deletions index.html
View file
Open in desktop
Expand Up @@ -4,31 +4,26 @@
</head>
<body>
<p>
To setup pants in your repo:
To setup Pants in your repo:
<pre>
curl -L -O https://pantsbuild.github.io/setup/pants
chmod +x pants
touch pants.ini
</pre>
The only requirement is that you have a python 2.7 interpreter somewhere on
your PATH.
</p>
<p>
The first time you run the new <code>./pants</code> script it will install
the latest pants version and then run it.
On subsequent runs it will find the already installed pants and run it.
The first time you run the new <code>./pants</code> script, it will install
the latest Pants version and generate a config file <code>pants.ini</code>
for you. On subsequent runs, the script will find the already installed
Pants and run it.
</p>
<p>
To pin the pants version, add an entry like so to pants.ini:
If you would like to use Pants plugins published to PyPi, you can configure them as
follows in <code>pants.ini</code>:
<pre>
[GLOBAL]
pants_version: 1.5.0
</pre>
If you use pants plugins published to pypi you can configure them as
follows, also in pants.ini:
<pre>
[GLOBAL]
pants_version: 1.5.0
pants_version: 1.14.0

plugins: [
'pantsbuild.pants.contrib.go==%(pants_version)s',
Expand All @@ -40,9 +35,9 @@
<p>
NB: The formatting of the plugins list is important; all lines below the
`plugins:` line must be indented by at least one white space to form logical
continuation lines. This is standard for python ini files, see
continuation lines. This is standard for Python ini files, see
<a href="http://tools.ietf.org/html/rfc822.html#section-3.1">RFC #822</a>
section 3.1.1 for the full rules python uses to parse ini file entries.
section 3.1.1 for the full rules Python uses to parse ini file entries.
</p>
</body>
</html>
20 changes: 20 additions & 0 deletions pants
View file
Open in desktop
Expand Up @@ -52,6 +52,9 @@ function get_exe_path_or_die {
}

function get_pants_ini_config_value {
if [[ ! -f 'pants.ini' ]]; then
return 0
fi
config_key="$1"
valid_delimiters="[:=]"
optional_space="[[:space:]]*"
Expand All @@ -74,6 +77,7 @@ EOF
# 2.) Grab pants version from pants.ini or default to latest.
# 3.) Check for a venv via a naming/path convention and execute if found.
# 4.) Otherwise create venv and re-exec self.
# 5.) Check if pants.ini already exists. If not, create with sensible defaults then exit.
#
# After that pants itself will handle making sure any requested plugins
# are installed and up to date.
Expand Down Expand Up @@ -137,10 +141,26 @@ function bootstrap_pants {
echo "${PANTS_BOOTSTRAP}/${target_folder_name}"
}

function autogen_pants_ini {
log "Config file \`pants.ini\` not detected in the repository root. Creating with sensible defaults:\n"
touch "pants.ini"
Copy link

Choose a reason for hiding this comment

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

I think you can remove this touch, just add more log "* ..." lines for any other sensible defaults we may add in the future, and still write it all at once in a heredoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to keep the touch because of the recursive call to ./pants -V. Otherwise the script will attempt to continually generate the file.

Copy link

Choose a reason for hiding this comment

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

Ah, great!

pants_version="$(./pants -V)"
log "* Pinning \`pants_version\` to the most recent stable release: ${pants_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.

After 1.15.0 is released, we'll also automatically pin pants_runtime_python_version!

* Pinning `pants_runtime_python_version` to Python 2.7. Ensure you have this interpreter available everywhere you plan to use Pants!

Will probably try to default to Python 3.6, then 3.7, then 2.7, so that dropping Py27 after 1.17.0 is a bit less surprising. TBD..

Copy link

Choose a reason for hiding this comment

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

Would it be even less surprising if we were to also mention in the logging to the user here that in 1.15.0 we will automatically pin pants_runtime_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.

I think that might confuse people here, because there is nothing actionable for them to be able to do.

We're only about 1-3 weeks away from 1.15.0, depending on how many RCs we have, so this will be fixed soon!

cat <<EOF >> "pants.ini"
[GLOBAL]
pants_version: ${pants_version}
EOF
green "\n\`pants.ini\` created. You may now run \`./pants\`."
Copy link

Choose a reason for hiding this comment

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

Within single quotes, nothing is substituted (which is why I try to use them where possible, to make it more clear when things are getting substituted into a string), so you can do things a little more fearlessly:

Suggested change
green "\n\`pants.ini\` created. You may now run \`./pants\`."
green '\n`pants.ini` created. You may now run `./pants`.'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this suggestion, but then shellcheck.py complains that it thinks we're intending to do command substitution and it explains we would then need double quotes. So I kept as is.

}

# Ensure we operate from the context of the ./pants buildroot.
cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
python="$(determine_pants_runtime_python_version)"
pants_dir="$(bootstrap_pants "${python}")"
if [[ ! -f 'pants.ini' ]]; then
autogen_pants_ini
exit 0
fi


# We set the env var no_proxy to '*', to work around an issue with urllib using non
Expand Down