Skip to content

[3.4] Backport CI config from master #2475

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

Merged
merged 11 commits into from
Jul 22, 2017
47 changes: 47 additions & 0 deletions .github/CONTRIBUTING.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
Contributing to Python
======================

Build Status
------------

- master

+ `Stable buildbots <http://buildbot.python.org/3.x.stable/>`_

- 3.6

+ `Stable buildbots <http://buildbot.python.org/3.6.stable/>`_

- 3.5

+ `Stable buildbots <http://buildbot.python.org/3.5.stable/>`_

- 2.7

+ `Stable buildbots <http://buildbot.python.org/2.7.stable/>`_


Contribution Guidelines
-----------------------
Please read the `devguide <https://cpython-devguide.readthedocs.io/>`_ for
guidance on how to contribute to this project. The documentation covers
everything from how to build the code to submitting a pull request. There are
also suggestions on how you can most effectively help the project.

Please be aware that our workflow does deviate slightly from the typical GitHub
project. Details on how to properly submit a pull request are covered in
`Lifecycle of a Pull Request <https://cpython-devguide.readthedocs.io/pullrequest.html>`_.
One key point is to keep comments on GitHub to only those related to the reviewing
the code in the pull request. All other discussions -- e.g. about the issue being
fixed -- should happen on bugs.python.org.

If you are making a code contribution or large documentation contribution,
please feel free to add yourself to the ``Misc/ACKS`` file alphabetically.


Code of Conduct
---------------
All interactions for this project are covered by the
`PSF Code of Conduct <https://www.python.org/psf/codeofconduct/>`_. Everyone is
expected to be open, considerate, and respectful of others no matter their
position within the project.
28 changes: 28 additions & 0 deletions .github/appveyor.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
version: 3.4.6+.{build}
clone_depth: 5
branches:
only:
- master
- /\d\.\d/
- buildbot-custom
build_script:
- cmd: PCbuild\build.bat -e
test_script:
- cmd: PCbuild\rt.bat -q -uall -u-cpu -rwW --slow --timeout=1200 -j0

# Only trigger AppVeyor if actual code or its configuration changes
only_commits:
files:
- .github/appveyor.yml
- .gitattributes
- Grammar/
- Include/
- Lib/
- Modules/
- Objects/
- PC/
- PCBuild/
- Parser/
- Programs/
- Python/
- Tools/
68 changes: 68 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
language: c
dist: trusty
sudo: false
group: beta

# To cache doc-building dependencies.
cache: pip

branches:
only:
- master
- /^\d\.\d$/

matrix:
fast_finish: true
allow_failures:
- env: OPTIONAL=true
include:
- os: linux
language: c
compiler: clang
# gcc also works, but to keep the # of concurrent builds down, we use one C
# compiler here and the other to run the coverage build. Clang is preferred
# in this instance for its better error messages.
env: TESTING=cpython
- os: osx
language: c
compiler: clang
# Testing under macOS is optional until testing stability has been demonstrated.
env: OPTIONAL=true
before_install:
- brew install openssl xz
- export CPPFLAGS="-I$(brew --prefix openssl)/include"
- export LDFLAGS="-L$(brew --prefix openssl)/lib"

# Travis provides only 2 cores, so don't overdo the parallelism and waste memory.
before_script:
- |
set -e
if ! git diff --name-only $TRAVIS_COMMIT_RANGE | grep -qvE '(\.rst$)|(^Doc)|(^Misc)'
then
echo "Only docs were updated, stopping build process."
exit
fi
./configure --with-pydebug
make -j4
Copy link
Member

Choose a reason for hiding this comment

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

We should perhaps keep the make clinic check. That might require also backporting .gitattributes (which might not be a bad idea anyway).

Copy link
Member Author

Choose a reason for hiding this comment

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

We should perhaps keep the make clinic check.

I don't think that it's worth it to add too many tests. I expect less than 10 commits in 3.4 next months, and each commit is carefully reviewed, and is very likely simply a backport.

That might require also backporting .gitattributes (which might not be a bad idea anyway).

This is for Windows end of line? I tried to forget this one. Tests pass on Windows on AppVeyor. Do we really need gitattributes?

Anyway, I suggest to create a separated change once this one is merged, if you consider that we should backport gitattributes.


script:
# Using the built Python as patchcheck.py is built around the idea of using
# a checkout-build of CPython to know things like what base branch the changes
# should be compared against.
# Only run on Linux as the check only needs to be run once.
- if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then ./python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST; fi
# `-r -w` implicitly provided through `make buildbottest`.
- make buildbottest TESTOPTS="-j4 -uall,-cpu"

