tempdir() containing spaces breaks installing source packages

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

tempdir() containing spaces breaks installing source packages

Ivan Krylov
Hello everyone!

Temp paths are used in system2() calls without shQuote() because
they are assumed not to contain spaces. On Windows,
GetShortPathName() is used to try to ensure that.

Unfortunately, sometimes GetShortPathName() silently fails to return a
8.3 file path and gives a full path instead (8.3 file names might be
disabled on newer Windows 10 installations, or there may be another
reason). This has been spotted in the wild [*]. When %USERPROFILE%
contains spaces, this results in tempdir() also containing spaces and
prevents the user from being able to install source packages.

As of <https://svn.r-project.org/R/trunk@77563>,

 - src/library/utils/R/packages2.R line 839 contains an unquoted
   temporary file path (fil) passed to system2(), which results in it
   being split and R CMD INSTALL not being able to find the package
   file. In other invocations of R CMD INSTALL in the same file, the
   path is properly quoted.

 - src/library/tools/R/check.R line 125 contains an unquoted temporary
   file path passed to system2, which results in Rterm.exe -f not being
   able to find the RtmpXXXXXX\RinXXXXXXXX file, causing the attempt to
   run tools:::makeLazyLoading(...) to fail.

I can report these two problems (thanks to Martin Maechler for the
Bugzilla account and the advice) and attach the patches required to fix
them, but there might be more. The bug report [**] is somewhat relevant
here (though changing the default behaviour of system2() is obviously
not the right solution as it would break existing code).

Is there anything I should consider before creating the PR as
described above?

--
Best regards,
Ivan

[*] https://stat.ethz.ch/pipermail/r-help/2019-December/465075.html

[**] https://bugs.r-project.org/bugzilla/show_bug.cgi?id=16127

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: tempdir() containing spaces breaks installing source packages

R devel mailing list
You might expand the scope of this a bit to include Windows usernames with
non-ASCII characters in them.  If I recall correctly, if you are logged
under a Cyrillic UTF-8 name then R will not even start.  We have seen this
in the wild.

Bill Dunlap
TIBCO Software
wdunlap tibco.com


On Fri, Dec 13, 2019 at 8:47 AM Ivan Krylov <[hidden email]> wrote:

> Hello everyone!
>
> Temp paths are used in system2() calls without shQuote() because
> they are assumed not to contain spaces. On Windows,
> GetShortPathName() is used to try to ensure that.
>
> Unfortunately, sometimes GetShortPathName() silently fails to return a
> 8.3 file path and gives a full path instead (8.3 file names might be
> disabled on newer Windows 10 installations, or there may be another
> reason). This has been spotted in the wild [*]. When %USERPROFILE%
> contains spaces, this results in tempdir() also containing spaces and
> prevents the user from being able to install source packages.
>
> As of <https://svn.r-project.org/R/trunk@77563>,
>
>  - src/library/utils/R/packages2.R line 839 contains an unquoted
>    temporary file path (fil) passed to system2(), which results in it
>    being split and R CMD INSTALL not being able to find the package
>    file. In other invocations of R CMD INSTALL in the same file, the
>    path is properly quoted.
>
>  - src/library/tools/R/check.R line 125 contains an unquoted temporary
>    file path passed to system2, which results in Rterm.exe -f not being
>    able to find the RtmpXXXXXX\RinXXXXXXXX file, causing the attempt to
>    run tools:::makeLazyLoading(...) to fail.
>
> I can report these two problems (thanks to Martin Maechler for the
> Bugzilla account and the advice) and attach the patches required to fix
> them, but there might be more. The bug report [**] is somewhat relevant
> here (though changing the default behaviour of system2() is obviously
> not the right solution as it would break existing code).
>
> Is there anything I should consider before creating the PR as
> described above?
>
> --
> Best regards,
> Ivan
>
> [*] https://stat.ethz.ch/pipermail/r-help/2019-December/465075.html
>
> [**] https://bugs.r-project.org/bugzilla/show_bug.cgi?id=16127
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel