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

Lists and IFS #1999

Closed
wants to merge 11 commits into from
Closed

Lists and IFS #1999

wants to merge 11 commits into from

Conversation

RomanHargrave
Copy link
Contributor

Similar to #1744

LD_LIBRARY_PATH must be a colon-separated string, according to LD.SO(8):

LD_LIBRARY_PATH
    A colon-separated list of directories in which to search for ELF libraries at execution-time.  
    Similar to the PATH environment variable.  Ignored in set-user-ID and set-group-ID programs.

Thus, in order for LD_LIBRARY_PATH to work with more than one entry, it must use : as a separator, rather than be handled as a first-class list in fish. This means I lose the benefits of treating LD_LIBRARY_PATH as a list in my fish configuration (to prevent cluttering). It would be convenient if fish were to implement IFS support for handling edge-cases like this.

Example of non-orthagonal behaviour:

set -x LD_LIBRARY_PATH /usr/local/lib /special_lib
ldd my_executable
    # my_library => Not found
contains /usr/local/lib $LD_LIBRARY_PATH 
    # true

Correct way (notice that LD_LIBRARY_PATH may no longer be treated as an array):

set -x LD_LIBRARY_PATH /usr/local/lib:/special_lib
ldd my_executable
    # my_library => /usr/local/lib/my_library.so.i
contains /usr/local/lib $LD_LIBRARY_PATH
    # false

@RomanHargrave RomanHargrave changed the title LD_LIBRARY_PATH export issues Lists and IFS Mar 26, 2015
@RomanHargrave
Copy link
Contributor Author

See also: #1656

@ridiculousfish
Copy link
Member

In #1374 we decided that only PATH, MANPATH, and CDPATH environment variables shall be made arrays; everything else will be a colon-delimited string. If I understand this request, it's for better support for inspecting colon-delimited strings.

@ridiculousfish ridiculousfish added this to the fish-future milestone Mar 26, 2015
@RomanHargrave
Copy link
Contributor Author

@ridiculousfish I read some other issues, and it seems as though a whitelist for variables that need to be exported with colon delimiters. This is largely what I was referring to. I added LD_LIBRARY_PATH the said list and verified that it worked.

@ridiculousfish
Copy link
Member

I hope we can avoid growing that whitelist. The problem is there's no end to it - if LD_LIBRARY_PATH, why not other LD variables, like LD_PRELOAD and LD_AUDIT, or the OS X analogs like DYLD_LIBRARY_PATH, or the gcc variables like LIBRARY_PATH and COMPILER_PATH.... It's impossible to list them all, and if we change the whitelist between releases, we risk breaking scripts.

(That said, if we're going to change it, now is the time to do it since this will be the first release with the new behavior.)

That said # 2, I wonder if I'm overthinking it. The bad behavior comes from fish splitting on colons when importing env vars. It is arguably less harmful to have fish introduce new colons when exporting them. That is, if fish inherits LD_LIBRARY_PATH, it keeps the colons. If the user then adds a new value to it as a list, fish colon-delimits it on export. So it would be asymmetric, but it might do what the user expects most of the time.

@RomanHargrave
Copy link
Contributor Author

@ridiculousfish in regards to that whitelist growing, perhaps it would be convenient to expose a way for the user to configure which of their environment variables should be exported like so. This prevents fish from having to account for every single program in existence wanting different input (don't you love it?).

@zanchey
Copy link
Member

zanchey commented Mar 30, 2015

That suggested behaviour sounds sane. It would be helpful if there were an easy way to split strings on a delimeter.

@RomanHargrave
Copy link
Contributor Author

@zanchey A split function would definitely be appreciated. The best I can come up with right now is this hack:

function split --argument delim spec
    if [ -f $spec ]
        cat $spec
    else if [ "x$spec" != "x" ]
        echo $spec
    end | sed "s#$delim#\n#g" -
end

which replaces the delimiter with a newline. Note that it supports reading from STDIN, a file, or passing a string. Fish will automatically treat newlines as a list separator, which means that this works:

for letter in (split ',' 'a,b,c,d')
         echo "letter: $letter"
end

and will output

letter: a
letter: b
letter: c
letter: d

Also, pinging @ridiculousfish to let you know that this issue got converted to a pull request (tisk tisk github cli).

@ridiculousfish
Copy link
Member

Also see my old #445 idea

zanchey and others added 10 commits April 12, 2015 23:58
Work on fish-shell#1730.

With thanks to Andrew Lutomirski (github.com/amluto) for the SOCK_DGRAM
trick:
fish-shell#2023 (comment)
Examples that work as expected (even completions don't get confused):

$ begin true; end;
$ begin if true; end; end
$ begin if true; echo hi; end

The last example correctly expects another 'end' to match 'begin'.

Fixes fish-shell#1248.
The TOK_END is swallowed by the subsequent job_list anyway.
Per discussion in fish-shell#1956, back this out until we have consensus.

This reverts commit 20a6b65.
@RomanHargrave
Copy link
Contributor Author

I was just reminded of the existence of this issue when git dicked up. Since #2022 was opened, this no longer needs to be open.

@faho faho removed this from the fish-future milestone Dec 13, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants