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

incremental serialize: restore recache order back to original #22272

Merged
merged 1 commit into from
Jun 9, 2017

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jun 7, 2017

the commit fix for #265 reshuffled this order, but that was wrong
the correctness of jl_recache_other has since been fixed,
permitting this to go back to the correct original order

fix #22171

the commit fix for #265 reshuffled this order, but that was wrong
the correctness of jl_recache_other has since been fixed,
permitting this to go back to the correct original order

fix #22171
@tkelman
Copy link
Contributor

tkelman commented Jun 7, 2017

tests?

@vtjnash
Copy link
Member Author

vtjnash commented Jun 7, 2017

I understand that this order is more correct (that's why it was done this way in v0.5), I just don't yet know how to construct counter-examples (like SIMD.jl triggered in #22171) to use as tests.

@ararslan
Copy link
Member

ararslan commented Jun 7, 2017

You could do something like this maybe to test:

mktempdir() do dir
    f = tempname()
    write(f, """
        Pkg.init("$dir")
        Pkg.clone("https://github.com/eschnett/SIMD.jl.git")
        Pkg.checkout("SIMD", "a4256db194e36a48922d28c3d702cb27d20a925d")
        using SIMD
        exit(isdefined(Main, :SIMD) ? 0 : 1)
    """)
    cmd = `$(Base.julia_cmd()) --startup-file=no --precompiled=yes --compilecache=yes $f`
    @test success(pipeline(cmd, stdout=STDOUT, stderr=STDERR))
end

I haven't checked this particular code, but something like it may work

@vtjnash
Copy link
Member Author

vtjnash commented Jun 7, 2017

The code for the tests needs to be versioned, and preferably a MWE. PkgEval will running tests on this functionality across a large range of code "in the wild".

@vtjnash vtjnash merged commit 8374ead into master Jun 9, 2017
@vtjnash vtjnash deleted the jn/22171 branch June 9, 2017 18:00
@tkelman tkelman added the needs tests Unit tests are required for this change label Jun 9, 2017
vtjnash added a commit that referenced this pull request Jun 13, 2017
the commit fix for #265 reshuffled this order, but that was wrong
the correctness of jl_recache_other has since been fixed,
permitting this to go back to the correct original order

fix #22171

(cherry-picked from 8dab956, PR #22272)
vtjnash added a commit that referenced this pull request Jun 15, 2017
the commit fix for #265 reshuffled this order, but that was wrong
the correctness of jl_recache_other has since been fixed,
permitting this to go back to the correct original order

fix #22171

(cherry-picked from 8dab956, PR #22272)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Unit tests are required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using SIMD hangs on master
3 participants