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

Re-enable running tests with multiple processes on Win64 #12257

Merged
merged 5 commits into from
Jul 22, 2015

Conversation

yuyichao
Copy link
Contributor

Since #7942 should be fixed now.

Somehow I'm getting errors from LLVM headers that seems totally unrelated. The build also takes much longer to download at the beginning.

@tkelman

@yuyichao yuyichao added the system:windows Affects only Windows label Jul 21, 2015
@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

Not sure what's wrong yet, but it might be related to how we have the compiler download cached on appveyor - you're supposed to get a Restoring "x86_64-4.9.2-release-win32-seh-rt_v4-rev3.7z"...OK right at the beginning so we can avoid downloading from slow unreliable sourceforge on every build, but the cache might be branch-by-branch or something strange like that?

@yuyichao
Copy link
Contributor Author

but the cache might be branch-by-branch or something strange like that?

That's also my guess and the only possible explaination I have at this point is that the cache is hashed by the environment setup...

@yuyichao
Copy link
Contributor Author

@tkelman FWIW, I see the following error on all master builds and not on any PR's. Is this expected / related?

On 32bit

Updating build cache
Cache entry not found: C:\projects\julia\x86_64-4.9.2-release-win32-seh-rt_v4-rev3.7z
Cache entry not found: C:\projects\julia\llvm-3.3-x86_64-w64-mingw32-juliadeps.7z

On 64bit

Updating build cache
Cache entry not found: C:\projects\julia\i686-4.9.2-release-win32-sjlj-rt_v4-rev3.7z
Cache entry not found: C:\projects\julia\llvm-3.3-i686-w64-mingw32-juliadeps.7z

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

That's expected, it just means it doesn't have the opposite bitness downloaded files - so far it hasn't been an issue?

@yuyichao
Copy link
Contributor Author

so we can avoid downloading from slow unreliable sourceforge on every build

Do we not have any more reliable download sources? (after github removes the download page)

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

I just moved a bunch of things to bintray. And we have our caching server which goes to s3. I can copy the relevant downloads over to bintray and adjust the url's, if you don't mind me pushing to this branch?

@yuyichao
Copy link
Contributor Author

if you don't mind me pushing to this branch?

Go ahead! That's the point of using a branch here. =)

one remaining exception is for msys make which does not need to be downloaded on appveyor
@yuyichao
Copy link
Contributor Author

The download address change doesn't seem to help =(.

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

cache of llvm might be stale for this branch 5db4311

yuyichao added a commit that referenced this pull request Jul 21, 2015
@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

Not sure whether that'll fix it, but apologies in retrospect for not versioning these binary filenames when I had to rebuild them for ABI changes.

@yuyichao
Copy link
Contributor Author

Not sure whether that'll fix it, but apologies in retrospect for not versioning these binary filenames when I had to rebuild them for ABI changes.

No need to apologize since you are the only one who will deal with these. ;-p

@yuyichao
Copy link
Contributor Author

Yay, the compilation passes!

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

Yeah, was just a stale cache for whatever reason - had you used this branch name before maybe? So switching from sourceforge to bintray wasn't strictly necessary, but the downloads should be faster and more reliable now.

@yuyichao
Copy link
Contributor Author

had you used this branch name before maybe?

I think it's not because of the branch name since it appears on three of my branches yyc/appveyor-test, yyc/appveyor-freeze and this one (yyc/windows-test) two of which are new (this one and yyc/appveyor-freeze) and two have worked before on very recent master (yyc/appveyor-test and yyc/appveyor-freeze).

Other new branches/PR's also don't have issue using the cache and this issue only appears for whatever commits not having the cpu cores setting, which is why I think it is indexed by the environment settings.

@tkelman
Copy link
Contributor

tkelman commented Jul 22, 2015

Ah, the most recent time we ran appveyor without JULIA_CPU_CORES: 1 was probably with a previous version of the llvm binaries that had a different ABI. So deleting JULIA_CPU_CORES: 1 went back to that stale cached copy of the binaries. I guess that makes sense.

@yuyichao
Copy link
Contributor Author

Yeah, that makes perfect sense then (I was actually wondering why it has something in the cache if it was a new setting and had never finished the build to get a change to update the cache yet....)

I vote for either add checksum or version the filename....

@tkelman
Copy link
Contributor

tkelman commented Jul 22, 2015

add checksum

We can totally do that. We have the technology.

@yuyichao
Copy link
Contributor Author

Hmmm, did I miss sth or is the cache not updated somehow?

https://ci.appveyor.com/project/StefanKarpinski/julia/build/1.0.7095/job/74o9m5aew7cml52m#L32

@tkelman
Copy link
Contributor

tkelman commented Jul 22, 2015

I think the cache only gets updated for branch builds, not pr builds. I could be wrong there.

@yuyichao
Copy link
Contributor Author

Makes sense then. I guess this is also why I don't see the cache update error for other PR builds.

So this actually tests that my non-locally-tested code works for both cases? LOL. Should be good to merge then after AppVeyor passes (or if you want to wait for Travis struggling with OSX.....).

tkelman added a commit that referenced this pull request Jul 22, 2015
Re-enable running tests with multiple processes on Win64
@tkelman tkelman merged commit 26f048e into master Jul 22, 2015
@tkelman tkelman deleted the yyc/windows-test branch July 22, 2015 02:10
@yuyichao
Copy link
Contributor Author

I'm excited now to see if we are free of the AppVeyor hang in the next few days...

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 46147ed on yyc/windows-test into ** on master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants