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

Move -Wmissing-prototypes from Makefile to .travis.yml #79

Merged
merged 1 commit into from
Jul 16, 2016

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented Jul 16, 2016

since MSVC doesn't understand this flag, and the current
mechanism for building Julia with MSVC goes through the makefile

as suggested by @petercolberg at #55 (comment)

since MSVC doesn't understand this flag, and the current
mechanism for building Julia with MSVC goes through the makefile
@tkelman
Copy link
Contributor Author

tkelman commented Jul 16, 2016

I shouldn't spend too much time getting to the bottom of this right now, but there might be a serious issue with utf8proc 2 and/or flisp or julia's use of it with msvc, even 2013 which has at least been able to get through Julia's bootstrap without all that many patches. If I try to use this branch of utf8proc with any version of Julia since the upgrade to utf8proc 2 was merged, I get

 cd /c/projects/julia/base && /c/projects/julia/usr/bin/julia.exe -C x86-64 --output-ji /c/projects/julia/usr/lib/julia/inference.ji --startup-file=no coreimg.jl
make[1]: *** [/c/projects/julia/usr/lib/julia/inference.ji] Error 253

or similar when trying to start bootstrap. Reverting JuliaLang/julia@10359ec gets through bootstrap again, ref https://ci.appveyor.com/project/tkelman/julia/build/1.0.917 (failing to compile ccalltest.c at the end is expected). Anyone who's curious about this should be able to reproduce by working off of https://github.com/tkelman/julia/commits/tk/avmsvc-clean and enabling appveyor on your personal fork of Julia.

@stevengj stevengj merged commit 47cbf7d into master Jul 16, 2016
@stevengj stevengj deleted the tk/movewarningflag branch July 16, 2016 10:16
@stevengj
Copy link
Member

Regarding the MSVC problem that's concerning, but I'm not sure what the problem could be. On Linux, I ran the utf8proc tests through valgrind with no problems.

@tkelman
Copy link
Contributor Author

tkelman commented Jul 16, 2016

that won't exercise the msvc-only ifdefs, which might now be subtly wrong for the latest code - ref warnings noted in #63 (comment)

@stevengj
Copy link
Member

I just looked at how flisp is calling utf8proc, and it seems wonky and antiquated. I'll put together a PR.

@stevengj
Copy link
Member

stevengj commented Jul 16, 2016

Though I don't see why it should crash. I was worried about the low-level calls to utf8proc_decompose etcetera, but they seem fine; they still match what utf8proc_map does internally. The only change I want to make is that we should call utf8proc_category rather than directly accessing utf8proc_property_t, but I don't see why that should crash it.

@tkelman
Copy link
Contributor Author

tkelman commented Jul 16, 2016

Hm. Dunno, guess I can try getting it going locally and see if I can remember how to work windbg, but I've got higher-priority things to work on right now. Mostly checking if the back-burner "in case we need it" msvc support might actually catch a real bug once in a while.

@stevengj
Copy link
Member

Yeah, I totally agree. Porting to multiple compilers is a great way to catch bugs. (Unfortunately, sometimes you catch compiler bugs too.)

tkelman added a commit that referenced this pull request Jul 26, 2016
stevengj pushed a commit that referenced this pull request Jul 27, 2016
* Add NEWS.md items for #79 and #80

* Prepare version numbers for 2.0.2

* Also update API version to 2.0.2
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.

2 participants