-
Notifications
You must be signed in to change notification settings - Fork 143
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
Tag a new release - 0.5 #117
Comments
We should probably run julia's tests against it on a variety of platforms. |
What's the best way to do so? The changes here are minor and there are no api changes - so I think this is pretty safe. |
Is it likely that there will be and API changes in the future? If not, then a 1.0 release may be warranted. Bugfixes would go into 1.0.x releases; new exported functions would go in 1.x releases, and any breaking API changes (including deleting exported functions) would require a 2.0 release. |
Prepare the Julia PR that changes the
Not necessarily API changes, but in terms of code #108 will be pretty big and still needs some cleanup. I also think we should look carefully at what musl has done with their libm since they started from pretty much the same code bases we did, but have been better at maintaining and pulling in bug fixes. |
Can you point to the musl implementation? The issue we have faced is that we never automated the migration from msun. It needs a bunch of sed scripts really, and unfortunately I didn't save that in #108. I'll have to do it once more carefully. |
By definition, there should be no major API changes, since the goal is to be a libm replacement. I would like to sort out #108 and keeping up with msun before tagging 1.0. |
http://git.musl-libc.org/cgit/musl/tree/src/math - looks like they don't have powerpc support (edit: at least not as optimized assembly) at the moment though |
That code looks quite clean! Do they maintain a record of what all they pull in from msun and how frequently? |
I meant tracking the msun logs. Looks like it started from msun, but has significantly changed, and ongoing fixes are manually tracked. I'll try this out to see if there is merit in following musl instead of msun before refreshing msun. |
Detailed notes on the musl approach - http://nsz.repo.hu/libm/ Many functions have been completely rewritten in musl since the original checkin. |
The numpy maintainers have also brought up bionic's libm, used on android: https://github.com/android/platform_bionic/tree/master/libm |
Bionic looks nice in that it does closely follow msun. |
I think at the very least, we should follow the bionic model here, and maintain a set of patches external to msun that we need in openlibm. With that we can be 1.0. I love what the musl guys have done, but need to check with them on long term maintenance plans and understand the community momentum, before we adopt that as the way forward. What Szabolcs Nagy has done there, is what I wanted to do with openlibm in the first place, but he's done really amazing stuff. |
I've just checked that building a RPM works with GCC6, and that's OK. But tests fail on i686:
test-float used to pass on i686, though test-double failed. (Tests also fail on ARMv7 but that was already the case with 0.4.) Also, do we actually need to bump the SOVERSION to 2? Did the API really break? |
I did not bump the SOVERSION here, but I do believe it was required when we removed some APIs. I think the test failure on ARM is now a couple of minor things. |
the undefined references also occurred on win32 appveyor for the test pr, and win64 errored due to incorrect dllexport |
@tkelman Could you turn on appveyor? I logged into appveyor and authorized it to access openlibm and kicked off a build? But, I guess we need an appveyor.yml file and some other things to be done to turn this on for every commit and PR? |
The i686 build is fixed. Can you try again? Should work on windows too, but would be nice to setup appveyor. |
There isn't currently a template for doing source builds with gcc on appveyor, it takes a fair bit of work to obtain the right toolchain and run from within msys2 or cygwin, it's not a 5-minute thing. |
Ok. Then I think this is good to tag for now, and we can make further releases as we find issues. |
For now there should be an option to "only build when appveyor.yml is present" that you can turn on to avoid the red status until we figure that out. |
I checked |
You can rebase JuliaLang/julia#15212 for now to see if it works on Windows within Julia's build system - I think there are still dllexport problems that need to be fixed before tagging. AppVeyor here standalone would have to look like a cut-down version of Julia's appveyor setup, which is kind of complicated. I actually am not sure whether openlibm's makefiles work properly if you try to build them on Windows by themselves, outside of Julia's build system. |
At least locally it's working on win32 but failing on win64, see the appveyor log from that PR for the dllexport-related error. Bisect points to 8b3b520 as being the first bad commit. |
I merged #118 and verified that all tests pass on 32-bit and 64-bit linux, as well as OS-X is the same level of tests passing as before. On ARM 32-bit, which used to hang/crash, the test suite now runs completely, with a few FP exception errors. If win32 and win64 are looking good, I think this is a go for 0.5. |
Wait for a bit, I'm running the build. |
Looks good, thanks! Also good to see ARM tests closer to passing completely. If you want a perfect release, you can have a look at these warnings:
https://kojipkgs.fedoraproject.org//work/tasks/9161/13149161/build.log |
What, the tests pass on PPC64LE? That's news! Being greedy, I should note that similar warnings appear there:
|
Oh, you missed all the fun yesterday... |
If you know how to get rid of the redef warnings, I'd be grateful. |
Hehe, I was actually going though my e-mail starting from the most recent. Not a the most logical way...
I hadn't realized it was system-dependent. Actually, that's because my |
As regards warnings on PPC, this is because recent gcc versions provide |
I have a patch shortly. |
Actually I was also looking into it. We'll be able to compare our changes. |
This should do it - I think, in
|
But will that work on all platforms? Why keep |
That's the question I have. If it works on linux and windows, we can get rid of bsd_cdefs.h |
Looks like it works if I get rid of all occurrences of |
See my
https://kojipkgs.fedoraproject.org//work/tasks/9885/13149885/build.log |
Regarding EDIT: we may as well call them |
BTW, if somebody wants to put a bounty on the first openlibm distribution package supporting PPC... :-) OK, I'm cheating a bit. |
Either fix is fine on the ENDIAN flags. This should fix the issue I was facing with |
2016-02-27 17:38 GMT+01:00 Milan Bouchet-Valat [email protected]:
I think the preferred way of testing the endianness is to use $ cc -dM -E - < /dev/null | grep ORDER Ed Schouten [email protected] |
Would be great if you could submit a PR! |
I've made a PR at #125. Indeed, I now think the cleanest solution is to use the gcc/clang macros, and define them when they aren't. That way, the code is easy to read for anybody familiar with these compilers. |
I think we are good to tag 0.5 now. |
sounds good to me |
Sorry, looks like I missed another warning with |
BTW, there are still a few warnings with GCC6. I've looked at them, and they don't seem to reflect actual bugs. They don't need to be fixed for the release, but it might be good to have a look at some point so that we can spot more easily really problematic ones. |
Tagged v0.5. |
Could you call it 0.5.0 instead? We'll probably have 0.5.1 after this rather than 0.6. |
Same for openspecfun. I've prepared draft releases. |
Good point. I may take a few hours to get to it, but if it is easy enough, can you edit it on github? |
Sure, I just did it. You'll have to update your PR JuliaLang/julia#15212 though. |
Done. |
With the ppc support, I think we should tag a new release - 0.5. Is there anything else we need to do before we tag?
The text was updated successfully, but these errors were encountered: