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

More robust test/examples.jl #207

Merged
merged 9 commits into from
Feb 12, 2021
Merged

More robust test/examples.jl #207

merged 9 commits into from
Feb 12, 2021

Conversation

tkf
Copy link
Collaborator

@tkf tkf commented Feb 11, 2021

This is a PR onto #200.

I think the test is failing because Base.current_project() doesn't work nicely with --project:

arakaki@amdci2 ~/.j/d/KernelAbstractions (vc/split)> pwd
/home/arakaki/.julia/dev/KernelAbstractions
arakaki@amdci2 ~/.j/d/KernelAbstractions (vc/split)> julia --startup-file=no --project=test -e 'display(Base.current_project())'
"/home/arakaki/.julia/dev/KernelAbstractions/Project.toml"⏎
arakaki@amdci2 ~/.j/d/KernelAbstractions (vc/split)> julia --startup-file=no --project=test -e 'display(Base.load_path())'
3-element Array{String,1}:
 "/home/arakaki/.julia/dev/KernelAbstractions/test/Project.toml"
 "/home/arakaki/.julia/environments/v1.5/Project.toml"
 "/home/arakaki/opt/julia/julia-1.5.2/share/julia/stdlib/v1.5"⏎

A quick workaround may be to use Base.load_path_setup_code() to propagate load path etc to the child processes.

@vchuravy
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Feb 11, 2021
@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

@vchuravy
Copy link
Member

Fascinatingly now it is only partly broken

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

Maybe the failure in Windows is due to the incompatibility between powershell and bash?

https://github.com/JuliaGPU/KernelAbstractions.jl/runs/1882919388?check_suite_focus=true#step:6:3

I just added the patch that is supposed to use bash everywhere.

I'm not sure why it'd fail on ubuntu, though https://github.com/JuliaGPU/KernelAbstractions.jl/runs/1882919107#step:9:128

I can't repro it locally

@vchuravy
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Feb 11, 2021
@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

@vchuravy
Copy link
Member

bors help

@vchuravy
Copy link
Member

bors delegate=@tkf

@DilumAluthge
Copy link
Collaborator

I think it's

bors delegate=tkf

@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

✌️ tkf can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

Thanks, Dilum!

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

bors try

bors bot added a commit that referenced this pull request Feb 11, 2021
@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

bors try

bors bot added a commit that referenced this pull request Feb 11, 2021
@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

bors try

bors bot added a commit that referenced this pull request Feb 11, 2021
@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

bors try

@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

try

Already running a review

@tkf
Copy link
Collaborator Author

tkf commented Feb 11, 2021

bors try

@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

try

Already running a review

@DilumAluthge
Copy link
Collaborator

Why all this business with the load path? Why not just do Pkg.test("KernelAbstractions")?

@vchuravy
Copy link
Member

Ha! If it were just that simple, Pkg.test doesn't like the mono-repo setup we are going for.

@tkf
Copy link
Collaborator Author

tkf commented Feb 12, 2021

So, it looks like this is a Julia 1.6-specific problem JuliaLang/julia#39631

(I think this is orthogonal to the issue that Pkg.test is not enough for this mono-repo setting.)

@vchuravy
Copy link
Member

right you need to add @stdlib

@tkf
Copy link
Collaborator Author

tkf commented Feb 12, 2021

I believe the behavior of 1.6 is a bug. You shouldn't need anything in the top-level dependencies for transitive dependencies. If it's in the manifest, it should work.

@tkf
Copy link
Collaborator Author

tkf commented Feb 12, 2021

OK, github action worked in my repo tkf#1 (esp https://github.com/tkf/KernelAbstractions.jl/runs/1883606478) so I think this is good to go.

I'm not merging via bors since I don't know if it supports merging to a non-default branch.

@bors
Copy link
Contributor

bors bot commented Feb 12, 2021

try

Timed out.

@vchuravy
Copy link
Member

Thanks Taka!

@vchuravy vchuravy merged commit 83554ec into JuliaGPU:vc/split Feb 12, 2021
@DilumAluthge
Copy link
Collaborator

Ha! If it were just that simple, Pkg.test doesn't like the mono-repo setup we are going for.

I'm not sure I follow. You should be able to do the following:

julia test.jl

Where the contents of test.jl are:

import Pkg

# KernelAbstractions
Pkg.develop(Pkg.PackageSpec(path = pwd()))

# CUDAKernels
Pkg.develop(Pkg.PackageSpec(path = joinpath(pwd(), "lib", "CUDAKernals")))

Pkg.test(["KernelAbstractions", "CUDAKernels"])

Does that not work? If it doesn't work, we should patch Pkg to make it work.

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.

3 participants