[ncurses] set TERMINFO_DIRS environment variable
#6994
Conversation
| @@ -88,5 +88,11 @@ dependencies = [ | |||
| HostBuildDependency("Ncurses_jll"), | |||
| ] | |||
|
|
|||
| init_block = raw""" | |||
| if Sys.isunix() | |||
There was a problem hiding this comment.
I am not even sure we need this... Adapted it from Xorg_xkeyboard_config
N/Ncurses/build_tarballs.jl
Outdated
| @@ -88,5 +88,11 @@ dependencies = [ | |||
| HostBuildDependency("Ncurses_jll"), | |||
| ] | |||
|
|
|||
| init_block = raw""" | |||
| if Sys.isunix() | |||
| ENV["TERMINFO"] = get(ENV, "TERMINFO", joinpath(Ncurses_jll.artifact_dir, "share", "terminfo")) | |||
There was a problem hiding this comment.
This I also adapted from Xorg_xkeyboard_config -- it only sets TERMINFO to point at the artifact if it wasn't already set. So the user can still set their own TERMINFO.
However, looking at man terminfo, it seems the precedence is TERMINFO > $HOME/.terminfo > TERMINFO_DIRS so perhaps I should set TERMINFO_DIRS here instead?
There was a problem hiding this comment.
I am aware of that issue, and in fact I am using the withenv workaround myself in GAP.jl. However, it is limited: I have code paths the user triggers which could also in the end need this. It is not practical to repeat the withenv in all code. So I think addressing this once and forall here makes sense.
Of course there is a hypothetical chance we'll break something. But that's true for almost any change we make. We'll deal with it if we get reports about it? In the worst case we can revert this change here -- of course we'll then (re-)break other things, but such is life.... ?
There was a problem hiding this comment.
Another option is that I just set the variable globally in GAP.jl, then it'll be limited in which code the change affects -- so breakage still is possible, but minimized shrug
There was a problem hiding this comment.
Yeah, especially for ccalls into library wrapping every single call with withenv is annoying.
giordano
changed the title
TERMINFO_DIRS environment variable
N/Ncurses/build_tarballs.jl
Outdated
| @@ -88,5 +88,11 @@ dependencies = [ | |||
| HostBuildDependency("Ncurses_jll"), | |||
| ] | |||
|
|
|||
| init_block = raw""" | |||
| if Sys.isunix() | |||
| ENV["TERMINFO_DIRS"] = get(ENV, "TERMINFO_DIRS", joinpath(artifact_dir, "share", "terminfo")) | |||
There was a problem hiding this comment.
Maybe we should append our path at the end (colon-separated) when there is already a TERMINFO_DIRS variable set?
Just to make sure that a valid directory is in there since there are different variants of how this directory is organized (using characters or ascii-decimal numbers for the subfolders), and this is a compile-time flag.
There was a problem hiding this comment.
That sounds like a good idea.
|
I just realised this PR makes https://github.com/giordano/Htop.jl redundant, as it was just setting TERMINFO_DIRS, I think I'll delete the repo, |
See JuliaPackaging/BinaryBuilder.jl#687
The recent change by @barche in PR #6980 reminded me about this.
(I should have done it with my recent ncurses 6.4 update sigh but since ncurses only seems to use versions of the form MAJOR.MINOR, it should be OK to use this fake version)