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

Appveyor script #30

Closed
wants to merge 12 commits into from
Closed

Appveyor script #30

wants to merge 12 commits into from

Conversation

fgregg
Copy link
Contributor

@fgregg fgregg commented Apr 5, 2016

This appveyor script will

  • built and test BTrees on windows for multiple versions of python and 32 and 64bit architectures
  • on a tag, deploy window wheels to pypi

You will need to

  1. setup appveyor for the BTrees repo: https://www.appveyor.com/docs
  2. create an encrypted variable for "password=the_zope.wheelbuilder_password" and let me know what it is, I'll incorporate it into the script. https://www.appveyor.com/docs/build-configuration#secure-variables

@fgregg fgregg mentioned this pull request Apr 5, 2016
- echo password=%password% >> %USERPROFILE%\\.pypirc
- set HOME=%USERPROFILE%
- pip install wheel twine
- ps: if($env:APPVEYOR_REPO_TAG -eq $TRUE) { python -W ignore setup.py && twine upload dist/* }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't that python setup.py be python setup.py bdist_wheel bdist_egg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like some body is already building window eggs for BTree? https://pypi.python.org/pypi/BTrees

Copy link
Member

Choose a reason for hiding this comment

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

It looks like some body is already building window wheels for BTree?

That would be the zope.wineggbuilder deployment, which is on the same hard-to-maintain machine as the zope.winbot deployment.

Copy link
Member

Choose a reason for hiding this comment

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

Note how existing wineggbuilder uploads lack support for Python 3.3, 3.4 and 3.5 (and also for wheels).

We'll probably want to disable wineggbuilder for BTrees as soon as Appveyor starts working. That can be done by specifying maxVersion in https://github.com/zopefoundation/zope.wineggbuilder/blob/master/rackspace.ini#L274

@mgedmin
Copy link
Member

mgedmin commented Apr 6, 2016

BTW the only other ZopeFoundation project that uses appveyor that I know of is zope.interface:
https://github.com/zopefoundation/zope.interface/blob/master/appveyor.yml

(It doesn't build wheels or upload them to PyPI -- at the moment.)

The approach to Python versions seems quite different than here. Any reason for that? Can we simplify the zope.interface appveyor.yml?

@@ -35,6 +36,6 @@ deploy_script:
- echo password=%password% >> %USERPROFILE%\\.pypirc
- set HOME=%USERPROFILE%
- pip install wheel twine
- ps: if($env:APPVEYOR_REPO_TAG -eq $TRUE) { python -W ignore setup.py && twine upload dist/* }
- ps: if($env:APPVEYOR_REPO_TAG -eq $TRUE) { python -W ignore setup.py bdist_wheel bdist_egg && twine upload dist/* }
Copy link
Member

Choose a reason for hiding this comment

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

I'd completely forgotten that the old wineggbuilder used to upload both .exe and .egg files. Question for the audience: do we still need .exe installers?

My gut feeling is that people who still use zc.buildout will need .egg, and people who have migrated to pip will need .whl, but probably nobody needs .exe. Unless there are still people who install stuff manually by downloading installers from PyPI and running them?

@fgregg
Copy link
Contributor Author

fgregg commented Apr 10, 2016

Do you need anything else from me right now?

@mgedmin
Copy link
Member

mgedmin commented Apr 13, 2016

Here's the encrypted zope.wheelbuilder password for Appveyor: RtpeKCle25vCixaUcJBu6Q==

Here's the appveyor project page, under my personal appveyor account: https://ci.appveyor.com/project/mgedmin/btrees

How does ZopeFoundation intend to manage appveyor accounts?

@mgedmin
Copy link
Member

mgedmin commented Apr 13, 2016

I don't know how to ask AppVeyor to try-build this PR.

I expect it will try a build when you push a commit to update the encrypted secret.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 13, 2016

@mgedmin
Copy link
Member

mgedmin commented Apr 14, 2016

Why do you say that? The current appveyor failures are expected: none of the branches being built had an appveyor.yml.

Now we're waiting for you to push any commit (e.g. updating the encrypted secret) to this branch so Appveyor gets a chance.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

Woops! Sorry it's building. Appveyor disables deploys in Pull Requests, so that's why that section is no showing up.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

Okay, I think this good to merge. The failing build is due to real failures of the tests. #32

@mgedmin
Copy link
Member

mgedmin commented Apr 14, 2016

Let's get the broken test fixed (or at least skip it on 64-bit windows with @unittest.skipIf or something) before merging this.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

testing the sys.maxsize here, since this is the only PR with appveyor tests

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

Heh, @mgedmin just turned off appveyor for branches w/o .appveyor.yml.

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

@fgregg should I close #34?

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

@tseaver sure, we can get it working in this branch.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

Okay the sys.maxsize idea won't work because

on windows python 3, x64

print(sys.maxsize) 
9223372036854775807

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

Hmm, that is weird. I know the C code actually uses PY_LONG_LONG for keys and values, so I'm not even sure why the error occurs.

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

For reference:

  • BTrees.tests.common.LARGEST_64_BITS is 9223372036854775807
  • BTrees.tests.common.SMALLEST_64_BITS is -9223372036854775808

and the code raising the OverflowError is not in BTrees C code: when it detects a too-large / too-small value, it raises ValueError.

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

hi @tseaver I merged in #35 and it's not a fix, yet.

This reverts commit b827505.
This reverts commit bc76de3.
@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

@mgedmin @tseaver maybe this PR should be merged into a branch, so that you can get the benefit of testing on appveyor without me being a bottleneck?

@tseaver
Copy link
Member

tseaver commented Apr 15, 2016

@fgregg I merged / rebased the non-reverted commits from your branch onto the fix-appveyor-build branch in this repo: see #36.

@tseaver
Copy link
Member

tseaver commented Apr 15, 2016

@fgregg Closing in favor of my changes on top of yours in #36.

@tseaver tseaver closed this Apr 15, 2016
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

3 participants