-
-
Notifications
You must be signed in to change notification settings - Fork 656
Conversation
It would be much, much less risky if JuliaPackaging/BinDeps.jl#277 only used the BinDeps-local version of this code on nightly Julia. |
This is moving code the code from C (where we know it occasionally fails silently, because it used to crash CI until we added code to make it fail silently), to Julia (where that failure should be much less likely, and also get reported correctly). |
I've asked you three or four times now in JuliaPackaging/BinDeps.jl#277 how many packages you tested this against, and not gotten a clear answer. Simple question, simple answer. |
Yes, I made sure it creates the same array from within BinDeps, so that packages that use it are unaffected:
|
In how many cases? Please answer the question. |
Several. Above I'm just showing the success of the latest test. |
CI seems to have gotten confused by the accidental close. Do I need to make a new PR? |
The existing code isn't going anywhere on 0.5 or 0.6. Does this fix anything that didn't already work? It's rather risky that it could break in a situation you haven't tested, so I still think this change should be nightly only. |
Yes. This handles error reporting correctly and reliably, and removes one minor source of unpredictably and undetectable failures from BinDeps.
The existing code breaks in situations that I have encountered. As a bugfix, this improves the reliability of the ldconfig handling on all versions. |
Can you give a concrete example of the old code doing the wrong thing? Should add tests for that if possible. |
If this breaks anything it's getting reverted. And I'd still appreciate a specific example of the old code doing the wrong thing. "Unpredictabl[e] and undetectable failures" and "situations that I have encountered" don't help anyone else understand what the problem is. |
Repository: JuliaLang/BinDeps.jl
Release: v0.7.0
Travis:
Diff: vs v0.6.0
requires
vs v0.6.0: no changescc: @vtjnash
Please make sure that: