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

file_exists() does not perform path expansion (anymore) #325

Closed
lorenzwalthert opened this issue Apr 15, 2021 · 7 comments · Fixed by #361
Closed

file_exists() does not perform path expansion (anymore) #325

lorenzwalthert opened this issue Apr 15, 2021 · 7 comments · Fixed by #361

Comments

@lorenzwalthert
Copy link

lorenzwalthert commented Apr 15, 2021

on macOS. Maybe related to #295? This is likely to break the reverse dependency test.

> fs::file_exists(fs::path_expand('~'))
/Users/lorenz 
         TRUE 
> fs::file_exists('~')
    ~ 
FALSE 
> packageVersion('fs')
[1] ‘1.5.0.9000

Previously, this worked:

> fs::file_exists(fs::path_expand('~'))
/Users/lorenz 
         TRUE 
> fs::file_exists('~')
    ~ 
TRUE 
> packageVersion('fs')
[1] ‘1.5.

Related: #222

@lorenzwalthert
Copy link
Author

lorenzwalthert commented Nov 30, 2021

This breaks {precommit} for a subset of users (part of {precommit} that's not tested on CRAN) now that {fs} is updated on CRAN and I forgot about it and found the half a year old issue. This potentially breaks other peoples code. Will the old behavior restored?

@gaborcsardi
Copy link
Member

Yes, it seems that now we need to call path_expand() in file_exists(). If this breaks lots of code, then we can have a patch release soon.

@lorenzwalthert
Copy link
Author

lorenzwalthert commented Nov 30, 2021

I mean it (apparently) did not break CRAN packages but I imagine people also have this in in their analysis code. I'd appreciate a patch release but I'll anyways release a new {precommit} version in the next 7 days.

@gaborcsardi
Copy link
Member

It seems to me that most file_* functions call path_expand(), so file_exists() should do the same.

salim-b added a commit to zdaarau/fokus that referenced this issue Dec 6, 2021
salim-b added a commit to salim-b/pkgpins that referenced this issue Dec 6, 2021
@salim-b
Copy link
Contributor

salim-b commented Dec 6, 2021

I mean it (apparently) did not break CRAN packages (...)

It actually did break the pins pkg when used with local folder boards created with e.g. pins::board_folder(path = "~/my_board") because of this line. Just cost me over an hour to figure that out. 😬

@gaborcsardi
Copy link
Member

I will make a new release in ~ 2 days.

@jennybc
Copy link
Member

jennybc commented Dec 7, 2021

It turns out this is being felt via usethis as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants