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

Using .Rbuildignore #1383

Closed
pdbailey0 opened this Issue Mar 22, 2017 · 16 comments

Comments

3 participants
@pdbailey0

pdbailey0 commented Mar 22, 2017

It seems knitr is ignoring the .Rbuildignore file when deciding what .Rmd files to knit.

roxygen2 was recently edited to allow it to ignore what is in .Rbuildignore so you can see example code here

Reproducible example: add the name of a knitr .Rmd file to your .Rbuildignore.
Expected result: the .pdf does not end up in the .tar.gz of the package.
Actual result: the .pdf does end up in the .tar.gz file. Also R CMD check gives a WARNING.

@yihui

This comment has been minimized.

Show comment
Hide comment
@yihui

yihui Mar 22, 2017

Owner

I don't get it -- why not add .pdf to .Rbuildignore?

Owner

yihui commented Mar 22, 2017

I don't get it -- why not add .pdf to .Rbuildignore?

@pdbailey0

This comment has been minimized.

Show comment
Hide comment
@pdbailey0

pdbailey0 Mar 22, 2017

I didn't add the suffix to .Rbuildignore, so it should also be ignored. See here.

pdbailey0 commented Mar 22, 2017

I didn't add the suffix to .Rbuildignore, so it should also be ignored. See here.

@yihui

This comment has been minimized.

Show comment
Hide comment
@yihui

yihui Mar 22, 2017

Owner

so it should also be ignored

Why? The R-exts manual did not say so.

As nrussell asked you on StackOverflow, what is the line from your .Rbuildignore file?

Please be sure to provide a minimal reproducible example (per contributing guide) instead of describing by words. The latter way is often ambiguous, and it can take many rounds before I can understand what you did exactly.

Owner

yihui commented Mar 22, 2017

so it should also be ignored

Why? The R-exts manual did not say so.

As nrussell asked you on StackOverflow, what is the line from your .Rbuildignore file?

Please be sure to provide a minimal reproducible example (per contributing guide) instead of describing by words. The latter way is often ambiguous, and it can take many rounds before I can understand what you did exactly.

@pdbailey0

This comment has been minimized.

Show comment
Hide comment
@pdbailey0

pdbailey0 Mar 23, 2017

Here is an example https://github.com/pdbailey0/knitIgnore

From R CMD check

* checking 'build' directory ... WARNING
Output(s) listed in 'build/vignette.rds' but not in package:
  'inst/doc/ignoredVignette.pdf'

remove the .Rbuildignore and that will go away

pdbailey0 commented Mar 23, 2017

Here is an example https://github.com/pdbailey0/knitIgnore

From R CMD check

* checking 'build' directory ... WARNING
Output(s) listed in 'build/vignette.rds' but not in package:
  'inst/doc/ignoredVignette.pdf'

remove the .Rbuildignore and that will go away

@yihui

This comment has been minimized.

Show comment
Hide comment
@yihui

yihui Mar 26, 2017

Owner

Okay, that is a much better bug report. I can see the issue with build/vignette.rds. However, this is out of my control (because of this). R's API tools::vignetteEngine() only gives me a pattern argument, and it is not straightforward for me to merge the patterns you specify in .Rbuildignore with the filename patterns in knitr.

This issue is not specific to knitr, either. It is a general issue of R package vignettes. For example, you can change Rmd vignettes to Rnw (Sweave), remove VignetteBuilder from DESCRIPTION, and see the same issue.

So you should report it to R core instead. Or ping @dmurdoch if you are feeling lucky.

Owner

yihui commented Mar 26, 2017

Okay, that is a much better bug report. I can see the issue with build/vignette.rds. However, this is out of my control (because of this). R's API tools::vignetteEngine() only gives me a pattern argument, and it is not straightforward for me to merge the patterns you specify in .Rbuildignore with the filename patterns in knitr.

