[feature request] add path/filepath.JoinList(elem ...string) string

[Dave Cheney]

path/filepath has both Split and Join for paths, but lacks a JoinList
counterpart for lists.

This function would immediately be useful inside the dashboard
builder, and for anyone manipulating PATH style environment variables
for use with os/exec.

I'm happy to implement this, but before I begin I wanted to ask if
this would be accepted.

Cheers

Dave

[Daniel Theophanes]

Just out of curiosity, What would this do? Could you give an example?

[David Symonds]

path.Join already has a signature of func Join(elem ...string) string.
What would JoinList look like?

[Dave Cheney]

(to this list, this time)

I was thinking of

// JoinList joins any number of list elements into a single list, adding
// an OS-specific ListSeparator. Elements that contain the OS-specific
// ListSeperator are split, and recombined internally.
// Empty elements are ignored.
func JoinList(elem ...string) string

[Daniel Theophanes]

Just out of curiosity, how is this different then the current Join sig?
Could you point out the diff?

// Join joins any number of path elements into a single path, adding
// a Separator if necessary. The result is Cleaned, in
// all empty strings are ignored.
func Join(elem ...string) string

[Rémy Oudompheng]

People's answer show what I fear, which is that coexistence of similarly named Join and JoinList causes great confusion.

We could also add a PathList type with a String method.

Rémy

[Dave Cheney]

Join() joins filepaths

Join("a", "b", "c") => "a/b/c"

JoinList joins PATH style environment lists

JoinList("a","b","c") => "a:b:c"

the idea being

path := filepath.JoinPath(os.Getenv("PATH"), "path/to/my/test/binary")

[Dave Cheney]

> People's answer show what I fear, which is that coexistence of similarly
> named Join and JoinList causes great confusion.
>
> We could also add a PathList type with a String method.

Yup, I'm not wedded to the name, I was just following the pattern in
path/filepath.

[David Symonds]

I'd vote against this. It's easy enough to use strings.Join with
os.ListSeparator.

[Dave Cheney]

On Mon, Jan 7, 2013 at 6:16 PM, David Symonds <dsym...@golang.org> wrote:
> I'd vote against this. It's easy enough to use strings.Join with
> os.ListSeparator.

Yes, but it happens that folks forget and assume all $PATHs are unix
paths. Here is an example I found this afternoon, which is actually
the opposite, using strings.Split() rather than filepath.SplitList()

https://codereview.appspot.com/7057052/

[Patrick Mylund Nielsen]

Checkmate.

--

[Dave Cheney]

There are also some edge cases which should be taken into account,

I think unix deals with PATH=/usr/bin::/usr/sbin # note the ::

but I don't know what other operating systems do.

Here is a proposal anyway, https://codereview.appspot.com/7067049/

[David Symonds]

The point of having filepath.Join (and friends) is to make it easy to
portably manipulate paths. You're proposing something to help with
portably manipulating environment lists. It's a reasonable goal, but
I'm not sure it's useful enough to be in the standard library and
hence be supported for N years. People monkey with paths all the time,
but I can't remember when I last felt the need to do anything with an
environment list.

[Dave Cheney]

That is a fair comment, they are rare, but we already have
filepath.SplitList() which pulls them apart. I would like a facility
that lets me put them back together again.

[David Symonds]

On Mon, Jan 7, 2013 at 6:25 PM, Dave Cheney <da...@cheney.net> wrote:

> That is a fair comment, they are rare, but we already have
> filepath.SplitList() which pulls them apart. I would like a facility
> that lets me put them back together again.

The symmetry would be nice, but even there of all the code that deals
with such lists I would imagine that 95% or more are just parsing
them, so SplitList is needed but JoinList is not. You're just sitting
on a corner case.

[Andrew Gerrand]

We have SplitList, and the only reason we don't have JoinList is because we didn't think to add it.

On 6 January 2013 23:23, David Symonds <dsym...@golang.org> wrote:

It's a reasonable goal, but
I'm not sure it's useful enough to be in the standard library and
hence be supported for N years.

Doesn't seem like such a burden to me, and I've had need for it a few times.

Really,

  filepath.JoinList(list)

is much nicer than

  strings.Join(list, string(filepath.ListSeparator))

