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

Optionize rmarkdown temp directory suffix #2550

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Conversation

certara-tzweers
Copy link
Contributor

We are making this pull request because we have been experiencing an issue with OneDrive deleting html files shortly after knitting them. We have been in email contact with @yihui about updating the knitr_files_dir funtion to aviod this issue (28 Mar 2024, Subject: Rmarkdown: patch for knitr_files_dir).

We have experienced apparently random deletions of html files from OneDrive synced folders shortly after completing the knitting of corresponding Rmd scripts. It appears that this is related to a feature (not bug...) described on this page:

Managing the File System - Win32 apps | Microsoft Learn

As such, the issue is a consequence of different components following their designs with an undesirable outcome. A solution to this could be to change the knitr_files_dir function in util.R so the temporary folder for knitr files is not created with a suffix of "_files" but almost anything else, such as "_knitrfiles".

With this update we have made the knitr temp directory suffix configurable via options(rmarkdown.files.suffix = "_knitrfiles") in the knitr_files_dir function.

@CLAassistant
Copy link

CLAassistant commented Apr 3, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

R/util.R Outdated Show resolved Hide resolved
Co-authored-by: Yihui Xie <xie@yihui.name>
@cderv
Copy link
Collaborator

cderv commented Apr 3, 2024

Thanks a lot.

Just a little more curious: What exactly happens with this connected files feature ?
When *_files folder is removed then the .html file is deleted too. Is that what happens?

Does it happen only on OneDrive folders?

Thank you !

@th3nme
Copy link

th3nme commented Apr 3, 2024

Thanks a lot.

Just a little more curious: What exactly happens with this connected files feature ? When *_files folder is removed then the .html file is deleted too. Is that what happens?

Does it happen only on OneDrive folders?

Thank you !

That is correct. Deleting either the .html or the folder will delete the other as well by default on a Windows device. This appears to be by design to keep the linked assets from a HTML file from breaking when the file is copied, moved or deleted etc.

The issue seems to surface when you have 2 or more users syncing a SharePoint folder while one of them is knitting. OneDrive (or any other sync tool) will start replicating the creates / deletes as the knitting is in progress from Person A to Person B. The remote user Person B receives an instruction to delete the temporary _files folder that was created and then deleted and that seems to be triggering the deletion of the .html file due to the Connected Files default as the computer not performing the knitting has no idea that the .html file should be preserved so it does what it thinks is correct and deletes the .html along with the _files folder.

The Connected Files behaviour can be configured by Group Policy to not link the 2 objects, but it is not clear what impact this would have on other applications and Microsoft does not recommend this unless absolutely necessary.

Adding the option to change the suffix will give the knitter a way of avoiding this issue but it may be worth considering changing the default from _files to literally almost anything else to avoid the Connected Files default behaviour.

@cderv
Copy link
Collaborator

cderv commented Apr 3, 2024

Thanks a lot. I did not know that and I am really happy to learn this today. This could explain some behavior we have seen for a long time now, and also others on another project using knitr and rmarkdown (Quarto).

Thanks again for finding this, and for the PR.

@th3nme
Copy link

th3nme commented Apr 3, 2024

While researching this issue we also found a post from 2019 describing the same issue with a Windows 10 and Ubuntu machine linked by Google Drive.

https://forum.posit.co/t/html-from-rmd-run-automatically-removed/25469/3

@yihui
Copy link
Member

yihui commented Apr 4, 2024

Originally we chose the _files suffix precisely because of the association mentioned above (when you copy/move foo.html, foo_files/ is copied/moved automatically). It seems this "intelligent" association does more harm than good in the case of OneDrive.

Adding the option to change the suffix will give the knitter a way of avoiding this issue but it may be worth considering changing the default from _files to literally almost anything else to avoid the Connected Files default behaviour.

I'm afraid it will be unlikely for us to change the default suffix in the future. It has been used for almost ten years now, and I'm sure that changing the default will create a lot of breaking changes to existing users...

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@certara-tzweers Although this is a simple one-liner PR, could you sign the CLA anyway? https://cla-assistant.io/rstudio/rmarkdown?pullRequest=2550

@yihui yihui merged commit 835335d into rstudio:main Apr 9, 2024
1 check passed
yihui added a commit to yihui/litedown that referenced this pull request Apr 29, 2024
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this pull request Aug 8, 2024
Merge remote-tracking branch 'rstudio/main' into jg-devel

# By Yihui Xie (10) and others
# Via GitHub
* rstudio/main:
  Fix Pandoc nightly install in GHA (rstudio#2559)
  feat(beamer): support latex_dependencies (rstudio#2558)
  start the next version
  CRAN release 2.27
  export S3 method
  rsconnect::deploySite() has combined the arguments account and server in the doc
  Update forum url (rstudio#2555)
  Fix regression w/ image path processing from absolute to relative (rstudio#2554)
  Update posit forum URL
  provide an option to set the `*_files/` directory suffix (rstudio#2550)
  start the next version
  CRAN release v2.26
  use _PACKAGE as recommended by roxygen2
  use \describe{} instead of \itemize{} (r-devel will warn against it)
  [GHA] Remove duplicate entry in matrix
  new r-lib/actions@v2 has newer setup-pandoc
  [GHA] New setup pandoc action (rstudio#2543)
  Correctly avoid writing to the input file when there are no preserved HTML chunks (rstudio#2535)
  Create FUNDING.yml
  Fix small grammatical error (rstudio#2533)

# Conflicts:
#	DESCRIPTION
#	R/publish_site.R
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this pull request Aug 8, 2024
* jg-devel: (21 commits)
  Builds and passes R CMD CHECK.
  Fix Pandoc nightly install in GHA (rstudio#2559)
  feat(beamer): support latex_dependencies (rstudio#2558)
  start the next version
  CRAN release 2.27
  export S3 method
  rsconnect::deploySite() has combined the arguments account and server in the doc
  Update forum url (rstudio#2555)
  Fix regression w/ image path processing from absolute to relative (rstudio#2554)
  Update posit forum URL
  provide an option to set the `*_files/` directory suffix (rstudio#2550)
  start the next version
  CRAN release v2.26
  use _PACKAGE as recommended by roxygen2
  use \describe{} instead of \itemize{} (r-devel will warn against it)
  [GHA] Remove duplicate entry in matrix
  new r-lib/actions@v2 has newer setup-pandoc
  [GHA] New setup pandoc action (rstudio#2543)
  Correctly avoid writing to the input file when there are no preserved HTML chunks (rstudio#2535)
  Create FUNDING.yml
  ...
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this pull request Aug 8, 2024
* rstudio/main:
  Fix Pandoc nightly install in GHA (rstudio#2559)
  feat(beamer): support latex_dependencies (rstudio#2558)
  start the next version
  CRAN release 2.27
  export S3 method
  rsconnect::deploySite() has combined the arguments account and server in the doc
  Update forum url (rstudio#2555)
  Fix regression w/ image path processing from absolute to relative (rstudio#2554)
  Update posit forum URL
  provide an option to set the `*_files/` directory suffix (rstudio#2550)
  start the next version
  CRAN release v2.26
  use _PACKAGE as recommended by roxygen2
  use \describe{} instead of \itemize{} (r-devel will warn against it)
  [GHA] Remove duplicate entry in matrix
  new r-lib/actions@v2 has newer setup-pandoc
  [GHA] New setup pandoc action (rstudio#2543)
  Correctly avoid writing to the input file when there are no preserved HTML chunks (rstudio#2535)
  Create FUNDING.yml
  Fix small grammatical error (rstudio#2533)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2024
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.

5 participants