notifications:
email: false
irc:
channels:
# This is set to a secure variable to prevent forks from notifying the
# IRC channel whenever they fail a build. This can be removed when travis
# implements https://github.com/travis-ci/travis-ci/issues/1094.
# The actual value here is: irc.freenode.net#python-dev
- secure: "s7kAkpcom2yUJ8XqyjFI0obJmhAGrn1xmoivdaPdgBIA++X47TBp1x4pgDsbEsoalef7bEwa4l07KdT4qa+DOd/c4QxaWom7fbN3BuLVsZuVfODnl79+gYq/TAbGfyH+yDs18DXrUfPgwD7C5aW32ugsqAOd4iWzfGJQ5OrOZzqzGjYdYQUEkJFXgxDEIb4aHvxNDWGO3Po9uKISrhb5saQ0l776yLo1Ur7M4oxl8RTbCdgX0vf5TzPg52BgvZpOgt3DHOUYPeiJLKNjAE6ibg0U95sEvMfHX77nz4aFY4/3UI6FFaRla34rZ+mYKrn0TdxOhera1QOgPmM6HzdO4K44FpfK1DS0Xxk9U9/uApq+cG0bU3W+cVUHDBe5+90lpRBAXHeHCgT7TI8gec614aiT8lEr3+yH8OBRYGzkjNK8E2LJZ/SxnVxDe7aLF6AWcoWLfS6/ziAIBFQ5Nc4U72CT8fGVSkl8ywPiRlvixKdvTODMSZo0jMqlfZSNaAPTsNRx4wu5Uis4qekwe32Fz4aB6KGpsuuVjBi+H6v0RKxNJNGY3JKDiEH2TK0UE2auJ5GvLW48aUVFcQMB7euCWYXlSWVRHh3WLU8QXF29Dw4JduRZqUpOdRgMHU79UHRq+mkE0jAS/nBcS6CvsmxCpTSrfVYuMOu32yt18QQoTyU="
on_success: change
on_failure: always
skip_join: true
10 changes: 0 additions & 10 deletions Lib/test/test_imaplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,16 +468,6 @@ def test_logincapa(self):
_server = self.imap_class(self.host, self.port)
self.check_logincapa(_server)

def test_logincapa_with_client_certfile(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why were these tests removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

See the commit 2bef165e1a142b83bc3f14e1017cd2aad50c9af8 of this PR which contains the full context, it's just a backport; http://bugs.python.org/issue30231

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Why is this bugfix part of the "Backport CI config from master" PR? As far as I can tell, it has nothing to do with backporting the config for AppVeyor, CodeConv, and Travis.

Unless this bugfix is necessary for running AppVeyor, CodeConv, or Travis, I would like this change removed from the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@larryhastings: Please see my summary of this PR, #2475 (comment)

Technically, I don't think that you can merge a PR if the CI fails. So it's not posisble to add the CI config in one PR and then fix failing tests. I could do the opposite, but it's not possible to make a PR depends on another one. Moreover, I used this issue to see which tests on the CI and test my fixes.

I think this reply below was actually meant to be in-line here. I am replying to it here.

2bef165 says "Remove the two tests which are already skipped." If they're skipped, then they're not failing, and they don't need to be removed.

Are they actually failing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see. I used "git cherry-pick -x" which keeps the commit message unchanged. In Python 3.6 and master, first I skipped the test using skipIf(True, "...") to take decide what to do with these tests. But later, we decided that it's not possible to fix the test, so I removed them in all branches.

The IMAP server now always reject our TLS certificate, so the test always fail. The question is more why the server accepted your TLS certificate in the past? :-)

with transient_internet(self.host):
_server = self.imap_class(self.host, self.port, certfile=CERTFILE)
self.check_logincapa(_server)

def test_logincapa_with_client_ssl_context(self):
with transient_internet(self.host):
_server = self.imap_class(self.host, self.port, ssl_context=self.create_ssl_context())
self.check_logincapa(_server)

def test_logout(self):
with transient_internet(self.host):
_server = self.imap_class(self.host, self.port)
Expand Down
6 changes: 5 additions & 1 deletion Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,11 @@ def setUp(self):
if support.can_symlink():
os.symlink(os.path.abspath(t2_path), self.link_path)
os.symlink('broken', broken_link_path, True)
self.sub2_tree = (sub2_path, ["link"], ["broken_link", "tmp3"])
if os.path.isdir(broken_link_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this code modified?

Copy link
Member

Choose a reason for hiding this comment

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

See bpo-23808.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Why is this bugfix part of the "Backport CI config from master" PR? As far as I can tell, it has nothing to do with backporting the config for AppVeyor, CodeConv, and Travis.

Unless this bugfix is necessary for running AppVeyor, CodeConv, or Travis, I would like this change removed from the PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

The CI fails without this fix. If the CI fails, you cannot merge this PR.

Moreover, a prefer a green CI to more easily spot failures, than to have to compare with the previous CI build to check if there are new failures ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I'll accept this into 3.4, Even Though It's In Security Fixes Only Mode etc etc etc.

# On Windows a symlink can has the FILE_ATTRIBUTE_DIRECTORY flag.
self.sub2_tree = (sub2_path, ["broken_link", "link"], ["tmp3"])
else:
self.sub2_tree = (sub2_path, ["link"], ["broken_link", "tmp3"])
else:
self.sub2_tree = (sub2_path, [], ["tmp3"])

Expand Down
8 changes: 4 additions & 4 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,24 +609,24 @@ def test_empty_env(self):
def test_invalid_cmd(self):
# null character in the command name
cmd = sys.executable + '\0'
with self.assertRaises(ValueError):
with self.assertRaises((ValueError, TypeError)):
subprocess.Popen([cmd, "-c", "pass"])

# null character in the command argument
with self.assertRaises(ValueError):
with self.assertRaises((ValueError, TypeError)):
subprocess.Popen([sys.executable, "-c", "pass#\0"])

def test_invalid_env(self):
# null character in the enviroment variable name
newenv = os.environ.copy()
newenv["FRUIT\0VEGETABLE"] = "cabbage"
with self.assertRaises(ValueError):
with self.assertRaises((ValueError, TypeError)):
subprocess.Popen([sys.executable, "-c", "pass"], env=newenv)

# null character in the enviroment variable value
newenv = os.environ.copy()
newenv["FRUIT"] = "orange\0VEGETABLE=cabbage"
with self.assertRaises(ValueError):
with self.assertRaises((ValueError, TypeError)):
subprocess.Popen([sys.executable, "-c", "pass"], env=newenv)

# equal character in the enviroment variable name
Expand Down