and it's not like we're going to want to use JoinList for anything else (it would be too confusing, what with SplitList there already!)

Andrew

[Dave Cheney]

So, one against, one for (with JoinList, not PathList) ... what is the
consensus here folks ?

[Patrick Mylund Nielsen]

Happy to settle the tie with "for" :)

--

[roger peppe]

i'd be happy to see JoinList added for symmetry, if nothing else.

> --
>
>

[speter]

What would it do when an element contains the separator?

Unix-ish systems generally allow directory names to contain ':' but you cannot include such directories in PATH. I'm not sure how it is handled for other uses; that is, do all applications using PATH-like environment variables / parameters handle it the same way as shells handle PATH, or do some of them support / require escaping colons...

If it is to be added, I'd suggest making it something like "JoinList(elem ...string) (string, error)", and return an error when an element contains the separator. That would at least allow it to correctly handle the most common case of PATH.

Peter

--

[minux]

On Thu, Jan 10, 2013 at 5:18 AM, speter <spete...@gmail.com> wrote:

What would it do when an element contains the separator?

Unix-ish systems generally allow directory names to contain ':' but you cannot include such directories in PATH. I'm not sure how it is handled for other uses; that is, do all applications using PATH-like environment variables / parameters handle it the same way as shells handle PATH, or do some of them support / require escaping colons...

If it is to be added, I'd suggest making it something like "JoinList(elem ...string) (string, error)", and return an error when an element contains the separator. That would at least allow it to correctly handle the most common case of PATH.

but then we can't use it this way:

path := filepath.JoinList(os.Getenv("PATH"), "path/to/my/test/binary")

and forced to write:

path, err := filepath.JoinList(append(filepath.SplitList(os.Getenv("PATH")), "path/to/my/test/binary"))

if err != nil {

     // what to do here?

}

[Dave Cheney]

> Unix-ish systems generally allow directory names to contain ':' but you
> cannot include such directories in PATH. I'm not sure how it is handled for
> other uses; that is, do all applications using PATH-like environment
> variables / parameters handle it the same way as shells handle PATH, or do
> some of them support / require escaping colons...

Have a look at the test cases in https://codereview.appspot.com/7067049/

> If it is to be added, I'd suggest making it something like "JoinList(elem
> ...string) (string, error)", and return an error when an element contains
> the separator. That would at least allow it to correctly handle the most
> common case of PATH.

We don't do that for path.Join(), which is essentially the same, but
for /, instead we clean the elements supplied.

[speter]

On Thu, Jan 10, 2013 at 6:44 AM, minux <minu...@gmail.com> wrote:

On Thu, Jan 10, 2013 at 5:18 AM, speter <spete...@gmail.com> wrote:

What would it do when an element contains the separator?

Unix-ish systems generally allow directory names to contain ':' but you cannot include such directories in PATH. I'm not sure how it is handled for other uses; that is, do all applications using PATH-like environment variables / parameters handle it the same way as shells handle PATH, or do some of them support / require escaping colons...

If it is to be added, I'd suggest making it something like "JoinList(elem ...string) (string, error)", and return an error when an element contains the separator. That would at least allow it to correctly handle the most common case of PATH.

but then we can't use it this way:

path := filepath.JoinList(os.Getenv("PATH"), "path/to/my/test/binary")

I didn't know this was an intended use case; it is not covered in (my interpretation of) Dave's description. For this use case I'd prefer having something like AppendToList()...

and forced to write:

path, err := filepath.JoinList(append(filepath.SplitList(os.Getenv("PATH")), "path/to/my/test/binary"))

if err != nil {

     // what to do here?

}

At least this one will never add directories to your path that you didn't intend to. "What to do here" will obviously depend on the application...

Peter

[speter]

I don't think it is equivalent with Join(). In the case of Join(), elements can only contain '/' when they have been joined in some way previously. This is not the case when handling (lists of) directory names.

Another issue is empty directories. While it is not common for users to have the empty directory in their paths, those who do likely have it for a reason and it is generally not desirable to remove them.

The documentation is clear about the behavior (sorry I somehow missed that part previously), but I think it puts too much responsibility on the caller (handling empty paths in list, checking for separator in directory name) which limits its usefulness / added value considerably.

Peter

[Andrew Gerrand]

On 10 January 2013 09:39, speter <spete...@gmail.com> wrote:

I don't think it is equivalent with Join(). In the case of Join(), elements can only contain '/' when they have been joined in some way previously. This is not the case when handling (lists of) directory names.

I think you're overthinking it, and I am sympathetic to minux's criticism. As API designers we should optimize for the common case, not the corner case. Adding AppendToList is going too far IMO. One of the main reasons for adding JoinList is for symmetry with Join, and the way Dave currently specifies it in his CL is very symmetrical. Doing some escaping (what escaping? what scheme?) on the path separator is not this function's responsibility.

 

Another issue is empty directories. While it is not common for users to have the empty directory in their paths, those who do likely have it for a reason and it is generally not desirable to remove them.

I can't think of any reason why this would ever be desirable. An empty path is meaningless on all operating systems that I know. Can you provide an example?

Andrew

[Ian Lance Taylor]

On Wed, Jan 9, 2013 at 5:06 PM, Andrew Gerrand <a...@golang.org> wrote:
>
> I can't think of any reason why this would ever be desirable. An empty path
> is meaningless on all operating systems that I know. Can you provide an
> example?

On Unix systems an empty element on a path effectively means the
current directory. Try putting a ':' at the start or end of PATH, or
putting "::" in PATH.

Ian

[Andrew Gerrand]

On 10 January 2013 12:30, Ian Lance Taylor <ia...@google.com> wrote:

On Unix systems an empty element on a path effectively means the
current directory.  Try putting a ':' at the start or end of PATH, or
putting "::" in PATH.

Wow, so it does! I thought you had to use ".".

[spete...@gmail.com]

On Jan 10, 2013, at 10:06, Andrew Gerrand <a...@golang.org> wrote:

On 10 January 2013 09:39, speter <spete...@gmail.com> wrote:

I don't think it is equivalent with Join(). In the case of Join(), elements can only contain '/' when they have been joined in some way previously. This is not the case when handling (lists of) directory names.

I think you're overthinking it, and I am sympathetic to minux's criticism. As API designers we should optimize for the common case, not the corner case. Adding AppendToList is going too far IMO. One of the main reasons for adding JoinList is for symmetry with Join, and the way Dave currently specifies it in his CL is very symmetrical. Doing some escaping (what escaping? what scheme?) on the path separator is not this function's responsibility.

Symmetry is nice when possible but I don't think it works out in this case. There is ambiguity whether an "element" is a directory name or a list. Consider JoinList("", "/mybin"): if "" is a dirname, the result should be ":/mybin", if it is a list (PATH) it should be "/mybin".

There is no ambiguity if JoinList only takes dirnames, and in that case I'd prefer it also did error check (no separators in dirnames). In that case, to append to PATH you could split+join or add a separate AppendToList.

Peter

[Jason Del Ponte]

Would JoinList need to be that smart?  I'd imagine a method that took two, or more strings and inserted ':' or os specific path list separators between the elements, and returned the generated string. Nothing more.  In this situation you could do:

   list := JoinList(OS.GetEnv("PATH"), mySecondPath, "some/other/path")

and be done with it. I think it would put too much responsibility on the JoinList method to include error checking/validation of the paths also.  Unless possibly verifying the paths provide use correct directory separators and valid characters .e.g '/' for linux and '\' windows, but again that dives into something else's responsibility, not directly JoinList's.

[speter]

Some of my previous messages were written in a bit of a hurry so please let me summarize / clarify my stance.

I think the primary concern should be how the new API helps users (developers) solve problems in practical use cases. Symmetry is a desirable property when adding new APIs, but it should not be a goal by itself. Another desirable property is "easy to use correctly, hard to use incorrectly", which I think is more important in this case.

