-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add preferences overloading #27
Conversation
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but compatibility with Julia v1.5- looks a bit problematic
@@ -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")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this can be
path_name = Symbol(string(product_name, "_path")) | |
path_name = Symbol(preference_name) |
to ensure consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, good point. For the preference the _path
is probably redundant
Did you measure impact on loading performance, for example for |
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
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.
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! |
I'd say the failure on nightly is unrelated, but I don't really understand what's the failure at all:
BTW, maybe we should run tests on Julia v1.6 now. The syntax in GitHub Actions should be |
It's on 32-bit only; I'll try to reproduce |
X-ref: JuliaLang/julia#39274 |
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 serviceUSE_BINARYBUILDER_XXX=1
more easily, and also so that we can serviceUSE_SYSTEM_XXX=1
), as well as for the Spack work.