search

BioJulia / Libz.jl

Fast, flexible zlib bindings.
Other
27 stars 17 forks source link

Add compatibility for Julia 1.3-rc4 #66

Closed staticfloat closed 4 years ago

staticfloat commented 4 years ago

Add compatibility for changes in Julia 1.3

Types of changes

This PR implements the following changes:

:clipboard: Additional detail

Previously, Julia bundled a zlib1.dll file on windows, necessitating platform-specific ccall() handles. In Julia 1.3, we are unifying the library name, so you will no longer need to do this. The added VERSION check here should make this package work on both 1.2- and 1.3+ versions of Julia, however if you want to simplify, you could change future versions of this package to be compatible with Julia 1.3+ and only use the libz name.

Note that we probably won't be shipping libz with Julia forever; eventually we may remove it entirely, so I would encourage you to switch over to providing your libz through the new Pkg Artifacts/JLL package system. (To emphasize how easy that would be, you would just need to switch this package over to using Project.toml, define it as compatible with Julia 1.3+, then add Zlib_jll as a dependency, say using Zlib_jll in your package, and you're done. Importing that Zlib_jll package will define libz for you, and will download/install it at Pkg.add() time so that you never need to worry about Julia shipping or not shipping libz in the future.

staticfloat commented 4 years ago

I've updated this PR to include an upgrade of the testing scripts to better reflect the current best accepted practices. I've also upgraded the package to declare compatibility with Julia 1.0+, hopefully nobody is still using 0.7. :)

kescobo commented 4 years ago

Any idea what this is about in the appveyor build?:

C:\julia\bin\julia -e 'using Libdl; Libdl.dlopen("Libz")'
ERROR: syntax: incomplete: invalid character literal

Maybe worth switching over to travis for windows too, see here.

I think it would also be good to allow failures on nightly.

staticfloat commented 4 years ago

Alright, the debugging showed me what I thought; the failure here is because Julia 1.3-rc3 doesn't include libz, but 1.3-rc4 should (and does, as shown in the nightly, although libz.dll is broken due to https://github.com/JuliaLang/julia/pull/33535). Let's merge this once that PR is merged and we get a new nightly that passes these tests; then when 1.3-rc4 is released next week, this should go all-green.

TransGirlCodes commented 4 years ago

@staticfloat 33535 is merged, is this ready to merge now too?

staticfloat commented 4 years ago

Let's re-trigger CI and see what happens on the nightly versions. If it passes, then yes, this is good to merge and you can tag a new release.

KristofferC commented 4 years ago

Let's try again now that RC-4 is on travis?

staticfloat commented 4 years ago

Yep, the last result is exactly what we expected (nightly passes, 1.3 fails) and if we do it one more time, we should get a full green flush!

giordano commented 4 years ago

Bump :slightly_smiling_face:

abx78 commented 4 years ago

bump!!! it's green!!!

abx78 commented 4 years ago

is this going to be released as a new version? I still pick up the one without this fix

JackDunnNZ commented 4 years ago

Is it possible for a release to be tagged please?

timholy commented 4 years ago

Anything depending on WinRPM (which is a lot of packages) would really benefit from a new release!

JackDunnNZ commented 4 years ago

@staticfloat are you able to tag a release here or ping someone that can please?

staticfloat commented 4 years ago

I cannot; I think we need @BenJWard to do it.

andreasnoack commented 4 years ago

Let's try pinging @bicycle1885 as well. Would you be able to make a new release?

TransGirlCodes commented 4 years ago

Ok I've just set the version in the project for 1.0.1 since this is a non-breaking change, and deleted the manifest file as it didn't look like it should be there.

I'll see if I can migrate it over to using the artifacts system and then make a release.

KristofferC commented 4 years ago

Does it not make sense to just tag a release right now with this and then an artifact release could come whenever that is finished?

TransGirlCodes commented 4 years ago

@JuliaRegistrator register branch=master

JuliaRegistrator commented 4 years ago

Pull request comments will not trigger Registrator as it is disabled. Please trying using a commit or issue comment.

TransGirlCodes commented 4 years ago

Ok, there's a pending release here: https://github.com/JuliaRegistries/General/pull/6530

TransGirlCodes commented 4 years ago

Does anyone have any insight as to why the AutoMerge checking over there is telling me the number of supported julia versions is being reduced? As far as I can tell, the julia compatibility specified is correct, according to how semver is described in Pkg docs.