As far as I can see, two specific use cases have been mentioned so far:
1: joining individual filepaths into a PATH-like pathlist
2: appending individual filepaths to a PATH-like pathlist
(Please don't hesitate to expand this list.)

Handling lists such as PATH is error-prone. The issues I've mentioned may seem like nitpicking, but I think these are especially important because the new API would likely be commonly used for managing execution paths such as PATH, CLASSPATH, PYTHONPATH etc, and thus promoting a solution (by adding a new API) that does not correctly solve the use cases could lead to common bugs in Go programs potentially leading to arbitrary code execution.

Because of this, I only support the idea of adding an API for joining filepaths if it helps users avoid common mistakes. (Doing it the wrong way is easy enough already with the existing APIs.) But it was only until relatively late in this thread that I realized that the proposed function didn't do what I would expect from such a new API to do (solve either use case while avoiding common mistakes).

The two most common mistakes I can think of are:
A: inadvertently adding the empty filepath to the list
(e.g. `os.Getenv("PATH") + os.PathListSeparator + filepath`, when PATH is potentially empty)
B: adding a set of unrelated filepaths due to a filepath containing the separator

So in my point of view the added value of a new API would be helping to avoid A and B in use cases 1 and 2; I'll refer to these as 1A, 1B, 2A, 2B (and possibly there are more).

The proposed specs / implementation initially didn't help users avoid any of 1A, 1B, 2A, 2B.
IIUC the updated version solves 1A, but avoiding 1B, 2A, 2B is still the caller's responsibility.

My concern is that such an addition can do more bad than good by giving users a false sense of security. Unlike with strings.Join(), they would be using an API specifically added for the purpose of joining filepaths, so it is not unreasonable for them to expect it to work properly in common cases (avoid common errors).

If the scope of the new function remains handling any mix of filepaths and lists-of-filepaths at the same time, as in the existing proposal, there is inherent ambiguity (the desired result of JoinList("", "/mybin") is different for use case 1 and 2). So I think that at the very least some specific explanation should be added to the docs about the potential issues for each use case, and perhaps examples for how to avoid them.

Limiting the scope of use cases directly handled by the function to just 1 (individual filepaths), and making it return an error when a path contains the separator would avoid 1A and 1B thus give a complete solution to that use case (easy to use correctly). Moreover, it would usually prevent users from using it in unintended ways (hard to use incorrectly). Adding a separate function for use case 2 may be an overkill, in that case an example could be added that describes how to do it correctly with the help of JoinList.

I'm glad to contribute to the implementation, tests, docs, examples if there is interest, whichever approach is deemed better (although it may not be sometime next week until I'll have time to work on it).

Peter

--
 
 

[speter]

On Fri, Jan 11, 2013 at 4:02 AM, Jason Del Ponte <delp...@gmail.com> wrote:

Would JoinList need to be that smart?  I'd imagine a method that took two, or more strings and inserted ':' or os specific path list separators between the elements, and returned the generated string. Nothing more.

For that you can already use strings.Join.

 

 In this situation you could do:

   list := JoinList(OS.GetEnv("PATH"), mySecondPath, "some/other/path")

and be done with it.

This will put the current directory in your list when PATH is not set, which is likely not the intended result.

 

I think it would put too much responsibility on the JoinList method to include error checking/validation of the paths also.  Unless possibly verifying the paths provide use correct directory separators and valid characters .e.g '/' for linux and '\' windows, but again that dives into something else's responsibility, not directly JoinList's.

'\' and '/' is clearly unrelated; if you use wrong inputs (invalid filepath on that OS) you get wrong results. JoinList shoudn't interfere with this; it would just make it more complicated to troubleshoot such issues.

Separators in filepaths is a different issue. These are valid filepaths for the OS, but cannot be added to PATH. This issue is specific to joining filepaths into lists, so my opinion is that if a new API is added with the specific purpose of joining filepaths into lists, it should take this into consideration. (See my other message for details.)

Peter

 

On Wednesday, January 9, 2013 8:01:59 PM UTC-8, speter wrote:

On Jan 10, 2013, at 10:06, Andrew Gerrand <a...@golang.org> wrote:

On 10 January 2013 09:39, speter <spete...@gmail.com> wrote:

I don't think it is equivalent with Join(). In the case of Join(), elements can only contain '/' when they have been joined in some way previously. This is not the case when handling (lists of) directory names.

I think you're overthinking it, and I am sympathetic to minux's criticism. As API designers we should optimize for the common case, not the corner case. Adding AppendToList is going too far IMO. One of the main reasons for adding JoinList is for symmetry with Join, and the way Dave currently specifies it in his CL is very symmetrical. Doing some escaping (what escaping? what scheme?) on the path separator is not this function's responsibility.

