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

fix chunking bug #179

Merged
merged 1 commit into from
Dec 31, 2014
Merged

fix chunking bug #179

merged 1 commit into from
Dec 31, 2014

Conversation

stevengj
Copy link
Member

This fixes a chunking bug identified by @jakebolewski in #178 (and includes a regression test).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.63%) when pulling ee41aa6 on stevengj:chunkbug into d531c08 on timholy:master.

@stevengj
Copy link
Member Author

Whoops, looks like a problem with this patch in Julia 0.3; will push a revised patch shortly.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) when pulling 3c4e6df on stevengj:chunkbug into d531c08 on timholy:master.

@stevengj
Copy link
Member Author

Uh oh, now there's a problem on 32-bit windows. Does Travis try 32-bit Linux or OSX?

@timholy
Copy link
Member

timholy commented Nov 20, 2014

I think Travis is 64-bit only, but I'm not certain.

@jakebolewski
Copy link

The bug looks Win32 specific, your branch passes the tests for Linux32.

On Wed, Nov 19, 2014 at 8:05 PM, Tim Holy [email protected] wrote:

I think Travis is 64-bit only, but I'm not certain.


Reply to this email directly or view it on GitHub
#179 (comment).

@stevengj
Copy link
Member Author

@tkelman, any thoughts on why there would be a Win32-specific bug here?

@tkelman
Copy link
Contributor

tkelman commented Nov 20, 2014

Dunno, maybe something involving the size of long, I think that's the major size-of-datatypes difference between Win32 and Linux32. Does this involve both Blosc and HDF5? If so, how did you compile Blosc? There could be binary incompatibilities between what you used and what the build service used for HDF5. Give me some time and I can test things locally, if you have specific things you'd like me to try.

@stevengj
Copy link
Member Author

Yes, this involves both Blosc and HDF5; Blosc is compiled with mingw32.

@tkelman
Copy link
Contributor

tkelman commented Nov 21, 2014

Blosc is compiled with mingw32

You mean legacy mingw.org mingw32? That's not good, those compilers are borderline obsolete and I wouldn't recommend anyone use them these days vs the i686 version of mingw-w64. There are quite a few runtime and header differences there. If you've statically linked the gcc runtime into the blosc dll (which it looks like, since you're just downloading a single file), I wouldn't be surprised if that's leading to trouble.

@tkelman
Copy link
Contributor

tkelman commented Nov 21, 2014

gcc -v would be useful info if the machine you did it on is nearby

@stevengj
Copy link
Member Author

I used i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2) on Debian for the 32-bit Windows binary, and i686-w64-mingw32-gcc (GCC) 4.6.3 for the 64-bit Windows binary. I've used these for cross-compiling other libs and never had a problem.

Correction, the 64-bit binary was built with x86_64-w64-mingw32-gcc (GCC) 4.6.3. And the 32-bit binary was built with i686-w64-mingw32-gcc (GCC) 4.6.3, not i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2). So I think these are the mingw-w64 versions?

@tkelman
Copy link
Contributor

tkelman commented Nov 21, 2014

Okay, yeah that's definitely mingw.org. I didn't realize you could still get those cross-compilers in up-to-date Linux distributions, I thought they had all transitioned to only supporting mingw-w64 by now.

I'm not sure whether using consistent compilers would fix this problem, but it couldn't hurt. Any special configure flags that you can remember, or did you build blosc the obvious straightforward way? I can try rebuilding blosc locally (or writing a spec file to put in winrpm) with i686-w64-mingw32-gcc and see if it makes any difference.

edit: i686-w64-mingw32-gcc is mingw-w64, but the version number might be a factor. we'll see over in Blosc.jl since it doesn't look like it's a HDF5 issue.

@stevengj
Copy link
Member Author

Can you just try Pkg.test("Blosc")? I should set up Blosc.jl to run Travis AppVeyor on Windows. (I just built it in the obvious way, e.g. x86_64-w64-mingw32-gcc -shared -O3 -msse2 -I. *.c -o libblosc64-1.5.0.dll)

@stevengj
Copy link
Member Author

Okay, looks like the Blosc test is failing on Win32 with Appveyor, so there must be a problem with the Win32 binary (or with the Blosc.jl ccalls.)

@stevengj
Copy link
Member Author

After rebasing, now everything is failing... it can't seem to install the HDF5 dependency. Any ideas?

@tkelman
Copy link
Contributor

tkelman commented Dec 19, 2014

Looks like winrpm might be down, the upstream repository is rebuilding a bunch of things again. And homebrew is mad, I don't know how to fix Will not force Homebrew to rebuild dependency "libhdf5"..

@jakebolewski
Copy link

Bump, the Windows failures for Blosc.jl should be fixed.

@timholy
Copy link
Member

timholy commented Dec 29, 2014

I've restarted the tests.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling 02a737b on stevengj:chunkbug into d830040 on timholy:master.

@stevengj
Copy link
Member Author

I removed the workaround since Blosc.jl is passing now on 32-bit windows.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.58%) when pulling 6b9b297 on stevengj:chunkbug into 3c1296a on timholy:master.

@stevengj
Copy link
Member Author

AppVeyor is dying in Pkg.build with failed to download zlib1 ... seems like an AppVeyor problem since it is failing to download http://download.opensuse.org/repositories/windows:/mingw:/win64/openSUSE_13.1//noarch/mingw64-zlib1-1.2.8-8.1.noarch.rpm which works for me (at least on Mac....I don't know if the Windows shell is getting confused by the slashes or something, but presumably this worked at one point in time?).

@timholy
Copy link
Member

timholy commented Dec 30, 2014

I will be happy to merge this when someone tells me it's OK for Windows, too, but I don't have any insight or way to test directly.

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

@timholy would you mind adding me and other committers here as a collaborator on your appveyor account so we can restart builds manually if needed?

We'll need to adjust appveyor slightly so it downloads the latest release instead of 0.3.0, I'll do that now.

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

The failure to download zlib looks like general unreliability of downloads from WinRPM, someone (probably me) needs to figure out how to mirror the upstream rpm repository JuliaPackaging/WinRPM.jl#36 since it's fairly unstable and we have a lot of download trouble. I checked the packaging on zlib (https://build.opensuse.org/package/show/windows:mingw:win64/mingw64-zlib) and it doesn't look like anything has changed there recently. They do occasionally change packaging around in ways that break things for us, so we should eventually set up a daily/weekly sanity check that packages like this one, Cairo, Tk, Gtk, Cbc, etc can all be installed before pulling the latest updates from the upstream repository into our stable mirror.

@timholy
Copy link
Member

timholy commented Dec 31, 2014

Done. For some reason my attempt to add @stevengj gave a "User with specified email address does not exist."

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

You can also disable or delete the webhook for github-multi-status if you'd like Tim, since it's completely redundant now.

@timholy
Copy link
Member

timholy commented Dec 31, 2014

I just spent about 4 minutes searching for how to do it. If you know how, be my guest.

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

Oh, sorry, it's only doable by people with admin access to the repo - under settings, webhooks & services, disable or delete the one that starts with http://github-multi-status.herokuapp.com/hook

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

Oh right @stevengj you'll need to tag a new version of Blosc, or we could do Pkg.checkout("Blosc") on AppVeyor here. Would also be nice to get JuliaIO/Blosc.jl#6 included in that.

edit: No, nevermind, that isn't the cause of the failure. It just looks like the appveyor configuration was still downloading 0.3.0. Might need a rebase again to update the appveyor.yml configuration for the merge?

@stevengj
Copy link
Member Author

(I've tagged a new Blosc version, but I'm not sure why it would matter since the failure was due to a Julia bug and not to a Blosc.jl bug.)

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

You're right, I edited my comment, sorry bout that. I guess the way AppVeyor works is it reads the appveyor.yml configuration from the merge commit at time of build queuing, and when we rebuild it later it doesn't see that the content of appveyor.yml has changed. So it was still running on Julia 0.3.0 which has the same bug. I'll test this locally on win32 and if it works then I think we can just merge, but will keep fingers crossed that appveyor will be consistent with local results.

@stevengj
Copy link
Member Author

Just force-pushed a rebase, let's see if that fixes appveyor.

@tkelman
Copy link
Contributor

tkelman commented Dec 31, 2014

It does work locally on win32 with Julia 0.4-dev, so I'm inclined to think AppVeyor should be successful this time. (It's queued, someone may have restarted a second build for 44ae186? don't think I did that.) Sorry this has taken so many iterations.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling 1da4e1e on stevengj:chunkbug into 44ae186 on timholy:master.

tkelman added a commit that referenced this pull request Dec 31, 2014
@tkelman tkelman merged commit 2bd70a8 into JuliaIO:master Dec 31, 2014
@timholy
Copy link
Member

timholy commented Dec 31, 2014

I find both AppVeyor's and Travis' interfaces semi-maddening, although slowly I've gotten used to Travis'. Here's what I do:

  • Go to https://ci.appveyor.com/projects. It shows me "HDF5.jl"
  • Hover over the gray bar. New icons show up ("Start new build", "Project builds history", "Deployments", "Project NuGet Feed", "Edit project settings"). That last one sounds good, click on that.
  • New page with tabs across the top "Latest build", "History", "Deployment", "Settings". I'm on "Settings". Long menu on the left side, starting with "General" and ending with "Delete project"
  • Start clicking through that menu. The most promising entry is "Notifications," but the only thing I can do on the notifications tab is add another one. Notably, "webhooks" is nowhere to be found. If I click on the "add" link, suddenly I'm presented with a popup menu that includes webhooks as one of its items. But sure enough, all I can do is add webhooks, not delete them.

@timholy
Copy link
Member

timholy commented Dec 31, 2014

Ah, you meant on GitHub! Done.

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.

5 participants