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

Add recursive = TRUE to unlink functions in render.R #577

Merged
merged 3 commits into from May 9, 2018

Conversation

flyaflya
Copy link
Contributor

@flyaflya flyaflya commented May 9, 2018

The creation of temp files using the tempfile() function creates files with a path of ".\\filename". However, when using the unlink function with default of recursive = FALSE, these created temp files are not removed as expected. When recursive = TRUE is added to the unlink function call, then it works. Code which reproduces the problem on my machine is below:

## mimic creation of tempFile name as is done in bookdown::render.R
tempFile = tempfile('render', '.', '.rds')
print(tempFile)
file.exists(tempFile)  ### returns FALSE, nothing written to file

## write something to file
writeLines("This is a temp file", con = tempFile)
file.exists(tempFile)  ### returns TRUE

## use unlink with default value of recursive = FALSE
unlink(tempFile)

## check if file exists
file.exists(tempFile)  ### returns TRUE, file still exists!

## use unlink with non-default value of recursive = TRUE
unlink(tempFile, recursive = TRUE)

## check if file exists
file.exists(tempFile)  ### returns FALSE, finally got rid of it

## I do not know why this option needs to be TRUE as it does not
## seem we are doing a recursive search down a directory tree.
## My only guess is it has something to to with the path specification
## as tempFile = ".\\render2b885fcas674b.rds" and I am not familiar
## with the ".\\" notation.

flyaflya added 2 commits May 9, 2018
Without recursive = TRUE, files are not getting deleted.  See reproducible example (at least on my machine) below:

```{r}
tempFile = tempfile('render', '.', '.rds')
print(tempFile)
file.exists(tempFile)  ### returns FALSE, nothing written to file

## write something to file
writeLines("This is a temp file", con = tempFile)
file.exists(tempFile)  ### returns TRUE

## use unlink with default value of recursive = FALSE
unlink(tempFile)

## check if file exists
file.exists(tempFile)  ### returns TRUE, file still exists!

## use unlink with default value of recursive = FALSE
unlink(tempFile, recursive = TRUE)

## check if file exists
file.exists(tempFile)  ### returns FALSE, finally got rid of it

## I do not know why this option needs to be TRUE as it does not
## seem we are doing a recursive search down a directory tree.
## My only guess is it has something to to with the path specification
## as tempFile = ".\\render2b885fcas674b.rds" and I am not familiar
## with the ".\\" notation.
Added recursive = TRUE to other unlinks in the render file.  The previous commit worked so well, that this seemed like a good idea to save Yihui some time.
@yihui
Copy link
Member

@yihui yihui commented May 9, 2018

So basically unlink() is unable to delete a file on your system, which is surprising to me. Does file.remove() work?

A path .\foo basically means foo. The dot . indicates the current working directory.

@flyaflya
Copy link
Contributor Author

@flyaflya flyaflya commented May 9, 2018

I debugged a little more and it turns out having something to do with my use of OneDrive. It turns out that if the file is created in a OneDrive synced working directory, unlink(recursive = FALSE) will NOT remove it. When I create the file in a non-OneDrive folder, unlink(recursive = FALSE) will work. file.remove(), in addition to unlink(recursive = TRUE), does seem to work in both situations. Dropbox has no problems.

PS: Thanks for the path notation answer. My true confusion with the path is the double backslash ".\\render...". I would have just used a single backslash when defining the file path.

@yihui
Copy link
Member

@yihui yihui commented May 9, 2018

In R (and many other languages), when you see double backslashes in a character string, it means a single literal backslash (yes, this is confusing forever: https://xkcd.com/1638/).

One more test I need you to help me with: does unlink() work when path does not contain the leading .\? In your example above, change tempFile to

tempFile = basename(tempfile('render', '.', '.rds'))

@flyaflya
Copy link
Contributor Author

@flyaflya flyaflya commented May 9, 2018

The behavior is unchanged with tempFile = basename(tempfile('render', '.', '.rds')). The issue with the OneDrive synced directory remains.

Thanks for the cartoon ... makes me feel better about my confusion :-)

@yihui yihui added this to the v0.8 milestone May 9, 2018
yihui
yihui approved these changes May 9, 2018
Copy link
Member

@yihui yihui left a comment

I think file.remove() makes better sense in this case. Thanks!

@yihui yihui merged commit f848c0e into rstudio:master May 9, 2018
1 check was pending
yihui added a commit that referenced this issue May 9, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 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

2 participants