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

Waldo install fails with dev glue #331

Closed
gaborcsardi opened this issue Aug 5, 2024 · 9 comments · Fixed by #333
Closed

Waldo install fails with dev glue #331

gaborcsardi opened this issue Aug 5, 2024 · 9 comments · Fixed by #333

Comments

@gaborcsardi
Copy link
Member

I don't know where the error lies, I am merely recording this here:

* installing *source* package ‘waldo’ ...
** package ‘waldo’ successfully unpacked and MD5 sums checked
** using staged installation
** R
** byte-compile and prepare package for lazy loading
Error in get(genname, envir = envir) : object 'compare_proxy' not found
Calls: <Anonymous> ... register_S3_method_delayed -> registerS3method -> get
Execution halted
ERROR: lazy loading failed for package ‘waldo’
* removing ‘/Users/gaborcsardi/Library/R/arm64/4.4/library/waldo’
* restoring previous ‘/Users/gaborcsardi/Library/R/arm64/4.4/library/waldo’
@jennybc
Copy link
Member

jennybc commented Aug 5, 2024

I suspect this is related to #310. @hadley do you have any thoughts?

@hadley
Copy link
Member

hadley commented Aug 5, 2024

I suspect you're right. Maybe this is a bug in R-devel?

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Aug 6, 2024

IDK if it is a bug in base R, but it seems that this has never actually worked. We don't see errors on the CI because we don't build waldo from source, except on R-devel. But it does fail the same way in all R versions I tried, 4.4.1, 4.3.x, 4.2.x, etc. 4.0.5, and also 3.6.3:

docker run -ti ghcr.io/r-hub/r-minimal/r-minimal:3.6.3 bash
installr -p -c tidyverse/glue waldo
** byte-compile and prepare package for lazy loading
Error in get(genname, envir = envir) : object 'compare_proxy' not found
Calls: <Anonymous> ... register_S3_method_delayed -> registerS3method -> get

@hadley
Copy link
Member

hadley commented Aug 6, 2024

Hmmmm, interesting. I will try and figure out what makes compare_proxy different from the other method. Or maybe it's just the first loaded?

@gaborcsardi
Copy link
Member Author

My (very uneducated) guess is that the difference is that waldo imports glue via NAMESPACE, whereas vctrs does not. So when waldo is being installed, glue is loaded, and that triggers the conditional generic, and R tries to load the generic, but since waldo is in a half-loaded state, it cannot.

So a simple workaround would be (if above is correct) to not import glue in NAMESPACE from waldo.

hadley added a commit to r-lib/waldo that referenced this issue Aug 6, 2024
@hadley
Copy link
Member

hadley commented Aug 6, 2024

Do you have any easier way to test that? I made r-lib/waldo#195.

(This does feel like a bug in R to me, since it means you have to think more about how your packages interact, which our previous solution did not 😬)

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Aug 6, 2024

Yep, your fix works. This is how to test it easily if you don't want to overwrite your system:

docker run -ti ghcr.io/r-hub/r-minimal/r-minimal:4.4.1 bash
installr -p -c tidyverse/glue r-lib/waldo@dont-import-glue

I agree that it is a bug in base R. A half-loaded package should not trigger the conditional S3 methods. (Although that'll make waldo's glue import behave differently at install time than at run time, but still better.)

@gaborcsardi
Copy link
Member Author

Base R bug report: https://bugs.r-project.org/show_bug.cgi?id=18776

@jennybc
Copy link
Member

jennybc commented Aug 6, 2024

We've encountered a variant of this problem before, with knitr.

See #253 and #254 (this PR especially contains a lot of write-up).

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.

3 participants