-
Notifications
You must be signed in to change notification settings - Fork 186
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
Artifacts #383
Artifacts #383
Conversation
Then we should add the links into the readme |
Any idea what those seg faults are about? Mixed arch check failing? |
You may want to comment out the portion of test matrix that is passing while you are debugging. Remote debugging of CI is a pain. Thus cutting the time down and reenabling later may help. |
Really have no idea, I have no experience with JNI and almost no in python native interface. A wild guess is that there may be something with stdlib memory allocators. It is written that the error is not reproduced locally, I wonder if it would be reproducable if we took the prebuilt artifact from the failed job, not the locally built one.
I have thought about it, but there is no very big benefit from it except the build being fixed much faster. |
My recommendation on the segfault builds is to cut the build to just fails, branch and submit it as a separate pull request. A big fat target of segfaults sitting in our pull system is likely to get my attention as I can't stand mysteries. |
@Thrameos It's done. You can squash the commits into one and merge. |
Very good job @KOLANICH! Thanks a lot! |
appveyor/install.sh
Outdated
@@ -36,6 +36,9 @@ echo "==== get modules" | |||
$EASYINSTALL pip | |||
$EASYINSTALL mock | |||
#$PIP install mock | |||
$PYTHON -m pip install --upgrade setuptools | |||
$PYTHON -m pip install --upgrade setuptools_scm | |||
$PYTHON -m pip install --upgrade git+https://github.com/pypa/pip.git git+https://github.com/pypa/wheel.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would discourage dragging in bleeding edge versions as these could fail the build. Is there any reason to use them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They have very infrequent releases, so if they have broken your stuff, and if it has landed into release, your package will be broken untill the next one.
So you need to know if they have made breaking changes as earlier as possible in order to fix them before they have landed.
There is a yet another reason to install everything reasonable from vcs, not from pip, but it is inapplicable to these packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point here. This could become a philosophical discussion, but it is pretty much out of our scope to ensure that something that essential like pip is being tested for future versions. If you use that argument, we would also need to test against numpy devel, unreleased Java versions etc. I mean that is the sense of stable APIs we can rely upon, right?
Then a last thing: cloning the whole git history for every CI run we make is overkill. Please get a source archive if you have to use the source for any reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use that argument, we would also need to test against numpy devel, unreleased Java versions etc.
If course we would, but building them as part of our build is not reasonable. But if they shipped prebuilt packages, why not.
cloning the whole git history for every CI run we make is overkill
If I clone manually with --depth=1
(they refuse to make pip do shallow clones by default pypa/pip#2432 ) it shouldn't be so.
I mean that is the sense of stable APIs we can rely upon, right?
Not quite. If anything has been broken, it means either the change is explicitly breaking and it is a bug in our package, or the change is not meant to be breaking and it is the bug in their package. Anyway it must be resolved before the release, otherwise users of stable would be very surprized.
Is this one ready to apply? |
IMHO it is. But you need to squash everything into 1 commit. |
Darn it I thought I hit squash. I will have to revert and reapply manually. |
On 21.11.18 18:57, Karl Nelson wrote:
Darn it I thought I hit squash. I will have to revert and reapply manually.
please do not revert, this will mess up the history even more :)
|
@marscher, the history is already overwritten and I fully approve that. It is quite improbable that someone has cloned this repo in that state, and even if one has and added some patches upon that branch, it is not a big problem to rebase. |
The revert on the web interface didn't do what I wanted it to do. As you pointed out it just created a new pull request which if applied would make a further mess. Thus I had to manually perform the reset to the merge point, squash the new patch in with the correct history, and the delete the tag created by the revert request. I rewrote the history with the correct squash/merge and tested that it worked properly by rebasing existing pull requests. Serves me right for trying to hit the merge button on my phone without double checking that I had squash set, then having to run to find a computer to fix the mess in 30 minutes. |
@KOLANICH BTW big thanks for speeding up the appveyor. That will really help when trying to debug something that only appears in the CI. |
Hmm, I haven't. I have actually slowed it down: now it builds lots of packages for lots of python versions (even the old 3.4 - the latest version available to the ones using WinXP) and platforms (someone still uses 32 bit Windows because of lower RAM requirements) and installs bleeding edge versions of 3 packages. The purpose of this PR was publishing of bleeding edge prebuilt packages, so users won't have to wait for a release in order to use them. |
It is nice we will have more people testing the latest versions. Does setuptools_scm include a jpype. |
@marsher, of course only to metadata. I doubt that there is any good way to guess that a package owner wants this variable to be set. Of course you can request the data using pkg_resources, but this is slow and inefficient. |
I would consider the package attribute useful.
Am 22. November 2018 19:43:03 MEZ schrieb KOLANICH <[email protected]>:
…
@marsher, of course only to metadata.
|
May be wrong, have never used appveyor before.