Canonicalises path from withSystemTempDirectory

[robstewart57]

… Stack.Setup

The System.IO.Temp withSystemTempDirectory function from
System.IO.Temp used in Stack.Setup returns a non-canonicalised
path. However, the parseAbsDir function in the Path module of the path
library expects canonicalised paths.

The change is applied to stack setup, sanityCheck and stack upgrade
in Stack.Setup .

Fixes #1017

[robstewart57]

Hang off accepting this PR actually, this same problem is causing a problem elsewhere. More anon...

[snoyberg]

Looks good. One thought: perhaps instead of modifying each place in the code, we could add a withSystemTempDir function to Path.IO that would handle the canonicalizing and parseAbsDir call, to make sure that future temp directory usages don't hit this.

By any change, do you have a temporary directory environment variable set to a relative path?

[robstewart57]

@snoyberg thanks for the review, and I agree. Rather than moving a version of withSystemTempDirectory to the path library, I've submitted a pull request to the temporary library.

batterseapower/temporary#16

If this gets accepted to temporary, I have a corresponding stack commit that uses the withCanonicalisedSystemTempDirectory function throughout, in place of withSystemTempDirectory. And I will submit another stack PR with this commit if or when the https://github.com/robstewart57/temporary/commit/6db0f9952d66d74363cb127d00b7586aefe3c53e temporary commit gets reflected on hackage.

So I'm closing this PR.

[snoyberg]

Actually, Path.IO is in stack itself, so it could just be added here. I'd still recommend going that route, as it will be easier to deal with dependencies that way.

[robstewart57]

@snoyberg ah ha! I've got to the bottom of this bug. It's a really interesting corner case.

The withSystemTempDirectory is used in a number of places throughout stack. It's implementation in the temporary library uses getTemporaryDirectory from System.Directory. The documentation says:

On Unix, getTemporaryDirectory returns the value of the TMPDIR environment variable or "/tmp" if the variable isn't defined.

I've looked into my ~/.bash_profile and sure enough,

# for bibtex2html
export TMPDIR=.

The bib2texhtml documentation says to do this:

Note for users using a version of bibtex2html prior to 1.98: TeXlive 2010 prevents bibtex2html to run bibtex in a temporary directory, resulting in an error message such as .... A workaround consists in telling bibtex2html to use the current directory for temporary files, using the following shell command before running bibtex2html:

export TMPDIR=.

The path starting "." is eventually passed back to stack in all places where withSystemTempDirectory is used in stack. The stack code base always assumes a canonicalised path is provided from this function, though as this TMPDIR=. corner case demonstrates that this is not necessarily the case.

@snoyberg What do think is the best resolution?

1. Ignore this corner case, the number of users who've specified the TMPDIR environment variable, for bibtex2html or otherwise, is tiny.
2. Canonicalise the path received from withSystemTempDirectory, as per my first PR.
3. Copy the temporary library functions to Path.IO in stack, with each modified to ensure that all with* functions return canonicalised paths.

[snoyberg]

I'd still say 3 is the right approach. Just to be clear though, I don't
anticipate any code being copied from the temporary package, those library
functions can just be wrapped.

On Wed, Sep 23, 2015, 12:01 AM Rob Stewart notifications@github.com wrote:

@snoyberg https://github.com/snoyberg ah ha! I've got to the bottom of
this bug. It's a really interesting corner case.

The withSystemTempDirectory is used in a number of places throughout
stack. It's implementation
https://github.com/batterseapower/temporary/blob/master/System/IO/Temp.hs#L35
in the temporary library uses getTemporaryDirectory from
System.Directory. The documentation
https://hackage.haskell.org/package/directory-1.2.3.1/docs/System-Directory.html#v:getTemporaryDirectory
says:

On Unix, getTemporaryDirectory returns the value of the TMPDIR environment
variable or "/tmp" if the variable isn't defined.

I've looked into my ~/.bash_profile and sure enough,

for bibtex2html

export TMPDIR=.

The bib2texhtml documentation
https://www.lri.fr/%7Efilliatr/bibtex2html/ says to do this:

Note for users using a version of bibtex2html prior to 1.98: TeXlive 2010
prevents bibtex2html to run bibtex in a temporary directory, resulting in
an error message such as .... A workaround consists in telling bibtex2html
to use the current directory for temporary files, using the following shell
command before running bibtex2html:

export TMPDIR=.

The path starting "." is eventually passed back to stack in all places
where withSystemTempDirectory is used in stack. The stack code base
always assumes a canonicalised path is provided from this function, though
as this TMPDIR=. corner case demonstrates that this is not necessarily
the case.

@snoyberg https://github.com/snoyberg What do think is the best
resolution?

Ignore this corner case, the number of users who've specified the
TMPDIR environment variable, for bibtex2html or otherwise, is tiny.
2.

Canonicalise the path received from withSystemTempDirectory, as per my
first PR.
3.

Copy the temporary library functions to Path.IO in stack, with each
modified to ensure that all with* functions return canonicalised paths.

—
Reply to this email directly or view it on GitHub
#1019 (comment)
.

[robstewart57]

Done #1047 .