Symmetry is nice when possible but I don't think it works out in this case. There is ambiguity whether an "element" is a directory name or a list. Consider JoinList("", "/mybin"): if "" is a dirname, the result should be ":/mybin", if it is a list (PATH) it should be "/mybin".

There is no ambiguity if JoinList only takes dirnames, and in that case I'd prefer it also did error check (no separators in dirnames). In that case, to append to PATH you could split+join or add a separate AppendToList.

Peter

 

Another issue is empty directories. While it is not common for users to have the empty directory in their paths, those who do likely have it for a reason and it is generally not desirable to remove them.

I can't think of any reason why this would ever be desirable. An empty path is meaningless on all operating systems that I know. Can you provide an example?

Andrew

--
 
 

[Jason Del Ponte]

You are absolutely right that it is not obvious what the best rules to apply are. But to play devils advocate, building an additional api with rules above those of the underlying system limits its usability.

- ':' are valid characters in a file - Due to this a method would need to be chosen to either, allow pre-existing path list provided within a single string as a valid input parameter, or instead quote(or other escaping) each input paramenter to explicitly identify them as independent path elements

- an empty string in a path list is valid in unix(not sure about windows) symbolizing local path. - The os.GetEnv() situation is a perfect example of unintended consequent of an empty string being added as a path list element. Alternatively, an empty string is a valid element, which if blocked would require usage of strings.Join(), losing out on what ever potential other validation exists within a path.JoinList(). I've never needed to use an empty string with in a path list, but i'm sure others have.

These issues are exacerbated by the fact that each OS will have their own rules to what makes a path list.  How are such topics handled in other parts of go, specifically when it comes to protecting an api user from them selves?

Cheers,

Jason

[speter]

Sorry for the delay, I've been busy with some other tasks, and it also turned out to be more complicated than I had thought.

I think manipulating PATH is the most common scenario and is one that the path/filepath package already explicitly targets (e.g. it is mentioned in the godoc for SplitList), so any addition should solve that (and other situations with equivalent semantics). Some programs may use quoting and/or escaping to avoid the list-separator-in-filepath issue, but SplitList doesn't support those (and cannot support them in a backwards compatible manner) so I think it should be out of scope for path/filepath.

I didn't really want to create a "counter-proposal" implementation, but I figured it is the most specific way to express my ideas, and it makes it much easier for both myself and others to verify that those actually work in practice.

So here is a CL; not for submission (at least not yet), but more just to show the approach I think would work best. (Would need at least more detailed docs, and examples for non-unix for submission.)
https://codereview.appspot.com/7141048

--- begin description ---
path/filepath: support joining (lists of) filepaths

This CL targets the following use cases:

U1: join individual filepaths into a list
U2: join lists of filepaths into a list
U3: append / prepend individual filepaths to a list

JoinFilepaths is added to solve U1, and JoinLists to solve U2. U3 can be solved by a combination of the two. Examples are added for each use case.

Type List is added to reduce the chance of users incorrectly using individual filepaths as lists or the other way around (potentially leading to security issues).
--- end description ---

More specifically, this approach avoids the following common problems:
P1: add empty filepath to list when attempting to join with the "empty list" (U2, U3). e.g. appending filepath "/mybin" to empty list "" resulting in ":/mybin" instead of "/mybin".
P2: add multiple unrelated filepaths when attempting to add a single filepath containing the list separator (U1, U3). e.g. attempting to append filepath "/mnt/c:/tmp/bin" to list "/bin:/sbin" resulting in "/bin:/sbin:/mnt/c:/tmp/bin" instead of an error.

Thus, I believe this approach would make it much easier for users to create Go programs that aren't prone to errors and security risks due to incorrect PATH manipulation implementations.

Peter

--
 
 

[speter]

I've been looking into this a bit more and I'm not sure if the current behavior of filepath.SplitList can be considered correct on Windows.