This issue is not specific to knitr, either. It is a general issue of R package vignettes. For example, you can change Rmd vignettes to Rnw (Sweave), remove VignetteBuilder from DESCRIPTION, and see the same issue.

So you should report it to R core instead. Or ping @dmurdoch if you are feeling lucky.

@yihui yihui closed this Mar 26, 2017

@yihui

This comment has been minimized.

Show comment
Hide comment
@yihui

yihui Mar 26, 2017

Owner

BTW, if I were you, instead of waiting for this issue to be fixed in base R, I'd rather use git branches instead. Then you won't need help from anyone else.

Owner

yihui commented Mar 26, 2017

BTW, if I were you, instead of waiting for this issue to be fixed in base R, I'd rather use git branches instead. Then you won't need help from anyone else.

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Mar 26, 2017

Contributor
Contributor

dmurdoch commented Mar 26, 2017

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Mar 26, 2017

Contributor
Contributor

dmurdoch commented Mar 26, 2017

@yihui

This comment has been minimized.

Show comment
Hide comment
@yihui

yihui Mar 27, 2017

Owner

Perfect. Thanks, Duncan.

wch/r-source@b6d5602

Owner

yihui commented Mar 27, 2017

Perfect. Thanks, Duncan.

wch/r-source@b6d5602

@yihui yihui added this to the v1.16 milestone Mar 27, 2017

@pdbailey0

This comment has been minimized.

Show comment
Hide comment
@pdbailey0

pdbailey0 Mar 27, 2017

Thanks, @dmurdoch! and thanks @yihui for making me make a clearer example and bringing Duncan into this.

pdbailey0 commented Mar 27, 2017

Thanks, @dmurdoch! and thanks @yihui for making me make a clearer example and bringing Duncan into this.

@pdbailey0

This comment has been minimized.

Show comment
Hide comment
@pdbailey0

pdbailey0 Apr 25, 2017

@dmurdoch, oddly I still get this behavior in 3.4.0.

Not sure where I should discuss this (Thanks for continuing to host this on your github yihui.)

pdbailey0 commented Apr 25, 2017

@dmurdoch, oddly I still get this behavior in 3.4.0.

Not sure where I should discuss this (Thanks for continuing to host this on your github yihui.)

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Apr 25, 2017

Contributor

@pdbailey0, do you have a test package I can try?

Contributor

dmurdoch commented Apr 25, 2017

@pdbailey0, do you have a test package I can try?

@pdbailey0

This comment has been minimized.

Show comment
Hide comment
@pdbailey0

pdbailey0 Apr 25, 2017

@dmurdoch I have a link up above. There is a lot there, so I'll point you to this comment.

pdbailey0 commented Apr 25, 2017

@dmurdoch I have a link up above. There is a lot there, so I'll point you to this comment.

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Apr 25, 2017

Contributor

Thanks. It's easy to reproduce the problem; not sure why I thought it was fixed before. I'll try again in R-patched soon.

Contributor

dmurdoch commented Apr 25, 2017

Thanks. It's easy to reproduce the problem; not sure why I thought it was fixed before. I'll try again in R-patched soon.

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Apr 25, 2017

Contributor

I've redone the fix in R-devel. I'll wait to port it to R-patched until after some testing. To see the change, you need rev 72622 or higher of R-devel.

Contributor

dmurdoch commented Apr 25, 2017

I've redone the fix in R-devel. I'll wait to port it to R-patched until after some testing. To see the change, you need rev 72622 or higher of R-devel.

@dmurdoch

This comment has been minimized.

Show comment
Hide comment
@dmurdoch

dmurdoch Apr 26, 2017

Contributor

@pdbailey0, it's now also in R-patched, as of revision 72629. Please test before the release of 3.4.1, and let me know if I've missed things again.

Contributor

dmurdoch commented Apr 26, 2017

@pdbailey0, it's now also in R-patched, as of revision 72629. Please test before the release of 3.4.1, and let me know if I've missed things again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment