Add preferences overloading #27

staticfloat · 2021-01-13T23:49:32Z

This makes use of Preferences.jl to allow overriding of the paths of products. This is necessary for JLLWrappers being used in v1.7 (so that we can service USE_BINARYBUILDER_XXX=1 more easily, and also so that we can service USE_SYSTEM_XXX=1), as well as for the Spack work.

…n v1.6+

This adds a `@load_preferences()` lookup that defaults to the artifact-local location for products, but can be overridden to arbitrary paths, including system-wide searches by writing out just the SONAME of the desired library.

User code (such as `BinaryBuilder`) needs to generate JLLs that have `Preferences` as a dependency.

giordano

Nice, but compatibility with Julia v1.5- looks a bit problematic

giordano · 2021-01-13T23:52:51Z

src/products/executable_generators.jl

@@ -58,10 +58,11 @@ macro declare_executable_product(product_name)
 end

 macro init_executable_product(product_name, product_path)
+ preference_name = string(product_name, "_path")
 path_name = Symbol(string(product_name, "_path"))


Maybe this can be

Suggested change

path_name = Symbol(string(product_name, "_path"))

path_name = Symbol(preference_name)

to ensure consistency?

I thought about that; but I'm not entirely sure I want the preference name to be libfoo_path; perhaps just libfoo is better? What do you think?

I see, good point. For the preference the _path is probably redundant

src/toplevel_generators.jl

test/runtests.jl

giordano · 2021-01-14T00:02:30Z

Did you measure impact on loading performance, for example for GTK3_jll?

src/wrapper_generators.jl

Co-authored-by: Mosè Giordano <[email protected]>

staticfloat · 2021-01-14T00:41:16Z

Did you measure impact on loading performance, for example for GTK3_jll?

Not yet; definitely something that needs doing; also I need to ensure that these operations are happening at the desired time (compile time, not runtime).

I decided that having `_path` on the end of preferences is a smart idea, so that we can do other things like `_dlflags` or something in the future.

Previously, we were loading preferences within `__init__()`; move it out to top-level statements so that the preferences are loaded at compile time and baked into the `.ji` files.

staticfloat · 2021-01-15T18:23:13Z

I did the performance comparison; no measurable performance impact as long as the work is done at compile-time, which I have verified with the latest update!

giordano · 2021-01-15T18:29:17Z

I'd say the failure on nightly is unrelated, but I don't really understand what's the failure at all:

JLLWrappers.jl: Error During Test at /home/runner/work/JLLWrappers.jl/JLLWrappers.jl/test/runtests.jl:11
  Got exception outside of a @test
  Failed to precompile Vulkan_Headers_jll [8d446b21-f3ad-5576-a034-752265b9b6f9] to /home/runner/.julia/compiled/v1.7/Vulkan_Headers_jll/jl_5SHlbR.
  Stacktrace:
    [1] error(s::String)
      @ Base ./error.jl:33
    [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IOStream, internal_stdout::IOContext{Base.PipeEndpoint})
      @ Base ./loading.jl:1356
    [3] compilecache(pkg::Base.PkgId, path::String)
      @ Base ./loading.jl:1302
    [4] _require(pkg::Base.PkgId)
      @ Base ./loading.jl:1017
    [5] require(uuidkey::Base.PkgId)
      @ Base ./loading.jl:910
    [6] require(into::Module, mod::Symbol)
      @ Base ./loading.jl:897
    [7] eval
      @ ./boot.jl:369 [inlined]
    [8] (::var"#3#10")()
      @ Main /buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:669
    [9] (::Base.redirect_stdio)(thunk::var"#3#10", stream::IOStream)
      @ Base ./stream.jl:1240
   [10] #2
      @ /buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:668 [inlined]
   [11] open(::var"#2#9", ::String, ::Vararg{String}; kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Base ./io.jl:330
   [12] open
      @ ./io.jl:328 [inlined]
   [13] macro expansion
      @ /buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:667 [inlined]
   [14] (::var"#1#8")(dir::String)
      @ Main ~/work/JLLWrappers.jl/JLLWrappers.jl/test/runtests.jl:24
   [15] mktempdir(fn::var"#1#8", parent::String; prefix::String)
      @ Base.Filesystem ./file.jl:732
   [16] mktempdir(fn::Function, parent::String)
      @ Base.Filesystem ./file.jl:730
   [17] macro expansion
      @ ~/work/JLLWrappers.jl/JLLWrappers.jl/test/runtests.jl:12 [inlined]
   [18] macro expansion
      @ /buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1152 [inlined]
   [19] top-level scope
      @ ~/work/JLLWrappers.jl/JLLWrappers.jl/test/runtests.jl:12
   [20] include(fname::String)
      @ Base.MainInclude ./client.jl:451
   [21] top-level scope
      @ none:6
   [22] eval
      @ ./boot.jl:369 [inlined]
   [23] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:268
   [24] _start()
      @ Base ./client.jl:492

BTW, maybe we should run tests on Julia v1.6 now. The syntax in GitHub Actions should be ^1.6.0-0

staticfloat · 2021-01-15T18:37:02Z

It's on 32-bit only; I'll try to reproduce

staticfloat · 2021-01-15T19:04:23Z

X-ref: JuliaLang/julia#39274

staticfloat added 4 commits January 13, 2021 00:04

Ignore .vscode

0b2a534

Tweak OpenLibm_jll UUID so that it doesn't conflict with our stdlib i…

f88b1cf

…n v1.6+

Add Preferences overrides for product paths

4dcf06f

This adds a `@load_preferences()` lookup that defaults to the artifact-local location for products, but can be overridden to arbitrary paths, including system-wide searches by writing out just the SONAME of the desired library.

Bump to v1.3.0

08dc949

User code (such as `BinaryBuilder`) needs to generate JLLs that have `Preferences` as a dependency.

staticfloat requested review from vchuravy and giordano January 13, 2021 23:49

giordano reviewed Jan 14, 2021

View reviewed changes

Add Preferences.jl compat shims for Julia v1.3-1.5

393d670

giordano reviewed Jan 14, 2021

View reviewed changes

src/wrapper_generators.jl Outdated Show resolved Hide resolved

src/wrapper_generators.jl Outdated Show resolved Hide resolved

Apply suggestions from code review

da1c30a

Co-authored-by: Mosè Giordano <[email protected]>

vchuravy mentioned this pull request Jan 14, 2021

Update README.md EnzymeAD/Enzyme.jl#35

Merged

staticfloat closed this Jan 14, 2021

staticfloat reopened this Jan 14, 2021

staticfloat added 2 commits January 14, 2021 20:18

Merge preference_name and path_name

0ccd118

I decided that having `_path` on the end of preferences is a smart idea, so that we can do other things like `_dlflags` or something in the future.

Move work to compile time

c2bec03

Previously, we were loading preferences within `__init__()`; move it out to top-level statements so that the preferences are loaded at compile time and baked into the `.ji` files.

giordano mentioned this pull request Jan 19, 2021

Generate wrapper header: override override path #28

Closed

staticfloat closed this Jan 19, 2021

staticfloat reopened this Jan 19, 2021

staticfloat merged commit 11ddbff into master Jan 19, 2021

staticfloat deleted the sf/preferences branch January 19, 2021 20:15

giordano mentioned this pull request Feb 4, 2021

Do not make JLL packages require Preferences #29

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add preferences overloading #27

Add preferences overloading #27

staticfloat commented Jan 13, 2021

giordano left a comment

giordano Jan 13, 2021

staticfloat Jan 14, 2021

giordano Jan 14, 2021

giordano commented Jan 14, 2021

staticfloat commented Jan 14, 2021

staticfloat commented Jan 15, 2021

giordano commented Jan 15, 2021

staticfloat commented Jan 15, 2021

staticfloat commented Jan 15, 2021

	path_name = Symbol(string(product_name, "_path"))
	path_name = Symbol(preference_name)

Add preferences overloading #27

Add preferences overloading #27

Conversation

staticfloat commented Jan 13, 2021

giordano left a comment

Choose a reason for hiding this comment

giordano Jan 13, 2021

Choose a reason for hiding this comment

staticfloat Jan 14, 2021

Choose a reason for hiding this comment

giordano Jan 14, 2021

Choose a reason for hiding this comment

giordano commented Jan 14, 2021

staticfloat commented Jan 14, 2021

staticfloat commented Jan 15, 2021

giordano commented Jan 15, 2021

staticfloat commented Jan 15, 2021

staticfloat commented Jan 15, 2021