It appears that Windows PATH can contain directory names that include the separator character (semicolon on Windows), as long as the name is quoted. (E.g.: `path "c:\funny;dir;name";c:\windows\system32`.) But filepath.SplitList will split such directory names into two (or more) parts. (In the previous example, {`"c:\funny`, `dir`, `name"`, `c:\windows\system32`}.) This is unlike on unix-ish systems where there is no support for quoting (nor escaping), thus it is not possible to add a directory name that includes the separator (colon) to the PATH (though I only checked POSIX docs and only tested with Linux/bash). Plan9 is of course unaffected by such issues. :)

Considering that this behavior on Windows can have security implications (e.g. in the above example, treating `dir` as a system-blessed directory when looking for executables), I believe it should be fixed to not split quoted directory names on Windows. Should I prepare a CL for this?

(Since filepath.SplitList is also used for splitting GOPATH, a related potential issue is how GOPATH should behave with quoted directory names and/or directory names with semicolons on Windows.)

Peter

[Dave Cheney]

Yes, SplitList should be fixed, we use that for GOPATH a lot. I think there is an issue somewhere about GOPATH and quoted paths. 

[speter]

I have created https://codereview.appspot.com/7181047/ for SplitList, but build on Windows is broken right now (crypto/x509 test fails). Should I hg-mail it now or wait until build is clean?

I couldn't find the issue for GOPATH and quoted paths, but as long as splitting is done correctly, unquoting can be considered a separate issue.

Peter

[minux]

On Tue, Jan 22, 2013 at 10:11 PM, speter <spete...@gmail.com> wrote:

I have created https://codereview.appspot.com/7181047/ for SplitList, but build on Windows is broken right now (crypto/x509 test fails). Should I hg-mail it now or wait until build is clean?

there is already LGTMed CL (7185043) fixing that breakage so feel free to mail your CL out

as the build breakage on windows should be fixed soon.

[speter]

It's been a while, but I finally got around to put together a cross-platform implementation that supports joining/appending/prepending for path-lists, as an external package.

I decided to use a new separate type List for lists (such as PATH / GOPATH) and string for individual filepaths, to avoid the ambiguity whether a value represents an individual path or a list. (E.g. on Unix, ":" can refer to a directory name consisting of a single colon, or a list consisting of the empty path "". In addition, Windows allows and sometimes requires quoted paths for %PATH%, but only allows unquoted paths for most file APIs, so using the same type for both is error-prone.)

go get speter.net/go/exp/path/filepath
http://speter.net/go/exp/path/filepath#List

The package also contains wrappers for functions in "path/filepath" so that both can be used with a single import.

Any feedback is welcome.

Peter

[Matt Reynolds]

Wall of text, sorry.  Not sure if it's better to respond in an old thread or start a new one, so I'll lean towards more context and resurrect this thread.

So, it's been over a year since this thread started, and effort appears to have stalled.  I'd love to have the JoinList (or PathList?) in the runtime, so I'm willing to do some work to get it there.  Here's what I've found.

Re-reading this thread and the review ( https://codereview.appspot.com/7181047/ and https://codereview.appspot.com/7067049/ ), it appears that the function was left with the following feedback/criticism :

* SplitList is useful, but has no symmetric function, either SplitList or JoinList

* While SplitList is useful, a few folks pointed out that having a more useful set of functions to deal with common env-var use cases would be useful also (appending to a list, etc)

* A few flaws in the existing implementations relating to path ambiguities were pointed out ( https://codereview.appspot.com/7067049/#msg18 ) and trouble with consistent delivery of functionality across platforms (noting Windows allows for all sorts of weird options)

To recap, the purpose of "path/filepath" is "Package filepath implements utility routines for manipulating filename paths in a way compatible with the target operating system-defined file paths."  I read this as having the following qualities:

* Filepath will help you manipulate strings that look like paths ; it doesn't *enforce* the underlying OS requirements for "valid" paths (but it'll help where it can)

* Filepath doesn't seem to attempt to deal with all underlying platform implementations, and leans heavily towards POSIX (filepath.ExpandEnv only expands Unix style vars ($), not Windows(%))

TL;DR - I propose providing simplistic string manipulation help for paths and names, leave path search and correctness to the developer (since filepath doesn't seem to help with this in other cases).  Another option is to add more specific functionality to "os" and reference it from filepath.

* Add JoinList supported on POSIX and windows (I started working on the impl, although I have no Windows machine on which to test)

** On POSIX systems, we accept [:]{1,2} the PathListSeparator as a valid entry, but we emit it strictly as the POSIX spec hints/suggests, replacing "::" at the start or end of a path string with a .: or :. respectively (Postel's law).

- "A zero-length prefix is a legacy feature that indicates the current working directory. It appears as two adjacent <colon> characters ( "::" ), as an initial <colon> preceding the rest of the list, or as a trailing <colon> following the rest of the list. A strictly conforming application shall use an actual pathname (such as .) to represent the current working directory in PATH." from http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

** In any other case, PathListSeparator is treated as part of the path, assuming the caller knows what they want.

** On Windows systems, I can't find a similar set of guidelines (help?), so I suggest sticking with the existing rigidly simplistic behavior ; a PathListSeparator present in a string will be treated as a PLS, quoting or no.  This is broken sometimes, but works most times.  Not sure if this is desired outside the "os" packages.

Further reading of the source ( https://github.com/golang/go/blob/master/src/path/filepath/path_windows.go#L75 , https://github.com/golang/go/blob/master/src/os/exec/lp_unix.go#L33 , etc ) leads me to believe that there is potential overlapping functionality in the "os" and "filepath" packages.  I'll keep looking to see if it's worthwhile to create an "os/path" which could then be used by "filepath" for the platform specific crap (it also appears that we could use syscalls to reduce the ambiguity in the "os" packages, making calls into Windows APIs to have it resolve the quoting crap for us).

Digression about colons in directory names with POSIX :

"Since <colon> is a separator in this context, directory names that might be used in PATH should not include a <colon> character." (http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03)

Tests I've run ( https://github.com/InfCol/sati/blob/master/util/path-exp.bash ) on darwin and linux (via bash) indicate this behavior holds out.  You can add a directory with a colon in the name to the PATH list ; it doesn't seem to provide search results with any combination of escaping of colons I've tried.

[speter]

> Filepath doesn't seem to attempt to deal with all underlying platform implementations, and leans heavily towards POSIX (filepath.ExpandEnv only expands Unix style vars ($), not Windows(%)

I think that's a mischaracterization of "path/filepath"; you seem to be conflating it with "os". (Note that it is "os.ExpandEnv", not "filepath.ExpandEnv".)

"path/filepath" aims to be "compatible with the target operating system-defined file paths" (emphasis on consistency with OS, no mention of *nix bias). In contrast, "os" aims to be "platform-independent" with a "Unix-like" design. So using '$' on all platforms is reasonable in "os", but I'd find something like that rather surprising in "filepath".

> I have no Windows machine on which to test

I haven't tried it but AppVeyor claims[1] to support Go on Windows and is free for open-source projects.


> On POSIX systems...

What would JoinList("", "/mybin") return?
Returning "/mybin" would make it asymmetric with SplitList, breaking programs trying to SplitList -> manipulate -> JoinList.
Returning ":/mybin" or ".:/mybin" would introduce a potential security issue when people try to use it for appending as in JoinList(os.Getenv("PATH"), "/mybin").

> On Windows systems, I can't find a similar set of guidelines (help?)

I'm not aware of such guidelines, but the CL for the SplitList fix that you linked to above [2] includes some tests for validating that SplitList's behavior is consistent with how Windows handles PATH. The same can be used for validating the behavior for join. Considering efficiency and the principle of least astonishment, I think the best approach is to quote the whole path element when it is required (when the path element includes a ';'), and use it verbatim otherwise.

> a PathListSeparator present in a string will be treated as a PLS, quoting or no.  This is broken sometimes, but works most times.

There's already strings.Join for use cases where "broken sometimes, but works most times" is enough. If anything, I see this line of thinking more as making the case *against* the need for an addition to the stdlib. Also note that in the review comment that you linked to above [3], rsc has stated explicitly that SplitList/JoinList would need to handle quoting/unquoting in accordance with OS behavior.


[1] http://www.appveyor.com/blog/2013/09/25/appveyor-update-and-new-pricing
[2] https://codereview.appspot.com/7181047/patch/33002/30017
[3] https://codereview.appspot.com/7067049/#msg18


Just my two cents,

Peter

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.